Skip to content

build: fix versioningit complaints in shallow git clones #2518

Merged
egparedes merged 6 commits intoGridTools:mainfrom
egparedes:build/fix-versioningit-complaints
Mar 12, 2026
Merged

build: fix versioningit complaints in shallow git clones #2518
egparedes merged 6 commits intoGridTools:mainfrom
egparedes:build/fix-versioningit-complaints

Conversation

@egparedes
Copy link
Contributor

@egparedes egparedes commented Mar 10, 2026

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.

Copilot AI review requested due to automatic review settings March 10, 2026 10:53
@egparedes egparedes requested a review from DropD March 10, 2026 10:55
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 versioningit fallback configuration from tool.versioningit.default-version to tool.versioningit.vcs.default-tag.
  • Update scripts/update.py to 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.

Copy link
Contributor

@DropD DropD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to work much better than my temporary fix over at #2484

Copy link
Contributor

@DropD DropD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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'

Copy link
Contributor

@DropD DropD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found out it doesn't fix the problem yet.

@egparedes egparedes requested a review from DropD March 12, 2026 09:31
Copy link
Contributor

@DropD DropD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works! Just one optional suggestion.

Comment on lines +51 to +63
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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use contextlib.redirect_stdout and contextlib.redirect_stderr here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@egparedes egparedes merged commit f774be1 into GridTools:main Mar 12, 2026
31 checks passed
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

Successfully merging this pull request may close these issues.

3 participants