Skip to content
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 #19034

Merged
merged 13 commits into from
Nov 7, 2024
Merged

Apply new versioning scheme #19034

merged 13 commits into from
Nov 7, 2024

Conversation

marbre
Copy link
Member

@marbre marbre commented Nov 5, 2024

This implements a new versioning scheme, similar to the one recently proposed with #18938 and applies it to iree-compiler, iree-runtime and the iree-dist tarball. The TF and TFLite compiler tools use the (legacy) versioning and need to be updated with a follow up commit. The common version number is determined based on the versions of the compiler and the runtime and the higher version is picked as the common on. In contrast to the proposed versioning scheme, version numbers of development releases are in the format X.Y.Z.dev+${gitcommithash} as discussed in #19003 and not X.Y.Z.dev as originally proposed.

This implements a new versioning scheme, similar to the one recently
proposed with iree-org#18938 and applies it to `iree-compiler`, `iree-runtime`
and the `iree-dist` tarball. The TF and TFLite compiler tools use the
(legacy) versioning and need to be updated with a follow up commit. The
common version number is determined based on the versions of the
compiler and the runtime and the higher version is picked as the common
on. In contrast to the proposed versioning scheme, version numbers of
development releases are in the format `X.Y.Z.dev0+${gitcommithash}` as
discussed in iree-org#19003 and not `X.Y.Z.dev` as originally proposed.

Not yet addressed is the refactoring of the workflows thus that the
`version-local.json` files get only written once (and not for every OS)
and to provide then to subsequent steps as artifacts.
@marbre
Copy link
Member Author

marbre commented Nov 5, 2024

I tested builds locally (development releases) and triggered the Oneshot candidate release on a branch which on top applies marbre/iree@0de8adb (limiting what is build). The release was published to https://github.com/marbre/iree/releases.

Copy link
Member

@ScottTodd ScottTodd left a comment

Choose a reason for hiding this comment

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

The overall structure of this LGTM. Some comments on the mechanics and style.

.gitignore Outdated Show resolved Hide resolved
build_tools/scripts/compute_common_version.py Outdated Show resolved Hide resolved
build_tools/scripts/compute_common_version.py Outdated Show resolved Hide resolved
build_tools/scripts/compute_common_version.py Outdated Show resolved Hide resolved
.github/workflows/pkgci_build_packages.yml Show resolved Hide resolved
@ScottTodd ScottTodd added infrastructure Relating to build systems, CI, or testing bindings/python Python wrapping IREE's C API labels Nov 6, 2024
Comment on lines 41 to 45
jobs:
# Note: metadata generation could happen in a separate trigger/schedule
# workflow. For cross platform builds, it's useful to just generate the
# metadata on Linux and pass that to later jobs using artifacts.
setup_metadata:
Copy link
Member

Choose a reason for hiding this comment

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

Oh, I think I meant for this to go in a workflow like schedule_candidate_release.yml as I was writing this comment for https://github.com/nod-ai/SHARK-Platform. This repository already has that workflow, and it could produce this metadata alongside the other workflow inputs (package_version, compiler_package_version, etc.). Fine to keep this here as a job in build_package.yml, but may want to clarify the comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was considering this as well and it is the logical next step. I iterate on it if I find some time for it tomorrow. Sending a patch to update the comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated the comment accordingly. Depending on when you want to merge this, I can also fill an issue to address moving this to in a follow up.

Copy link
Member

Choose a reason for hiding this comment

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

I think we could merge it soon, though we may want to also update the package names to iree-base-compiler and iree-base-runtime alongside the new versions. If we just land this as-is, users won't automatically get the new versions from nightly or stable releases from pip.

Maybe having this PR just roll out the versions and then a follow-up change the names is fine.

compiler/version.json Outdated Show resolved Hide resolved
@marbre marbre enabled auto-merge (squash) November 7, 2024 19:54
@marbre marbre merged commit 6d55a11 into iree-org:main Nov 7, 2024
36 checks passed
@marbre marbre deleted the versioning-scheme-1 branch November 8, 2024 09:16
marbre added a commit to marbre/iree that referenced this pull request Nov 12, 2024
This documents the versioning scheme proposed with iree-org#18938, refined as
part of the discussion iree-org#19003 (first drafted patch) and iree-org#19034 (second
drafted patch) and applied with commit 6d55a11.
marbre added a commit to marbre/iree that referenced this pull request Nov 12, 2024
This documents the versioning scheme proposed with iree-org#18938, refined as
part of the discussion iree-org#19003 (first drafted patch) and iree-org#19034 (second
drafted patch) and applied with commit 6d55a11.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bindings/python Python wrapping IREE's C API infrastructure Relating to build systems, CI, or testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants