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.

Sign up for GitHub

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

Jump to bottom

[beta-1.71.0] detect the channel a PR wants to merge into #12200

Closed
wants to merge 1 commit into from
Closed

[beta-1.71.0] detect the channel a PR wants to merge into #12200

wants to merge 1 commit into from

Conversation

weihanglo
Copy link
Member

What does this PR try to resolve?

This detects which channel a pull request wants to merge into.

It is hard because bors runs in a different repo.
Bors knows nothing about branches or tag in this repo.
It only see one base commit and a merge commit AFAIK.

Basically the assumption and idea are

  1. bors create a merge commit, so HEAD~ should find the base branch.
  2. Cargo maintains rust-1.y.0 branch for each Rust minor version releases.
    Therefore, we can use the symbolic name of origin/rust-1.x.0
    to determine if it aims for stable, beta, or nightly channel.
  3. After we know which channel it is targeted at,
    we can skip jobs that are likely to be broken.

How should we test and review this PR?

This is like #12181 but targeted on 1.71.0.

r? ehuss

@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 May 29, 2023
@weihanglo
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented May 29, 2023

⌛ Trying commit 10a60f4 with merge c7eea87...

bors added a commit that referenced this pull request May 29, 2023
@bors
Auto merge of #12200 - weihanglo:which-channel-backport-rust-1.71.0, …
c7eea87 
…r=<try>

chore: detect the channel a PR wants to merge into

### What does this PR try to resolve?

This detects which channel a pull request wants to merge into.

It is hard because bors runs in a different repo.
Bors knows nothing about branches or tag in this repo.
It only see one base commit and a merge commit AFAIK.

Basically the assumption and idea are

1. bors create a merge commit, so `HEAD~` should find the base branch.
2. Cargo maintains `rust-1.y.0` branch for each Rust minor version releases.
   Therefore, we can use the symbolic name of `origin/rust-1.x.0`
   to determine if it aims for stable, beta, or nightly channel.
3. After we know which channel it is targeted at,
   we can skip jobs that are likely to be broken.

### How should we test and review this PR?

This is like #12181 but targeted on 1.71.0.

r? ehuss
@weihanglo weihanglo changed the base branch from master to rust-1.71.0 May 29, 2023 19:47
@rustbot
Copy link
Collaborator

rustbot commented May 29, 2023

Could not assign reviewer from: ehuss.
User(s) ehuss are either the PR author or are already assigned, and there are no other candidates.
Use r? to specify someone else to assign.

@bors
Copy link
Contributor

bors commented May 29, 2023

⚠️ The base branch changed to rust-1.71.0, and the PR will need to be re-approved.

@bors bors added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 29, 2023
@bors
Copy link
Contributor

bors commented May 29, 2023

⌛ Trying commit 10a60f4 with merge e5696fd...

bors added a commit that referenced this pull request May 29, 2023
@bors
Auto merge of #12200 - weihanglo:which-channel-backport-rust-1.71.0, …
e5696fd 
…r=<try>

chore: detect the channel a PR wants to merge into

### What does this PR try to resolve?

This detects which channel a pull request wants to merge into.

It is hard because bors runs in a different repo.
Bors knows nothing about branches or tag in this repo.
It only see one base commit and a merge commit AFAIK.

Basically the assumption and idea are

1. bors create a merge commit, so `HEAD~` should find the base branch.
2. Cargo maintains `rust-1.y.0` branch for each Rust minor version releases.
   Therefore, we can use the symbolic name of `origin/rust-1.x.0`
   to determine if it aims for stable, beta, or nightly channel.
3. After we know which channel it is targeted at,
   we can skip jobs that are likely to be broken.

### How should we test and review this PR?

This is like #12181 but targeted on 1.71.0.

r? ehuss
ehuss
ehuss reviewed May 29, 2023
View reviewed changes
.github/workflows/main.yml Outdated
sh linkcheck.sh --all --path ../src/doc cargo

# Jobs here must be skipped if they aren't going to merge into master (nightly).
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this comment also explain more about the reason this exists? That is, saying something like "This is needed because GitHub Actions treats success() as false if a job is skipped, and the bors success/failure jobs below need to ignore skipped jobs."

Copy link
Member Author

Choose a reason for hiding this comment

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

Now it looks better. Thanks for the suggestion!

@weihanglo weihanglo force-pushed the which-channel-backport-rust-1.71.0 branch 3 times, most recently from 8203259 to a3354a4 Compare May 29, 2023 20:47
@weihanglo
Copy link
Member Author

Let's @bors try to see if bors can survive with skipped build_std job.

@bors
Copy link
Contributor

bors commented May 29, 2023

⌛ Trying commit a3354a4 with merge b3ad383...

bors added a commit that referenced this pull request May 29, 2023
@bors
Auto merge of #12200 - weihanglo:which-channel-backport-rust-1.71.0, …
b3ad383 
…r=<try>

chore: detect the channel a PR wants to merge into

### What does this PR try to resolve?

This detects which channel a pull request wants to merge into.

It is hard because bors runs in a different repo.
Bors knows nothing about branches or tag in this repo.
It only see one base commit and a merge commit AFAIK.

Basically the assumption and idea are

1. bors create a merge commit, so `HEAD~` should find the base branch.
2. Cargo maintains `rust-1.y.0` branch for each Rust minor version releases.
   Therefore, we can use the symbolic name of `origin/rust-1.x.0`
   to determine if it aims for stable, beta, or nightly channel.
3. After we know which channel it is targeted at,
   we can skip jobs that are likely to be broken.

### How should we test and review this PR?

This is like #12181 but targeted on 1.71.0.

r? ehuss
@weihanglo
Copy link
Member Author

I guess it's good to merge?

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. labels May 29, 2023
@weihanglo weihanglo changed the title chore: detect the channel a PR wants to merge into [beta-1.71.0] detect the channel a PR wants to merge into May 29, 2023
@bors
Copy link
Contributor

bors commented May 29, 2023

💥 Test timed out

@weihanglo weihanglo force-pushed the which-channel-backport-rust-1.71.0 branch from a3354a4 to a919c0e Compare May 30, 2023 10:47
@ehuss
Copy link
Contributor

ehuss commented May 30, 2023

Looks good, thanks! We should probably try to monitor the Actions runs over the next week or two to make sure we didn't make any mistakes with this. It can be tricky when jobs pass when they shouldn't, since it can take a long while for people to notice.

@bors r+

@bors
Copy link
Contributor

bors commented May 30, 2023

📌 Commit a919c0e has been approved by ehuss

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 30, 2023
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label May 30, 2023
@bors
Copy link
Contributor

bors commented May 30, 2023

⌛ Testing commit a919c0e with merge 7b9e7c4...

bors added a commit that referenced this pull request May 30, 2023
@bors
Auto merge of #12200 - weihanglo:which-channel-backport-rust-1.71.0, …
7b9e7c4 
…r=ehuss

[beta-1.71.0] detect the channel a PR wants to merge into

### What does this PR try to resolve?

This detects which channel a pull request wants to merge into.

It is hard because bors runs in a different repo.
Bors knows nothing about branches or tag in this repo.
It only see one base commit and a merge commit AFAIK.

Basically the assumption and idea are

1. bors create a merge commit, so `HEAD~` should find the base branch.
2. Cargo maintains `rust-1.y.0` branch for each Rust minor version releases.
   Therefore, we can use the symbolic name of `origin/rust-1.x.0`
   to determine if it aims for stable, beta, or nightly channel.
3. After we know which channel it is targeted at,
   we can skip jobs that are likely to be broken.

### How should we test and review this PR?

This is like #12181 but targeted on 1.71.0.

r? ehuss
@bors
Copy link
Contributor

bors commented May 30, 2023

💔 Test failed - checks-actions

@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 May 30, 2023
@weihanglo weihanglo mentioned this pull request May 30, 2023
Merged
@weihanglo
Copy link
Member Author

Close. See #12204.

@weihanglo weihanglo closed this May 30, 2023
@weihanglo weihanglo deleted the which-channel-backport-rust-1.71.0 branch May 30, 2023 21:46
bors added a commit that referenced this pull request May 30, 2023
@bors
Auto merge of #12204 - rust-lang:revert-12181-which-channel-backport,…
814aa46 
… r=epage

Revert "chore: detect the channel a PR wants to merge into"

Reverts #12181

GitHub Action expression `success()` will look into the chain of dependent jobs. On master CI passed but on a backport it still failed <#12200 (comment)>

This isn't worth pursuing at this moment either. To fix it, it's unavoidable to write something like below for bors job

```yaml
    if: "always()
      && !(contains(needs.*.result, 'cancelled')
        || contains(needs.*.result, 'skipped')
        || contains(needs.*.result, 'failure')
      )
      && github.event_name == 'push'
      && github.ref == 'refs/heads/auto-cargo'
    "
```

I don't think that's good in long-term.

r? ehuss
bors added a commit that referenced this pull request May 30, 2023
@bors
Auto merge of #12204 - rust-lang:revert-12181-which-channel-backport,…
3f09c0b 
… r=epage

Revert "chore: detect the channel a PR wants to merge into"

Reverts #12181

GitHub Action expression `success()` will look into the chain of dependent jobs. On master CI passed but on a backport it still failed <#12200 (comment)>

This isn't worth pursuing at this moment either. To fix it, it's unavoidable to write something like below for bors job

```yaml
    if: "always()
      && !(contains(needs.*.result, 'cancelled')
        || contains(needs.*.result, 'skipped')
        || contains(needs.*.result, 'failure')
      )
      && github.event_name == 'push'
      && github.ref == 'refs/heads/auto-cargo'
    "
```

I don't think that's good in long-term.

r? ehuss
bors added a commit that referenced this pull request May 31, 2023
@bors
Auto merge of #12204 - rust-lang:revert-12181-which-channel-backport,…
99430e5 
… r=epage

Revert "chore: detect the channel a PR wants to merge into"

Reverts #12181

GitHub Action expression `success()` will look into the chain of dependent jobs. On master CI passed but on a backport it still failed <#12200 (comment)>

This isn't worth pursuing at this moment either. To fix it, it's unavoidable to write something like below for bors job

```yaml
    if: "always()
      && !(contains(needs.*.result, 'cancelled')
        || contains(needs.*.result, 'skipped')
        || contains(needs.*.result, 'failure')
      )
      && github.event_name == 'push'
      && github.ref == 'refs/heads/auto-cargo'
    "
```

I don't think that's good in long-term.

r? ehuss
@weihanglo weihanglo mentioned this pull request Jul 26, 2023
Merged
bors added a commit that referenced this pull request Jul 26, 2023
@bors
Auto merge of #12395 - weihanglo:cargo-semver-checks, r=<try>
aeac80e 
ci: rewrite bump check and respect semver

### What does this PR try to resolve?

This ports `ci/validate-version-bump.sh` and #12200 to the new xtask `bump-check`. It also adds the ability to set the correct baseline revision when checking version bump. That is, it fixes #12347.

In addition, this integrates the community tool `cargo-semver-checks`. SemVer violation can now be detected earlier.

### How should we test and review this PR?

This PR extracts the registry query part from `xtask-unpublished` and removes other pars. I don't feel like we need it in the short term.

For how it works, please the check doc comment in each function.

One concern is that this check is still a required job for bors. I believe `@obi1kenobi` is quite responsive and willing to help if there is something wrong. So, waiting for an upstream fix won't be a problem for cargo. Also as a good citizen in the community, we can always contribute back.
(Take it easy `@obi1kenobi,` don't be stressed out 🙂)
<!-- homu-ignore:end -->
bors added a commit that referenced this pull request Jul 26, 2023
@bors
Auto merge of #12395 - weihanglo:cargo-semver-checks, r=<try>
aa5f096 
ci: rewrite bump check and respect semver

### What does this PR try to resolve?

This ports `ci/validate-version-bump.sh` and #12200 to the new xtask `bump-check`. It also adds the ability to set the correct baseline revision when checking version bump. That is, it fixes #12347.

In addition, this integrates the community tool `cargo-semver-checks`. SemVer violation can now be detected earlier.

### How should we test and review this PR?

This PR extracts the registry query part from `xtask-unpublished` and removes other pars. I don't feel like we need it in the short term.

For how it works, please the check doc comment in each function.

One concern is that this check is still a required job for bors. I believe `@obi1kenobi` is quite responsive and willing to help if there is something wrong. So, waiting for an upstream fix won't be a problem for cargo. Also as a good citizen in the community, we can always contribute back.
(Take it easy `@obi1kenobi,` don't be stressed out 🙂)
<!-- homu-ignore:end -->
bors added a commit that referenced this pull request Aug 1, 2023
@bors
Auto merge of #12395 - weihanglo:cargo-semver-checks, r=epage
6145d0c 
ci: rewrite bump check and respect semver

### What does this PR try to resolve?

This ports `ci/validate-version-bump.sh` and #12200 to the new xtask `bump-check`. It also adds the ability to set the correct baseline revision when checking version bump. That is, it fixes #12347.

In addition, this integrates the community tool `cargo-semver-checks`. SemVer violation can now be detected earlier.

### How should we test and review this PR?

This PR extracts the registry query part from `xtask-unpublished` and removes other pars. I don't feel like we need it in the short term.

For how it works, please the check doc comment in each function.

One concern is that this check is still a required job for bors. I believe `@obi1kenobi` is quite responsive and willing to help if there is something wrong. So, waiting for an upstream fix won't be a problem for cargo. Also as a good citizen in the community, we can always contribute back.
(Take it easy `@obi1kenobi,` don't be stressed out 🙂)
<!-- homu-ignore:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ehuss ehuss ehuss left review comments

Assignees

@ehuss ehuss

Labels
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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@weihanglo @bors @rustbot @ehuss