-
Notifications
You must be signed in to change notification settings - Fork 13.3k
ci: add a pr builder to test tools when submodules are updated #62560
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
Conversation
Do not merge this yet, I want to push test commits that change submodules to make sure all of this works. |
cc1fcbf
to
0182f82
Compare
The builder will skip time consuming tasks (like the actual tests) when it detects no updated submodules.
105c6b0
to
2d2dcb0
Compare
Ok this works fine and it's ready for a review. Commit with submodule update - Commit without submodule updates (41 seconds) |
@bors: r+ Nice! |
📌 Commit 2d2dcb0 has been approved by |
set -e | ||
# Submodules pseudo-files inside git have the 160000 permissions, so when | ||
# those files are present in the diff a submodule was updated. | ||
if git diff HEAD^ | grep "^index .* 160000" >/dev/null 2>&1; then |
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.
This looks like it only tests the last commit of the PR as opposed to all of them for changing submodules?
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.
This checks the merge commit, so any submodule change should be detected.
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.
Ah, so HEAD^
will be the parent that's the old master, not the other parent. Makes sense.
I never know which side HEAD^
picks with merge commits...
…hton ci: add a pr builder to test tools when submodules are updated This PR adds the x86_64-gnu-tools builders to PRs where submodules are updated. Since it's not possible to *start* the builder only when submodule changes are detected, I opted into adding a "decider" task at the start of the job which sets the `SKIP_JOB` environment variable when submodules are not updated, and I gated the most time-consuming tasks (the actual build and artifacts upload) on the variable not being there. All of this is conditionally included in the `steps/run.yml` only when a template parameter is present, so it should only affect that builder on PRs. The cost for this should be a dummy builder running for 2/3 minutes for each PR, and we should be able to handle it. Fixes #61837 r? @alexcrichton
💔 Test failed - checks-azure |
@bors retry Spurious failure. |
@bors p=3 |
…hton ci: add a pr builder to test tools when submodules are updated This PR adds the x86_64-gnu-tools builders to PRs where submodules are updated. Since it's not possible to *start* the builder only when submodule changes are detected, I opted into adding a "decider" task at the start of the job which sets the `SKIP_JOB` environment variable when submodules are not updated, and I gated the most time-consuming tasks (the actual build and artifacts upload) on the variable not being there. All of this is conditionally included in the `steps/run.yml` only when a template parameter is present, so it should only affect that builder on PRs. The cost for this should be a dummy builder running for 2/3 minutes for each PR, and we should be able to handle it. Fixes #61837 r? @alexcrichton
☀️ Test successful - checks-azure, checks-travis, status-appveyor |
This PR adds the x86_64-gnu-tools builders to PRs where submodules are updated.
Since it's not possible to start the builder only when submodule changes are detected, I opted into adding a "decider" task at the start of the job which sets the
SKIP_JOB
environment variable when submodules are not updated, and I gated the most time-consuming tasks (the actual build and artifacts upload) on the variable not being there. All of this is conditionally included in thesteps/run.yml
only when a template parameter is present, so it should only affect that builder on PRs.The cost for this should be a dummy builder running for 2/3 minutes for each PR, and we should be able to handle it.
Fixes #61837
r? @alexcrichton