-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Inference of setuptools' own version from git history (without setuptools-scm) #4948
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
base: main
Are you sure you want to change the base?
Conversation
@@ -71,6 +71,8 @@ jobs: | |||
timeout-minutes: 75 | |||
steps: | |||
- uses: actions/checkout@v4 | |||
with: | |||
fetch-depth: 0 |
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.
In the absence of the commit/tag history it is not possible to infer the version correctly. So we need a deep checkout.
This has been discussed in https://github.com/pypa/setuptools/pull/4537/files#r1701658826.
The approach seems to be the standard workaround for the same problem in setuptools-scm
, see pypa/setuptools-scm#480, pypa/setuptools-scm#952 (comment). Tracked in actions/checkout#249 and actions/checkout#1471.
setup.py
Outdated
except subprocess.CalledProcessError: # e.g.: git not installed or history missing | ||
with open("NEWS.rst", encoding="utf-8") as fp: | ||
match = re.search(r"v\d+\.\d+.\d+", fp.read()) # latest version | ||
return f"{match[0] if match else '0.0.0'}.dev+{time.strftime('%Y%m%d')}" |
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.
%Y%m%d
can be changed to more specific time to improve monotonicity... Please let me know if a more specific tag would be better.
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.
BTW, the except
is the code path that takes effect when git tag version is not available (tarballs or shallow clones).
I have considered checking if a PKG-INFO
file is available but that would increase the complexity of this function... Please let me know if this more complex approach should be followed...
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.
OK, it may be necessary to check for PKG-INFO
, because otherwise we may see inconsistency between sdist and wheel the following behaviour when running python -m build
:
$ python -m build
Successfully built setuptools-78.1.2.tar.gz and setuptools-78.1.2.dev0+20250416-py3-none-any.whl
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.
OK, it may be necessary to check for
PKG-INFO
, because otherwise we may see the following behaviour when runningpython -m build
:
So I inevitably had to add some lines in aee4dc9 to deal with this problem:
It does increase the complexity of the solution.
We can see now consistency between version inference in sdist and wheel: https://github.com/pypa/setuptools/actions/runs/14501313828/job/40681414420?pr=4948#step:8:3078
@@ -12,7 +12,7 @@ | |||
_VALID_NAME = re.compile(r"^([A-Z0-9]|[A-Z0-9][A-Z0-9._-]*[A-Z0-9])$", re.I) | |||
_UNSAFE_NAME_CHARS = re.compile(r"[^A-Z0-9._-]+", re.I) | |||
_NON_ALPHANUMERIC = re.compile(r"[^A-Z0-9]+", re.I) | |||
_PEP440_FALLBACK = re.compile(r"^v?(?P<safe>(?:[0-9]+!)?[0-9]+(?:\.[0-9]+)*)", re.I) | |||
_PEP440_FALLBACK = re.compile(r"^(?P<safe>v?(?:[0-9]+!)?[0-9]+(?:\.[0-9]+)*)", re.I) |
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.
This is an existing bug that I have identified when working on this PR.
I added a new doctest to avoid regression down in the same file:
>>> best_effort_version("v78.1.0-2-g3a3144f0d")
'78.1.0.dev0+sanitized.2.g3a3144f0d'
def best_effort_version(version: str) -> str: | ||
def best_effort_version( | ||
version: str, | ||
template: str = "{safe}.dev0+sanitized.{sanitized}", |
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.
This customisable template parameter is not strictly necessary, it can be removed if we don't mind versions looking like 78.1.0.dev0+sanitized.2.g3a3144f0d
@@ -102,8 +101,6 @@ setenv = | |||
TWINE_USERNAME = {env:TWINE_USERNAME:__token__} | |||
commands = | |||
python -c "import shutil; shutil.rmtree('dist', ignore_errors=True)" | |||
# unset tag_build and tag_date pypa/setuptools#2500 | |||
python setup.py egg_info -Db "" saveopts |
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.
I think we would no longer need this.
Possibly needs more tests, but posting to collect feedback if the direction is viable. |
return _normalization.best_effort_version(version, "{safe}.dev+{sanitized}") | ||
except subprocess.CalledProcessError: # e.g.: git not installed or history missing | ||
candidates = filter(None, starmap(_extract_version, _VERSION_FALLBACK)) | ||
return next(candidates, f"0.0.0.dev+{time.strftime('%Y%m%d')}") |
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.
Reposting from thread in https://github.com/pypa/setuptools/pull/4948/files#r2047494928:
The exact format of the version can be further discussed, e.g. %Y%m%d
can be changed to more specific time to improve monotonicity... Please let me know if a more specific tag would be better.
The except
is the code path that takes effect when git
is not installed and/or tag is not available (tarballs or shallow clones).
Reading PKG-INFO
ensures consistency between sdists/wheels:
Reading NEWS.rst
is a fallback when tags are missing from shallow clones (or someone is using a git tarball without sdist metadata).
I cannot see 0.0.0
being produced unless there is something wrong with the repo (e.g. missing NEWS.rst
file), but it is added to make static analysis happy.
Custom function results should be similar to setuptools-scm but without using setuptools-scm.
Should be no longer necessary now that `setup.cfg` is gone.
Summary of changes
This is a follow-up (and alternative approach to) #4537, #4532 and try to address some of the shortcomings identified.
The approach implemented is to call directly
git describe --abbrev --match 'v?[0-9]*' --dirty
falling back to read the latest version fromNEWS.rst
and adding a "dirty" local time-based version.Potential points of discussion:
What happens if a "downstream" packager simply download a tar ball from Github or performs a shallow clone without tag history?
In this PR, the version would be determined as follows:
NEWS.rst
So downstream packagers may see a "non-pristine"/"dirty" version string if they don't have access to the commit/tag history, which may be undesirable, but not prohibitive.
Are we reimplemented and "half-backing" functionality already present in
setuptools-scm
?Well, yes. But we take advantage that version tags in setuptools are very predictable to simplify the code and that we already have implemented some functionality that allow us to shoehorn malformed version strings into compliant ones.
This means that we can implement the functionality with a couple of lines1 of code assuming the installed version of git supports the command
git describe --abbrev --match 'v?[0-9]*' --dirty
(which unless I am wrong is available at least since version 2.0.5 of git: https://git-scm.com/docs/git-describe/2.0.5).On top of that, this PR implements a functionality not present on
setuptools-scm
(AFAIK), extract a fallback version fromNEWS.rst
. All things considered the custom function implementation is implemented with a couple of lines1 of code.Closes #4530
Pull Request Checklist
newsfragments/
.(See documentation for details)
Footnotes
The amount of lines necessary increased after consider fallback to
PKG-INFO
to ensure version consistency better sdist/wheel. ↩ ↩2