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

Bump patch versions automatically on master #2870

Merged
merged 40 commits into from
Sep 13, 2022
Merged

Bump patch versions automatically on master #2870

merged 40 commits into from
Sep 13, 2022

Conversation

ehildenb
Copy link
Member

@ehildenb ehildenb commented Sep 10, 2022

Fixes: #1928

Part of: #2869

  • Bump patch versions automatically on PRs. It's set up to have the following logic (to allow manually setting version, and automatically bumping it):
    • If the major and minor version identifiers are different than the merge base (eg. bump from 5.3.X => 5.4.0), do not bump the patch number.
    • If the major and minor version identifiers are the same as the merge base, then automatically bump the patch number to be one more than the merge base patch number.
    • Do version string substitution through the repository.
    • If there are any changes in the repo, push them to the current branch.
  • Set up concurrency groups on all github workflows, so that old jobs are appropriately cancelled (otherwise we run into GitHub public runner limit of 15 concurrent at a time pretty quickly).
  • Removes the push: ... specifier for update.yml job. Should this remain @goodlyrottenapple? It seems unnecessary.

TODO:

  • Enable this new status check as required on CI.

@ehildenb
Copy link
Member Author

It's working here
image

@ehildenb ehildenb mentioned this pull request Sep 10, 2022
14 tasks
@ehildenb
Copy link
Member Author

Note that I had to make this change to our organization secret:
image

I think it should be OK, because secrets are still not shared with PRs run from forks.

@ehildenb
Copy link
Member Author

Appears to be working properly:
image

@ehildenb
Copy link
Member Author

ehildenb commented Sep 12, 2022

Your understanding is not quite correct. CI will do this only the minimum number of times needed (not every time you push). That will be:

  • If master branch is updated, and then this branch is pushed to, CI will now bump the version here to the next version after master branch. You will have to git pull.
  • When you first open the PR, CI will automatically push the new version to your branch (within 30s).

It's always safe to resolve the merge conflict with git checkout --ours -- package/version or git checkout --theirs -- package/version, CI will always make sure the version identifier is set correctly afterwards either way. Also, it should always be possible to do git fetch origin ; git rebase origin/MYBRANCH with no merge conflicts, which is a pretty painless process.

The wins are that we have an accurate version number no matter what, and a simplified release process, and better ability to do semantic versioning.

@radumereuta
Copy link
Contributor

radumereuta commented Sep 12, 2022

Are you sure you still want to update to 5.4.0?
It will make the changelog a bit weird.

@ehildenb
Copy link
Member Author

It's the easiest way to make sure we have version number continuity. I could also choose 5.3.202, if we know this one will be merged next.

@ehildenb
Copy link
Member Author

But already last night when I was trying, it would have been 5.3.199, so who knows?

@radumereuta
Copy link
Contributor

You're right. Leave it as v5.4. I will add a note in the changelog.

@radumereuta
Copy link
Contributor

Are you sure we're still running all workflows we intend?
I'm counting 10 GH workflows + 1 Jenkins before
and 4 GH workflows + 1 Jenkins on this branch.

@ehildenb
Copy link
Member Author

ehildenb commented Sep 12, 2022

Where did you get 10 GH workflows? You can see in the PR that I didn't change any of the other workflows other than adding the concurrency throttle.

@F-WRunTime
Copy link
Member

I'm just counting 4 GH workflows and 1 Jenkins

@radumereuta
Copy link
Contributor

Look at other PRs. #2873 which doesn't have these changes has 8+1 (sorry, not 10)
image

@ehildenb
Copy link
Member Author

I see 2 there triggered by push events, which this PR took care of.

@ehildenb
Copy link
Member Author

I wonder why these checks are not required. I think I've identified the problems though:

Tests run on the other PR:

Test / Nix (ubuntu-latest) (pull_request) Successful in 8m
Update / Nix: Maven (pull_request) Successful in 7m
Test / Nix (macos-11) (pull_request) Successful in 20m
Update / Nix flake sumbodule sync (pull_request) Successful in 52s
continuous-integration/jenkins/pr-head — This commit looks good
 
Update / Nix: Maven (push) Successful in 9m
Update / Nix flake sumbodule sync (push) Successful in 43s


Test / Nix flake (ubuntu-latest) (pull_request) Successful in 5m
Test / Nix flake (macos-11) (pull_request) Successful in 11m

Tests run on this PR (before last commit):

Test / Nix (ubuntu-latest) (pull_request) Successful in 14m
Update / Nix: Maven (pull_request) Successful in 7m
Test / Nix (macos-11) (pull_request) Successful in 20m
Update / Nix flake sumbodule sync (pull_request) Successful in 42s
continuous-integration/jenkins/pr-head — This commit looks good

The tests in the first block on the other PR are run here. The tests in the second block are because I disabled push event handling on this PR for Nix: Maven and Nix: submodule sync. For some reason we were running this job doubly for pushes and pull-request events. The third block I suspect is because there was a stage name clash, and github's interface is not handling it correctly. Indeed, the output now contains all 7 entries (6 github runners, and 1 Jenkins):

Test / Nix (ubuntu-latest) (pull_request) In progress — This check has started...
Test Nix Flake / Nix flake (ubuntu-latest) (pull_request) In progress — This check has started...
Update / Nix: Maven (pull_request) In progress — This check has started...
Test / Nix (macos-11) (pull_request) In progress — This check has started...
Test Nix Flake / Nix flake (macos-11) (pull_request) In progress — This check has started...
Update / Nix flake sumbodule sync (pull_request) Successful in 42s
continuous-integration/jenkins/pr-head Pending — This commit is being built

@ehildenb
Copy link
Member Author

We generally expect every test to pass here right? If so, we should enable this list of tests as required.

@ehildenb ehildenb changed the title Bump patch versions automatically on PRs Bump patch versions automatically on master Sep 12, 2022
@radumereuta
Copy link
Contributor

We generally expect every test to pass here right? If so, we should enable this list of tests as required.

We might need to doublecheck with @goodlyrottenapple on this.

@@ -1,9 +1,10 @@
name: 'Update'
on:
pull_request:
push:
branches-ignore:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this changed?

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'm not sure why it was like this in the first place, but basically it resulted in us running CI double on each pull request, once for the branch trigger and once for the pull_request trigger. @goodlyrottenapple may know why we were doing this before, I suspect it was so that someone could debug this workflow in the past without opening a PR, but I don't know.

Either way, should not need to run it twice for every branch.

@ehildenb
Copy link
Member Author

I'm going to merge and then require these extra status checks.

@rv-jenkins rv-jenkins merged commit 7763a99 into master Sep 13, 2022
@rv-jenkins rv-jenkins deleted the bump-patch branch September 13, 2022 04:17
ehildenb added a commit that referenced this pull request Sep 13, 2022
h0nzZik pushed a commit to h0nzZik/k that referenced this pull request Nov 24, 2022
…untimeverification#2263)

* haskell-backend/src/main/native/haskell-backend: e0fedde3 - kore-0.55.0.0 (runtimeverification#2873)

* haskell-backend/src/main/native/haskell-backend: 689b15b8 - Update to latest NixOS (runtimeverification#2870)
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.

In-submodule builds of K report incorrect patch version number
5 participants