build: fix versioningit complaints in shallow git clones #2518
build: fix versioningit complaints in shallow git clones #2518egparedes merged 6 commits intoGridTools:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the project’s versioningit configuration to avoid noisy/version-resolution issues when the repository is available only as a shallow clone (as in CI), by shifting the fallback version source to the VCS-specific fallback option.
Changes:
- Switch
versioningitfallback configuration fromtool.versioningit.default-versiontotool.versioningit.vcs.default-tag. - Update
scripts/update.pyto read/update the new config key. - Minor refactor/clarification in
src/gt4py/__about__.py(rename cached-version variable and update related comments).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/gt4py/__about__.py |
Clarifies fallback-version comments and renames the cached version tuple for readability. |
scripts/update.py |
Updates the version-bump helper to use tool.versioningit.vcs.default-tag. |
pyproject.toml |
Moves fallback version to tool.versioningit.vcs.default-tag to better cover shallow-clone scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
DropD
left a comment
There was a problem hiding this comment.
I just realized this suffers from the same problem as what I did in #2484:
In C2SM/icon4py#1096, i fixed gt4py to my branch, to make sure not to brake anything. But now uv sync --upgrade-package gt4py fails with
packaging.version.InvalidVersion: Invalid version: '1.1.6+unknown.version.details.post1841+82d25bed'
DropD
left a comment
There was a problem hiding this comment.
Found out it doesn't fix the problem yet.
…void any output being written
DropD
left a comment
There was a problem hiding this comment.
Works! Just one optional suggestion.
| try: | ||
| old_stdout_fd = os.dup(1) # duplicate current stdout fd | ||
| os.dup2(devnull.fileno(), 1) # replace fd 1 with /dev/null | ||
| old_stderr_fd = os.dup(2) | ||
| os.dup2(devnull.fileno(), 2) | ||
|
|
||
| version = versioningit.get_version(path) | ||
|
|
||
| finally: | ||
| os.dup2(old_stdout_fd, 1) | ||
| os.close(old_stdout_fd) | ||
| os.dup2(old_stderr_fd, 2) | ||
| os.close(old_stderr_fd) |
There was a problem hiding this comment.
Why not use contextlib.redirect_stdout and contextlib.redirect_stderr here?
There was a problem hiding this comment.
I don't think it works here because versioningit opens a new subprocess to call git and I assumed that the contextlib redirects would only modify the descriptors for the current cpython process, but not for other processes. To be honest, I didn't even try that, because a couple of LLMs also agreed and suggested to use this approach.
Suppress output of 'versioningit` when there are non-fatal errors getting tag information from shallow clones. Add small cleanups and fixes for the scripts and testing around project version handling.