fix(core): exclude vm-dev tag from git describe version glob#843
Open
mjamiv wants to merge 1 commit intoNVIDIA:mainfrom
Open
fix(core): exclude vm-dev tag from git describe version glob#843mjamiv wants to merge 1 commit intoNVIDIA:mainfrom
mjamiv wants to merge 1 commit intoNVIDIA:mainfrom
Conversation
`git describe --tags --long --match "v*"` matches the `vm-dev` tag
alongside release tags. When `vm-dev` sits on or past the latest
release tag in the commit graph (currently the case on main), git
picks it, and the resulting `vm-dev-N-gSHA` string strips the leading
`v` down to `m-dev-N-gSHA`. With `commits == 0` that's returned
verbatim, producing a binary that reports itself as `m-dev`:
$ openshell --version
openshell m-dev
Confirmed on v0.0.28 (NVIDIA#832) and reproduced on v0.0.29 as well.
Restrict the glob to numeric release tags (`v[0-9]*`). All release
tags follow `v\d+\.\d+\.\d+`, so this loses no valid version — it
only filters out `vm-dev`, `vm-prod`, and any similar non-release
tags that share the `v` prefix.
Verified locally on this repo's current HEAD:
# before
$ git describe --tags --long --match 'v*'
vm-dev-0-g355d845
# after
$ git describe --tags --long --match 'v[0-9]*'
v0.0.29-2-g355d845
Closes NVIDIA#832
Signed-off-by: mjamiv <michael.commack@gmail.com>
|
Thank you for your submission! We ask that you sign our Developer Certificate of Origin before we can accept your contribution. You can sign the DCO by adding a comment below using this text: I have read the DCO document and I hereby sign the DCO. You can retrigger this bot by commenting recheck in this Pull Request. Posted by the DCO Assistant Lite bot. |
Author
|
I have read the DCO document and I hereby sign the DCO. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Exclude the
vm-devdevelopment tag from thegit describeglob that derives the build-time version string. Currently--match "v*"catches it, producing a binary that reports itself asm-dev. Fixed by requiring a numeric segment afterv.Related Issue
Closes #832
Changes
crates/openshell-core/build.rs::git_version: change--match "v*"to--match "v[0-9]*". Leaves a short comment explaining why (thevm-devcollision) so the next reader doesn't think the glob is over-restrictive.Root Cause
git describe --tags --long --match "v*"matches every tag that starts withv, includingvm-devandvm-prod. On current main,vm-devsits on or past the latest release tag in the commit graph, so git picks it:git_versionthen strips the leadingv(line 77), producingm-dev-0-g355d845, which parses cleanly astag="m-dev",commits=0and is returned verbatim. The resulting binary reports:The original issue logged this for v0.0.28; I reproduced it on v0.0.29 as well (installed via
uv tool install openshell==0.0.29 --force, samem-devoutput). See comment: #832 (comment)Fix verification
On current repo HEAD:
The fixed glob walks past
vm-dev/vm-prodand lands on the newest numeric release tag. All real release tags followv\d+\.\d+\.\d+, so this loses no valid version — it only filters out the dev tags that the format string was never meant to interpret.The
guess-next-devpath on line 90 (v0.0.29-2-gSHA→0.0.30-dev.2+g355d845) now works as intended.Testing
cargo check --package openshell-corecompiles cleancargo fmt --package openshell-core -- --checkcleangit describedirectly — the fix is one glob character so unit tests aren't useful, but the before/after demonstration on real tags ismise run pre-commit—misenot installed in this environment; rancargo fmt --check+cargo checkfor the affected crate by handChecklist
fix(core): …