-
Notifications
You must be signed in to change notification settings - Fork 31
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
Apply new versioning scheme #398
Conversation
a9425b4
to
9b073c0
Compare
Progress on nod-ai#130 and nod-ai#294. With this, every package has its own `version_info.json` which defaults to a dev build (`X.Y.Z.dev`). Based on this version information the version identifier for a release candate can be computed (`X.Y.ZrcYYYYMMDD`) which is written to corresponding `version_info_rc.json` files. This PR further adapt the build packages workflow to make use of the script and apply the new versioning scheme. Even though sharktank packages are not yet build, the workflow already generates the rc version numbers for sharttank and shortfin.
9b073c0
to
8a6fc4a
Compare
.github/workflows/build_packages.yml
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.
Did you test this workflow? Should be possible with workflow_dispatch
over at https://github.com/marbre/SHARK-Platform/actions/workflows/build_packages.yml
... which reminds me, we may want to disable it in forks somehow. I tested at https://github.com/ScottTodd/SHARK-Platform/actions/workflows/build_packages.yml, generating these: https://github.com/ScottTodd/SHARK-Platform/releases/tag/dev-wheels
I see that several forks are running the action too, which is failing because of the missing token: https://github.com/nithinsubbiah/sharktank/actions/workflows/build_packages.yml but some runs did succeed at one point: https://github.com/nithinsubbiah/sharktank/releases/tag/dev-wheels
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.
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.
Looks promising :)
https://github.com/marbre/SHARK-Platform/actions/runs/11615896057/job/32347586648#step:4:1434
******************** BUILD COMPLETE ********************
Generated binaries:
total 2512
-rw-r--r-- 1 root root 2568751 Oct 31 16:31 shortfin-3.0.0rc20241031-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
try: | ||
version_info = load_version_info(VERSION_INFO_RC_FILE) | ||
except FileNotFoundError: | ||
print("version_info_rc.json not found. Default to dev build") | ||
version_info = load_version_info(VERSION_INFO_FILE) |
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.
Oh, interesting choice to have two different files. Did you consider other approaches? I can think of a few:
- Overwrite the file (makes git index dirty for development though)
- Have
setup.py
load fromversion_info.json
and do thePACKAGE_RC_VERSION = PACKAGE_BASE_VERSION + "rc" + datetime.today().strftime("%Y%m%d")
here
Multiple ways to do it, and what you have works.
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 there are even more:
- Read the version from a
version_info
(text) and override with the updated one.- Use the version info file in the
pyproject.toml
making thesetup.py
obsolete:
version = {file = version_info}
- Drawback: Git gets dirty, limited flexibility.
- Use the version info file in the
With regards to the ones you listed:
- I would like to avoid that the index gets dirty.
- This introduces a dependency on
packaging
which isn't strictly needed. Furthermore, with this every builder calculates the version instead of computing this once in the CI.
But I am happy to iterate in a follow up but this one should also be compatible with what we will need for IREE.
Progress on #130 and #294.
With this, every package has its own
version_info.json
which defaults to a dev build (X.Y.Z.dev
). Based on this version information the version identifier for a release candate can be computed (X.Y.ZrcYYYYMMDD
) which is written to correspondingversion_info_rc.json
files. This PR further adapt the build packages workflow to make use of the script and apply the new versioning scheme. Even though sharktank packages are not yet build, the workflow already generates the rc version numbers for sharktank and shortfin.