Skip to content

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

Merged
merged 1 commit into from
Jul 14, 2019

Conversation

pietroalbini
Copy link
Member

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 10, 2019
@pietroalbini
Copy link
Member Author

Do not merge this yet, I want to push test commits that change submodules to make sure all of this works.

@pietroalbini pietroalbini force-pushed the tools-builders-on-prs branch from cc1fcbf to 0182f82 Compare July 10, 2019 14:08
The builder will skip time consuming tasks (like the actual tests) when
it detects no updated submodules.
@pietroalbini pietroalbini force-pushed the tools-builders-on-prs branch 2 times, most recently from 105c6b0 to 2d2dcb0 Compare July 10, 2019 14:23
@pietroalbini
Copy link
Member Author

Ok this works fine and it's ready for a review.

Commit with submodule update - Commit without submodule updates (41 seconds)

@alexcrichton
Copy link
Member

@bors: r+

Nice!

@bors
Copy link
Collaborator

bors commented Jul 10, 2019

📌 Commit 2d2dcb0 has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 10, 2019
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
Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member

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...

@bors
Copy link
Collaborator

bors commented Jul 12, 2019

⌛ Testing commit 2d2dcb0 with merge 307e87a...

bors added a commit that referenced this pull request Jul 12, 2019
…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
@bors
Copy link
Collaborator

bors commented Jul 12, 2019

💔 Test failed - checks-azure

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 12, 2019
@pietroalbini
Copy link
Member Author

@bors retry

Spurious failure.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 13, 2019
@Mark-Simulacrum
Copy link
Member

@bors p=3

@bors
Copy link
Collaborator

bors commented Jul 14, 2019

⌛ Testing commit 2d2dcb0 with merge fa6b706...

bors added a commit that referenced this pull request Jul 14, 2019
…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
@bors
Copy link
Collaborator

bors commented Jul 14, 2019

☀️ Test successful - checks-azure, checks-travis, status-appveyor
Approved by: alexcrichton
Pushing fa6b706 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 14, 2019
@bors bors merged commit 2d2dcb0 into rust-lang:master Jul 14, 2019
@pietroalbini pietroalbini deleted the tools-builders-on-prs branch July 14, 2019 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

azure: Run tool builds whenever a tool submodule is updated
6 participants