-
Notifications
You must be signed in to change notification settings - Fork 380
Add SLANG_VERSION for source archives #9020
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
Conversation
Fixes shader-slang#9019 Up until now, we've used git describe as the primary source of truth for the version number. To be clear, it still is, but we need to consider what downstream consumers and packagers of Slang may be doing. e.g., the following use cases: - Rebuilding from source archives - Rebuilding from source archives ingested into a separate version control system To that end, we define a VERSION file, and enable git export-subst on it. For source releases created using git archive, this will replace the contents of VERSION with the most recent version number, as expressed relative to the most recent version tag in the Slang git repository (our source of truth). The cmake rule is updated to prioritize the VERSION file (if and only if export-subst has been applied to the file) higher than the git describe contents, since the source package may have been ingested into another VCS (possibly also git). It's possible this won't be accurate--additional patches may have been applied on top--but we're doing what we can here to ensure the versioning on the filenames is correct. Not every eventuality can be forseen.
Rename `VERSION` to `SLANG_VERSION` to avoid a macOS-specific build failure. On macOS, the filesystem is often configured as case-insensitive. Well, C++ STL includes a file called `version`. Unfortunately, the root of the repository is getting included for whatever reason, which means that #include <version> picked up our `VERSION` file instead of the `version` file it was expecting..
Since Slang builds use git tags extracted via git describe as the source of truth for version information, we should ensure the tags are available whenever we build (both for CI and for Releases). For release builds, this issue was obscured because we used the triggering ref to determine the version. This worked, for the most part, except for some internal versioning information not readily visible, which may have been incorrect. For CI builds, this appears to have been incorrect for some time, but not necessarily an issue, since we don't normally need to have the version numbers 'correct' in CI. In practice, though, we should probably be using the same path for CI as for Releases so we don't have diverging behavior.
|
/format |
|
🌈 Formatted, please merge the changes from this PR |
Format code for PR shader-slang#9020
Let's skip fetching tags for CI. While the difference in time seems to be negligible, I suspect that having a stable library filename (like we would with version 0.0.0) might be advantageous for scripting things like coverage information.
|
Hm, testing this is non-trivial because submodules aren't included by |
In the release workflows, we use the triggering ref as the source of version information, when generating the package filenames. But the triggering ref can be a branch name, and a common pattern is for these branch names to have a hierarchy, e.g., "dev/USER/branchname". Hence the sanitization of version information.
Renames SLANG_VERSION to slang_git_version, and hides it under the cmake directory.
jkwak-work
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
jkiviluoto-nv
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Improve the explanation of why git tags are needed, and how to get them.
bb9fae4
jkwak-work
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Fixes #9019
Slang already depended on git tags for version information, but a recent change to add library versioning created an additional dependency on git to provide the library version number when generating the library filenames. This information is simply not available in a source package. (The source packages we generate don't configure correctly with the preset profiles due to the submodules not being included in the packaging, but that doesn't mean we should make the problem worse.)
To address this, a
cmake/slang_git_versionfile has been added to store the version number, which is filled out automatically ongit archiveusing git'sexport-subst. For source releases created usinggit archive, this will replace the contents ofslang_git_versionwith the most recent version number.GitVersion.cmakeis updated to prioritize the contents of theslang_git_versionfile (if and only ifexport-substhas been applied to the file) higher than thegit describeresults. This order was chosen in case the source archive was subsequently ingested into a different git repository.