Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Semantic versioning #43

Closed
serenity4 opened this issue Apr 30, 2021 · 13 comments
Closed

Semantic versioning #43

serenity4 opened this issue Apr 30, 2021 · 13 comments

Comments

@serenity4
Copy link
Member

As I understand it, VulkanCore uses a particular convention for versions, where the major and minor numbers are identical to the wrapped Vulkan version, and the patch number being the "version" of the wrapper itself.

The problem is that it is not in line with the semantic versioning of other packages. Therefore it may lead to issues like JuliaGPU/Vulkan.jl#16 because releases are assumed to never be breaking unless the major version of Vulkan changes (which does not happen often!). However, we may have breaking changes in how we wrap things, or even the specification itself may yield breaking changes (version 1.2.162 did).

Furthermore, since we now have the headers as an Artifact (Vulkan_Headers_jll) whose version is that of the Vulkan specification, I see little need in tracking the version also here and even less so as our package version.

Therefore I propose to respect semantic versioning from now on for future releases.

@Gnimuc
Copy link
Member

Gnimuc commented Apr 30, 2021

The Patch version is simply used to indicate what specific weekly release of the Vulkan specification and headers was used when generating the files. The Patch version should be ignored in general application development.

According to LunarG's whitepaper, there shouldn't be any breaking changes between the patch releases.

There might be something we did it wrong when generating those bindings, I'd say we should figure out the root cause, make a fix and bump the patch version again.

@serenity4
Copy link
Member Author

serenity4 commented Apr 30, 2021

The core + extensions should be stable, yes. The thing is, we include beta extensions, and those may break. Breakings may include a variable being defined in a version and removed in another one, which results in not being able to use the package because of an UndefVarError.
We can choose not to include beta extensions, but since VulkanCore is supposed to be the entry point to the API, I don't see why we wouldn't include everything.

@Gnimuc
Copy link
Member

Gnimuc commented Apr 30, 2021

Maybe it's a good idea to separate out those fragile extensions into another package(or even packages) like VulkanExtensions.jl?

@Gnimuc
Copy link
Member

Gnimuc commented Apr 30, 2021

why we wouldn't include everything.

For now, those platform-specific extensions are all bundled into the package, which, I think, is also wrong. I have already found some bugs as reported in #41.

I'm still exploring how to get better and robust multi-platform support. LibCURL is a good example and I believe we should do the same to VulkanCore, especially those platform-specific extensions.

@serenity4
Copy link
Member Author

For now, those platform-specific extensions are all bundled into the package, which, I think, is also wrong. I have already found some bugs as reported in #41.

You're right. Since Clang can't cross-compile, maybe we should move platform-specific code generation to a package build step, so that the host generates code? Unless we have a handy system for generating code for each platform so that we can conditionally include the right files (like LibCURL)?

Maybe it's a good idea to separate out those fragile extensions into another package(or even packages) like VulkanExtensions.jl?

Yes, this looks better. We could have:

  • VulkanCore.jl: wraps only the core Vulkan API, which is platform-independent, no build step or platform-dependent tweaking is required and follows Vulkan semver
  • VulkanExtensions.jl: includes extensions including platform-dependent extensions (with the VK_USE_PLATFORM... flags) and follows Vulkan semver
  • VulkanBetaExtensions.jl: includes all extensions which do not follow Vulkan semver (with the VK_ENABLE_BETA_EXTENSIONS flag).

In my comment I said:

The core + extensions should be stable, yes.

But I'm not sure about the second part actually. Do extensions have a guaranteed stability like the core, at the exception of course of those from VK_ENABLE_BETA_EXTENSIONS?

@Gnimuc
Copy link
Member

Gnimuc commented Apr 30, 2021

maybe we should move platform-specific code generation to a package build step, so that the host generates code?

This is what I've done my best to avoid. Clang.jl should be able to generate exactly the same code regardless of the host system environment. (To do something on the users' machine is always problematic, the only stable operation is probably downloading...)

The current Clang master is working at a full cross-compile mode(I even removed the support for the host build), users can now run the generator without having a compiler(libc headers) or system headers installed.

Do extensions have a guaranteed stability like the core, at the exception of course of those from VK_ENABLE_BETA_EXTENSIONS?

I don't know too. I guess we could ask somebody in the Vulkan community to get a confirmation.

@serenity4
Copy link
Member Author

The current Clang master is working at a full cross-compile mode(I even removed the support for the host build), users can now run the generator without having a compiler(libc headers) or system headers installed.

That's great! So conditional includes as being done on LibCURL could do the trick I guess.

I guess we could ask somebody in the Vulkan community to get a confirmation.

I asked on the Khronos Slack, we'll see if someone answers. Otherwise from KhronosGroup/Vulkan-Headers#167 I get the feel that only beta extensions should be allowed to break.

@Gnimuc
Copy link
Member

Gnimuc commented Apr 30, 2021 via email

@serenity4
Copy link
Member Author

Mine was quicker 😄

Are some extensions (other than beta extensions) allowed to break between minor/patch releases? Or should they be as stable as the core API?

for core versioning it states that change in patch version requires full compatibility while change in minor version requires backwards compatibility
https://www.khronos.org/registry/vulkan/specs/1.2-extensions/html/vkspec.html#_compatibility_guarantees_informative
not 100% if this also applies to extension versions but since i dont see no seperate specification for extension version numbers i expect them to use the same

@serenity4
Copy link
Member Author

serenity4 commented Apr 30, 2021

So, it looks like KHR extensions are as stable as the core API, and multi-vendor/vendor extensions should be but not necessarily. Also, citing the spec:

If extension functionality is promoted, there is no guarantee of direct compatibility

and large changes have to be marked as deprecated or obsoleted. This table shows the extensions associated with the platform-specific flags.

It looks like there is quite a difference between the very stable core API and stable-ish extensions. It would make sense to me to separate those into a separate package for extensions.

EDIT: Regarding the sentence about promotion above, I don't think it applies to the headers being broken, more so about transitioning from the extension to the core interface. Thinking more about it, maybe just provisional (beta) extensions should be in a separate package, if required. We may just decide not to support them in VulkanCore. What do you think?

@Gnimuc
Copy link
Member

Gnimuc commented May 4, 2021

This table shows the extensions associated with the platform-specific flags.

Could you paste the table here? I cannot open that link.

FYI, I've added cross-platform support in #39.

@serenity4
Copy link
Member Author

Here is the table in question:

Extension Name Window System Name Platform-specific Header Required External Headers Controlling vulkan.h Macro
VK_KHR_android_surface Android vulkan_android.h None VK_USE_PLATFORM_ANDROID_KHR
VK_KHR_wayland_surface Wayland vulkan_wayland.h <wayland-client.h> VK_USE_PLATFORM_WAYLAND_KHR
VK_KHR_win32_surface, VK_KHR_external_memory_win32, VK_KHR_win32_keyed_mutex, VK_KHR_external_semaphore_win32, VK_KHR_external_fence_win32, VK_NV_external_memory_win32, VK_NV_win32_keyed_mutex Microsoft Windows vulkan_win32.h <windows.h> VK_USE_PLATFORM_WIN32_KHR
VK_KHR_xcb_surface X11 Xcb vulkan_xcb.h <xcb/xcb.h> VK_USE_PLATFORM_XCB_KHR
VK_KHR_xlib_surface X11 Xlib vulkan_xlib.h <X11/Xlib.h> VK_USE_PLATFORM_XLIB_KHR
VK_EXT_directfb_surface DirectFB vulkan_directfb.h <directfb/directfb.h> VK_USE_PLATFORM_DIRECTFB_EXT
VK_EXT_acquire_xlib_display X11 XRAndR vulkan_xlib_xrandr.h <X11/Xlib.h>, <X11/extensions/Xrandr.h> VK_USE_PLATFORM_XLIB_XRANDR_EXT
VK_GGP_stream_descriptor_surface, VK_GGP_frame_token Google Games Platform vulkan_ggp.h <ggp_c/vulkan_types.h> VK_USE_PLATFORM_GGP
VK_MVK_ios_surface iOS vulkan_ios.h None VK_USE_PLATFORM_IOS_MVK
VK_MVK_macos_surface macOS vulkan_macos.h None VK_USE_PLATFORM_MACOS_MVK
VK_NN_vi_surface VI vulkan_vi.h None VK_USE_PLATFORM_VI_NN
VK_FUCHSIA_imagepipe_surface Fuchsia vulkan_fuchsia.h <zircon/types.h> VK_USE_PLATFORM_FUCHSIA
VK_EXT_metal_surface Metal on CoreAnimation vulkan_metal.h None VK_USE_PLATFORM_METAL_EXT
VK_QNX_screen_surface QNX Screen vulkan_screen.h <screen/screen.h> VK_USE_PLATFORM_SCREEN_QNX

@serenity4
Copy link
Member Author

I think we can close this now, with the conclusion that semantic versioning of VulkanCore.jl will follow that of Vulkan, and that beta extensions won't be supported (in this repo at least) for stability/compatibility reasons.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants