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

Unconditionally use the GitHub workflow run ID as build ID. #29

Merged
merged 1 commit into from
Jan 2, 2024

Conversation

nat-goodspeed
Copy link
Contributor

AUTOBUILD_BUILD_ID is captured in the package tarball's autobuild-package.xml as build_id. No other information in autobuild-package.xml permits uniquely identifying the build that constructed the package in question. The changeset hash could reference any of a number of builds of the same changeset.

Without knowing the specific build, we don't even know the date the package was built.

AUTOBUILD_BUILD_ID is captured in the package tarball's autobuild-package.xml
as build_id. No other information in autobuild-package.xml permits uniquely
identifying the build that constructed the package in question. The changeset
hash could reference any of a number of builds of the same changeset.

Without knowing the specific build, we don't even know the date the package
was built.
Copy link
Member

@bennettgoble bennettgoble left a comment

Choose a reason for hiding this comment

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

Hey, Nat. This change would limit the flexibility of the action. It's more common to use the commit ID (commit SHA) as part of pre-release package versions, normally something along the lines of MAJOR.MINOR.PATCH+devSHA, ex. 1.0.0-dev+52b7714a or 2.0.1-nightly+52b7714a or the like)

Using the git sha instead of the run ID works because it is assumed that actions do not promote artifacts unless the build was completely successful.

I'm not in favor of limiting this action to using the workflow ID as the build ID as it would inhibit our movement to more modern versioning schemes or, at a minimum, preclude versioning schemes that do not use workflow ID.

Copy link
Member

@bennettgoble bennettgoble left a comment

Choose a reason for hiding this comment

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

I punched up some reasons this should not be done, but the real problem is autobuild's reliance on build ID at all. Ideally it should be possible to push a version tag such as v1.0.0 and have autobuild output an artifact with the filename foo-v1.0.0.tar.zst Build ID is valid metadata, but it shouldn't be baked into the filename --that's ornithological to this PR though, so carry on...!

@nat-goodspeed
Copy link
Contributor Author

Thanks. Appending AUTOBUILD_BUILD_ID to the version is done by individual build-cmd.sh scripts, e.g. curl's. So we could migrate away from that convention if we wanted.

Moreover, that's using the VERSION.txt version-file mechanism. I know you want to migrate away from that entirely; perhaps we will.

For me the point of setting AUTOBUILD_BUILD_ID to the run ID isn't to affect the version string of the package. It's to bake the run ID into the autobuild-package.xml file found in every autobuild package tarball. Since autobuild rolls up individual autobuild-package.xml metadata into consuming packages, ultimately the viewer's autobuild-package.xml file contains all the metadata for every package built into that viewer -- but only the metadata captured by autobuild.

I want the source validation script to be able to verify that the run that built a given package was the most recent release of the project branch. It's happened that somebody has updated the branch and produced a new package build, without actually updating the viewer with that new package URL. Given the run ID, we can validate that. Without it, we can't.

@nat-goodspeed nat-goodspeed merged commit f20722d into main Jan 2, 2024
3 checks passed
@nat-goodspeed nat-goodspeed deleted the build-id branch January 2, 2024 15:41
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.

2 participants