Handle Missing Tags in Versioning by Setting Default to 0#3359
Handle Missing Tags in Versioning by Setting Default to 0#3359pshriwise merged 4 commits intoopenmc-dev:developfrom
Conversation
paulromano
left a comment
paulromano
left a comment
There was a problem hiding this comment.
Looks good to me but I'll hold off on merging in case @pshriwise or @MicahGale have any feedback.
| if(VERSION_STRING STREQUAL "") | ||
| message(FATAL_ERROR "No git tags found. Run 'git fetch --tags' and try again.") | ||
| set(VERSION_STRING "0.0.0") | ||
| message(WARNING "No git tags found. Version set to 0. Run 'git fetch --tags' to ensure proper versioning.") |
There was a problem hiding this comment.
There's more to it than just fetching for forks. One must add the upstream remove (this repo) and fetch tags from there. If the tags aren't in their forked repo this won't help sadly.
There was a problem hiding this comment.
Yeah I think maybe drop the "here's how to fix it" bit, because it's a bit too nuanced for cmake. I think the bigger thing is having a guide in the developer docs.
There was a problem hiding this comment.
I think more detail in the dev guide, yes. The warning here can provide direction to that section ideally.
MicahGale
left a comment
MicahGale
left a comment
There was a problem hiding this comment.
I think updating developer docs should be part of this PR, unless this needs to be deployed asap.
|
I have created a new section in the doc for this since it is optional. However, we can update the documentation if needed. |
MicahGale
left a comment
MicahGale
left a comment
There was a problem hiding this comment.
The docs look good and really help with "minimizing astonishment"
MicahGale
left a comment
MicahGale
left a comment
There was a problem hiding this comment.
LGTM (GH didn't include comment in last review???)
Co-authored-by: Micah Gale <mgale@fastmail.com>
pshriwise
left a comment
pshriwise
left a comment
There was a problem hiding this comment.
Teeny tiny clarification and then I'm happy. Thanks @ahnaf-tahmid-chowdhury!
Co-authored-by: Patrick Shriwise <pshriwise@gmail.com>
|
It seems the workflow run was canceled. Is there anything else to update @pshriwise? |
…#3359) Co-authored-by: Micah Gale <mgale@fastmail.com> Co-authored-by: Patrick Shriwise <pshriwise@gmail.com>
…#3359) Co-authored-by: Micah Gale <mgale@fastmail.com> Co-authored-by: Patrick Shriwise <pshriwise@gmail.com>
…#3359) Co-authored-by: Micah Gale <mgale@fastmail.com> Co-authored-by: Patrick Shriwise <pshriwise@gmail.com>
4 participants
Problem
When forking and cloning OpenMC, all tags are not always present in the resulting clone. This causes issues with CMake's git-based versioning pathway:
Failure case: If
gitis available but no tags exist, version detection fails.Incorrect versioning: The git SHA is correct, but the displayed version is incorrect.
Solution
Updated
GetVersionFromGit.cmaketo follow thesetuptools_scmconvention.If no git tags are found, the version defaults to
0.0.0instead of failing.A warning message is displayed, instructing users to run
git fetch --tags.Notes
Users who wish to fetch tags can do so manually:
Ready for Review
Tagging @pshriwise and @MicahGale for feedback.