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

Improve GitHub Actions CI config #13317

Merged
merged 2 commits into from
Jan 18, 2024
Merged

Improve GitHub Actions CI config #13317

merged 2 commits into from
Jan 18, 2024

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Jan 18, 2024

This pull request improves Cargo CI by:

  1. Explicitly listing the branches the main CI should be run on (to avoid running on unwanted branches, particularly in forks)
  2. By adding a concurrency config so that only one "CI" runs for each "group" (one by PR), very inspired by rust-lang/rust one
  3. By making all the jobs run only if triggered from this repository (to avoid having forks unnecessarily run CI jobs)
  4. By making all the checkouts take fetch-depth: 2 like done in rust-lang/rust (to avoid unnecessary work, saves bandwidth and maybe a bit of time)
  5. By including refs/heads/try in the success and failure bors job gates (to fix bors try from timing out)

The main motivation for this PR was (at first) to make the CI jobs not run on forks since it's a waste of CI resources as well as always failing (like this); but after looking at the main.yml config I took the opportunity to improve the situation by using rust-lang/rust ci.yml as reference.

@rustbot
Copy link
Collaborator

rustbot commented Jan 18, 2024

r? @epage

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 18, 2024
Copy link
Contributor

@epage epage left a comment

Choose a reason for hiding this comment

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

Thanks!

.github/workflows/main.yml Outdated Show resolved Hide resolved
.github/workflows/main.yml Show resolved Hide resolved
.github/workflows/main.yml Outdated Show resolved Hide resolved
.github/workflows/main.yml Outdated Show resolved Hide resolved
.github/workflows/main.yml Outdated Show resolved Hide resolved
@weihanglo
Copy link
Member

Regardless, let's see if it works with bors

@bors try

bors added a commit that referenced this pull request Jan 18, 2024
Improve GitHub Actions CI config

This pull request improves Cargo CI by:
 1. Explicitly listing the branches the main CI should be run on (to avoid running on unwanted branches, particularly in forks)
 2. By adding a [concurrency](https://docs.github.com/en/actions/using-jobs/using-concurrency) config so that only one "CI" runs for each "group" (one by PR), very inspired by rust-lang/rust [one](https://github.com/rust-lang/rust/blob/c485ee71477a29041895c47cc441b364670f3772/.github/workflows/ci.yml#L35)
 3. By making all the jobs run only if triggered from this repository (to avoid having forks unnecessarily run CI jobs)
 4. By making all the checkouts take `fetch-depth: 2` like done in [rust-lang/rust](https://github.com/rust-lang/rust/blob/c485ee71477a29041895c47cc441b364670f3772/.github/workflows/ci.yml#L452-L455) (to avoid unnecessary work, saves bandwidth and maybe a bit of time)

The main motivation for this PR was (at first) to make the CI jobs not run on forks since it's a waste of CI resources as well as always failing (like [this](https://github.com/Urgau/cargo/actions/runs/7567435677/job/20606586124)); but after looking at the `main.yml` config I took the opportunity to improve the situation by using rust-lang/rust [ci.yml](https://github.com/rust-lang/rust/blob/master/.github/workflows/ci.yml) as reference.
@bors
Copy link
Contributor

bors commented Jan 18, 2024

⌛ Trying commit 355e88a with merge 9ffa6c3...

@Urgau Urgau force-pushed the improve-ci branch 2 times, most recently from c60488f to e30dac4 Compare January 18, 2024 15:11
@Urgau
Copy link
Member Author

Urgau commented Jan 18, 2024

Hum, I have been informed by @weihanglo that apparently bors try kinda works but always timeout.

I believe this is due to an overly constrained condition in the success and failure bors gates. I've made some changes that I think will solve this. Let's try?

@weihanglo
Copy link
Member

Sure. Do it again.

@bors try

bors added a commit that referenced this pull request Jan 18, 2024
Improve GitHub Actions CI config

This pull request improves Cargo CI by:
 1. Explicitly listing the branches the main CI should be run on (to avoid running on unwanted branches, particularly in forks)
 2. By adding a [concurrency](https://docs.github.com/en/actions/using-jobs/using-concurrency) config so that only one "CI" runs for each "group" (one by PR), very inspired by rust-lang/rust [one](https://github.com/rust-lang/rust/blob/c485ee71477a29041895c47cc441b364670f3772/.github/workflows/ci.yml#L35)
 3. ~~By making all the jobs run only if triggered from this repository (to avoid having forks unnecessarily run CI jobs)~~
 4. ~~By making all the checkouts take `fetch-depth: 2` like done in [rust-lang/rust](https://github.com/rust-lang/rust/blob/c485ee71477a29041895c47cc441b364670f3772/.github/workflows/ci.yml#L452-L455) (to avoid unnecessary work, saves bandwidth and maybe a bit of time)~~

The main motivation for this PR was (at first) to make the CI jobs not run on forks since it's a waste of CI resources as well as always failing (like [this](https://github.com/Urgau/cargo/actions/runs/7567435677/job/20606586124)); but after looking at the `main.yml` config I took the opportunity to improve the situation by using rust-lang/rust [ci.yml](https://github.com/rust-lang/rust/blob/master/.github/workflows/ci.yml) as reference.
@bors
Copy link
Contributor

bors commented Jan 18, 2024

⌛ Trying commit e30dac4 with merge d504113...

@bors
Copy link
Contributor

bors commented Jan 18, 2024

☀️ Try build successful - checks-actions
Build commit: d504113 (d504113fd9c280eac24e3be1af850f257828459b)

1 similar comment
@bors
Copy link
Contributor

bors commented Jan 18, 2024

☀️ Try build successful - checks-actions
Build commit: d504113 (d504113fd9c280eac24e3be1af850f257828459b)

@epage
Copy link
Contributor

epage commented Jan 18, 2024

Looks like everything is good?

@bors r+

@bors
Copy link
Contributor

bors commented Jan 18, 2024

📌 Commit 2739b6d has been approved by epage

It is now in the queue for this repository.

@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 Jan 18, 2024
@bors
Copy link
Contributor

bors commented Jan 18, 2024

⌛ Testing commit 2739b6d with merge e01eeb0...

@bors
Copy link
Contributor

bors commented Jan 18, 2024

☀️ Test successful - checks-actions
Approved by: epage
Pushing e01eeb0 to master...

@bors bors merged commit e01eeb0 into rust-lang:master Jan 18, 2024
20 checks passed
@Urgau Urgau deleted the improve-ci branch January 18, 2024 17:19
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 24, 2024
Update cargo

10 commits in 1ae631085f01c1a72d05df1ec81f3759a8360042..7bb7b539558dc88bea44cee4168b6269bf8177b0
2024-01-17 17:26:41 +0000 to 2024-01-20 00:15:32 +0000
- feat: inherit jobserver from env for all kinds of runner (rust-lang/cargo#12776)
- Fix static_mut_ref warning. (rust-lang/cargo#13329)
- fix(trim-paths): remap common prefix only (rust-lang/cargo#13210)
- fix(cargo-rustdoc): use same path by output format logic everywhere (rust-lang/cargo#13325)
- chore: Make MSRV=N-2 the workspace default (rust-lang/cargo#13324)
- Fix precise-prerelease tracking link. (rust-lang/cargo#13320)
- test(pkgid): keep package ID format in sync (rust-lang/cargo#13322)
- Improve GitHub Actions CI config (rust-lang/cargo#13317)
- Go back to passing an empty `values()` when no features are declared (rust-lang/cargo#13316)
- fix(`--package`): accept `?` if it's a valid pkgid spec (rust-lang/cargo#13315)

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 24, 2024
Update cargo

10 commits in 1ae631085f01c1a72d05df1ec81f3759a8360042..7bb7b539558dc88bea44cee4168b6269bf8177b0
2024-01-17 17:26:41 +0000 to 2024-01-20 00:15:32 +0000
- feat: inherit jobserver from env for all kinds of runner (rust-lang/cargo#12776)
- Fix static_mut_ref warning. (rust-lang/cargo#13329)
- fix(trim-paths): remap common prefix only (rust-lang/cargo#13210)
- fix(cargo-rustdoc): use same path by output format logic everywhere (rust-lang/cargo#13325)
- chore: Make MSRV=N-2 the workspace default (rust-lang/cargo#13324)
- Fix precise-prerelease tracking link. (rust-lang/cargo#13320)
- test(pkgid): keep package ID format in sync (rust-lang/cargo#13322)
- Improve GitHub Actions CI config (rust-lang/cargo#13317)
- Go back to passing an empty `values()` when no features are declared (rust-lang/cargo#13316)
- fix(`--package`): accept `?` if it's a valid pkgid spec (rust-lang/cargo#13315)

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 24, 2024
Update cargo

10 commits in 1ae631085f01c1a72d05df1ec81f3759a8360042..7bb7b539558dc88bea44cee4168b6269bf8177b0
2024-01-17 17:26:41 +0000 to 2024-01-20 00:15:32 +0000
- feat: inherit jobserver from env for all kinds of runner (rust-lang/cargo#12776)
- Fix static_mut_ref warning. (rust-lang/cargo#13329)
- fix(trim-paths): remap common prefix only (rust-lang/cargo#13210)
- fix(cargo-rustdoc): use same path by output format logic everywhere (rust-lang/cargo#13325)
- chore: Make MSRV=N-2 the workspace default (rust-lang/cargo#13324)
- Fix precise-prerelease tracking link. (rust-lang/cargo#13320)
- test(pkgid): keep package ID format in sync (rust-lang/cargo#13322)
- Improve GitHub Actions CI config (rust-lang/cargo#13317)
- Go back to passing an empty `values()` when no features are declared (rust-lang/cargo#13316)
- fix(`--package`): accept `?` if it's a valid pkgid spec (rust-lang/cargo#13315)

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 24, 2024
Update cargo

10 commits in 1ae631085f01c1a72d05df1ec81f3759a8360042..7bb7b539558dc88bea44cee4168b6269bf8177b0
2024-01-17 17:26:41 +0000 to 2024-01-20 00:15:32 +0000
- feat: inherit jobserver from env for all kinds of runner (rust-lang/cargo#12776)
- Fix static_mut_ref warning. (rust-lang/cargo#13329)
- fix(trim-paths): remap common prefix only (rust-lang/cargo#13210)
- fix(cargo-rustdoc): use same path by output format logic everywhere (rust-lang/cargo#13325)
- chore: Make MSRV=N-2 the workspace default (rust-lang/cargo#13324)
- Fix precise-prerelease tracking link. (rust-lang/cargo#13320)
- test(pkgid): keep package ID format in sync (rust-lang/cargo#13322)
- Improve GitHub Actions CI config (rust-lang/cargo#13317)
- Go back to passing an empty `values()` when no features are declared (rust-lang/cargo#13316)
- fix(`--package`): accept `?` if it's a valid pkgid spec (rust-lang/cargo#13315)

r? ghost
@ehuss ehuss added this to the 1.77.0 milestone Mar 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. 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.

6 participants