-
Notifications
You must be signed in to change notification settings - Fork 423
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
chore: CI creates lean-pr-testing-NNNN branches at Std too #3200
Conversation
Mathlib CI status (docs):
|
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.
LGTM. I wonder if the “"This shouldn't be possible”
code path should simply error: We expect the tag to exist, so it can just fail and crash if it doesn’t
Co-authored-by: Joachim Breitner <mail@joachim-breitner.de>
git checkout "$BASE" | ||
|
||
EXISTS="$(git ls-remote --heads origin lean-pr-testing-${{ steps.workflow-info.outputs.pullRequestNumber }} | wc -l)" | ||
echo "Branch exists: $EXISTS" | ||
if [ "$EXISTS" = "0" ]; then | ||
echo "Branch does not exist, creating it." | ||
git checkout -b lean-pr-testing-${{ steps.workflow-info.outputs.pullRequestNumber }} | ||
git switch -c lean-pr-testing-${{ steps.workflow-info.outputs.pullRequestNumber }} "$BASE" |
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.
Hmm turns out
git switch -c foo base
works if base
is a local tag, but not if it is a (remote) branch.
The old code (git checkout base
) would work for a (remote) branch, but not for a tag. Kinda annoying.
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.
Maybe we need to parse this output to go onto a common path in both cases
$ git ls-remote --heads --tags --exit-code https://github.com/leanprover-community/mathlib4 nightly-testing-2024-01-22
7d803278d1b5978fccc14f992d9ffb692077ed93 refs/heads/nightly-testing-2024-01-22
$ git ls-remote --heads --tags --exit-code https://github.com/leanprover/std4 nightly-testing-2024-01-22
c15b5a8f04e84bc7aab8b1062a4a80de3046865f refs/tags/nightly-testing-2024-01-22
Alternatively, we can once manually convert all nightly-testing-YYYY-MM-DD
branches to tags on mathlib4. Maybe easier.
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.
Possible fix in #3208
Currently we create
lean-pr-testing-NNNN
branches at Mathlib automatically for each Lean PR.We don't automatically create one at Std; mostly simply because Std fails less often, so it has been okay to do this manually as needed. It is conceptually simpler, however, if this is done uniformly.
This PR:
nightly-testing-YYYY-MM-DD
tag exists at Std (like it already doesn't proceed if that tag is missing at Mathlib)lean-pr-testing-NNNN
branches at Stdlean-pr-testing-NNNN
branches at Mathlib, updates the Std dependency to use thelean-pr-testing-NNNN
branch at StdNote that because most users do not have write access at Std, in order to make updates to
lean-pr-testing-NNNN
branches there they will need to make PRs. These will be merged with a very low bar, and feel free to ping me for assistance on this. If this is annoying we will automate. Also, frequent contributors to Lean may ask @digama0 or @joehendrix for write access in order to easily work on these branches.This PR requires that we have a secret here with write access at Std. I'm arranging that on zulip.
I will update the documentation at https://leanprover-community.github.io/contribute/tags_and_branches.html to reflect these changes when they are merged.