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

[CI] Use src/VERSION for maintenance build on release branch #11522

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

straight-shoota
Copy link
Member

In a release branch, maintenance builds should pull the version from src/VERSION instead of the branch name. That allows us to build package increments from a release branch. A use case example would be crystal-lang/distribution-scripts#170: It changes the distribution package, but not the actual Crystal release. We would like to update and replace the 1.2.2 releases with this package increment.
The changes are pulled into the release branch in #11515. This also triggers a maintenance build, but it's tagged as a dev build. With this patch, we can directly publish the build artifacts from that maintenance build because they're properly tagged as 1.2.2.

I pushed the same commit to two different branches to show the difference:

The workflows for tagged and maintenance releases are identical except for the prepare step. So the build should be functionally identical, whether it's from a tagged commit or a non-tagged commit in a release branch.

Since the version is identical to the last tagged release, the workflow will automatically publish docker images with the respective tag. I'm not sure if we should keep it that way or use different tags for the docker images and require manual promotion to avoid accidental overrides.

Comment on lines +222 to +225
# We need to sanitize it because there are restrictions on some places
# where the version is use (Mac pkg names, snap branch).
VERSION=${VERSION//_/-}
VERSION=${VERSION//\//-}-dev
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be applied unconditionally I guess (to be on the safe side).

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 figure if src/VERSION has underscores or slashes, we're in more trouble anyways. So this isn't technically necessary.

@bcardiff
Copy link
Member

bcardiff commented Dec 1, 2021

What would be the process to incremente the package iteration? I was expecting to see a variable or a new file for that.

Would this work if two new package iteration are wanted for the same minor version (but different patch version)? I guess yes as long as each minor has its release branch when needed. Currently is not the case.

I thought we had a check to use version numbers only on tagged releases. But is the other way around: tagged commits must match version file (bin/ci takes care of this). We are good.

I'm not sure if snap will like two packages with the same version, but lets see how it goes. Worst case there is a need to manually yank the previous release.

@straight-shoota
Copy link
Member Author

Yeah we currently don't have any way to express package iterations. I don't think an indication is commonly used for docker images. But there you have at an easy means to identify different images by their hash id.

Would this work if two new package iteration are wanted for the same minor version (but different patch version)? I guess yes as long as each minor has its release branch when needed. Currently is not the case.

I suppose we could do that. But I'm not sure it makes much sense. In my opinion, after a patch is released, all prior patch releases of the x.y branch are outdated. I don't think it makes sense to support more than one patch release for any minor release branch simultaneously.

I'm not sure if snap will like two packages with the same version, but lets see how it goes. Worst case there is a need to manually yank the previous release.

I have no idea how this works out for snap. I'm not familiar with that at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants