-
Notifications
You must be signed in to change notification settings - Fork 613
Add git SHA to wheel versions using versioningit #2070
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?
Add git SHA to wheel versions using versioningit #2070
Conversation
|
Hi @janbernloehr! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
4f29d71 to
eb2ddbf
Compare
tianyu-l
left a comment
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.
Thanks. Sorry that I'm not familiar with the stuff.
Could you share
- a comparison on what's the before & after behavior? Specifically what's the benefit?
- what's the full flow of doing a release after this change. Possibly also update https://github.com/pytorch/torchtitan/blob/main/docs/release.md?
torchtitan/_version.py
Outdated
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.
what's its relationship to assets/version.txt?
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.
Good point, that file would not be used anymore. As long as a git repo is available, the version will be derived from it. if we are on a tag, the tag is used, if not then the distance to the latest tag plus the short hash is used, e.g. 0.2.0.post77+gf5e3a84e (77 commits after v0.2.0 tag)
pyproject.toml
Outdated
| # Automatically generate version from git tags and commits at BUILD TIME | ||
| # Development builds: 0.2.0.post77+gf5e3a84e (77 commits after v0.2.0 tag) | ||
| # Tagged releases: 0.2.0 (when building from a git tag) | ||
| # To override version for releases: set SETUPTOOLS_SCM_PRETEND_VERSION=x.y.z |
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.
where and when do we set this?
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.
you only set this if you want to override the default behavior. the default should be fine since releases are done on a tag.
The before behavior is: if you build a wheel of torchtitan it will get the version of
done We can also choose a different mechanism to derive the version number. I chose
|
tianyu-l
left a comment
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.
another noob question
This makes it hard to distinguish development builds from release builds.
What would a scenario when people need development builds, e.g. is torchtitan nightly build a "development build"?
docs/release.md
Outdated
| ## Stable Releases | ||
| Currently we follow a lightweight release process. | ||
| - Update the version number in `assets/version.txt` with a PR. The version numbering should follow https://semver.org/. | ||
| - Update the version number in `torchtitan/_version.py` with a PR (ignore the DO NOT EDIT). The version numbering should follow https://semver.org/. |
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'm still a bit puzzled.
The after behavior is: Only when building on the commit of a tag, we get the version from the tag, e.g. v0.2.0. In all other cases, the version is derived by setuptools
There are several places one can edit a version.
- the file
torchtitan/_version.py - create the tag with a version (e.g. v0.2.0) when you do https://github.com/pytorch/torchtitan/releases/new
- In
pyproject.toml, you mentioned "To override version for releases: set SETUPTOOLS_SCM_PRETEND_VERSION=x.y.z"
In the past, IIUC the version in tag and the version in version.txt are (somewhat?) independent, so we need to manually keep track.
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.
The file _version.py will be generated by setuptools_scm - so its content does not really matter. However, if git is not available, then it falls back to the version defined in this file.
So you can control the version either by creating a tag - or you overwrite it by exporting and envvar SETUPTOOLS_SCM_PRETEND_VERSION=x.y
In other cases, setuptools_scm will generate the version.
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.
You are right that is too confusing. Let us use versioningit instead of setuptools_scm which is significantly simpler:
- On a tag
v0.2.0you get0.2.0 - On a commit after
v0.2.0you get0.2.0+gitb425c6f
Similar to other torch libraries. Moreover, in CI there is .github/scripts/update_version.sh which is called with BUILD_VERSION set externally. I made sure that this still works and uses the externally provided version.
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.
@janbernloehr
change LGTM, glad that I pushed :)
How do we verify this works? Would you be able to demonstrate this in the fork? i.e. create a new release and observe automatically updated tag?
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.
What I did to test it was
Run uv build --wheel . on the branch as is:
Successfully built dist/torchtitan-0.2.0+gita53b69fc-py3-none-any.whl
Create new tag v0.3.0 and run uv build --wheel .
Successfully built dist/torchtitan-0.3.0-py3-none-any.whl
Run BUILD_VERSION=0.1.0.dev20251022+cpu bash .github/scripts/update_version.sh
Successfully built dist/torchtitan-0.1.0.dev20250921+cpu-py3-none-any.whl
I have also created a "fake" release v0.3.0 on my fork: https://github.com/janbernloehr/torchtitan/actions/runs/19601607282/job/56133915316#step:4:1005
Successfully built torchtitan-0.3.0.tar.gz and torchtitan-0.3.0-py3-none-any.whl
99f458d to
e117a61
Compare
Signed-off-by: jbernloehr <[email protected]>
e117a61 to
a53b69f
Compare
tianyu-l
left a comment
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.
LGTM, will let @joecummings take a final look before merge.
|
Thanks for the PR. I feel we can remove |
so context is that update_version.sh is for producing nightly. So not sure if / how it can be saved. @joecummings who added the file may know how. |
|
If one install TorchTitan package correctly, the version should be in metadata. And since this PR make version to be dynamic, it will automatically reflect the latest version with git SHA. |
|
Removing the version.txt file will break any builds intending to use the PyTorch Dev Infra build system and since this isn't urgent, I'd like to not merge quite yet. I know there's been changes to how Dev Infra wants to continue these workflows so let me get @seemethere's thoughts as well. Will update on this thread by EOD today. |
Stack from [ghstack](https://github.com/ezyang/ghstack/tree/0.12.0) (oldest at bottom): * __->__ #2083 This PR and #2070 can resolve #2043. This should not affect `.github/scripts/update_version.sh` as `.github/scripts/update_version.sh` will append the version at the end of the file, which will overwrite the value.
@atalman It looks like we could pass in an override to the nightly package here. Am I understanding correctly? If so, does it make sense to bubble up to the top level build script? I'm also curious in general if continuing to hack on these Nova workflows is the right way to go to do these nightly builds? |
Adds automatic git SHA suffixing to wheel versions during development builds, matching the behavior of other PyTorch libraries (e.g., pytorch/ao).
Changes:
versioningitfor automatic version generation from git tags0.2.0+gitf5e3a84e0.2.0For releases: Versions are automatically determined from git tags (e.g.,
v0.2.0).Build system: Adds
versioningit>=3.3to build requirements. No runtime dependencies added.