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

Use 1.51.0 in old cargos test #10167

Merged
merged 3 commits into from
Feb 27, 2022
Merged

Use 1.51.0 in old cargos test #10167

merged 3 commits into from
Feb 27, 2022

Conversation

Rustin170506
Copy link
Member

@Rustin170506 Rustin170506 commented Dec 5, 2021 •
edited
Loading

Remove TODO.
If the default toolchain is the same as "stable", just skip the avoids_split_debuginfo_collision test.

@rust-highfive
Copy link

r? @alexcrichton

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 5, 2021
@ehuss
Copy link
Contributor

ehuss commented Dec 5, 2021

Thanks! Unfortunately it looks like these tests have atrophied over time. Would you be willing to try to get them back into running shape? If not, that's ok, I can probably get them working again. It might be somewhat involved to make the output conditional based on the version.

It looks like all versions from 1.51 onwards aren't working due to some output changes (and there have been various output changes over time).

I'm also wondering if it might be feasible to remove the #[ignore] lines to prevent that. I can't remember if there was a really good reason for that.

@Rustin170506
Copy link
Member Author

Would you be willing to try to get them back into running shape?

I will try to fix it in the next week.

@ehuss ehuss assigned ehuss and unassigned alexcrichton Dec 13, 2021
@ehuss ehuss 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 Dec 13, 2021
@Rustin170506
Copy link
Member Author

@ehuss It's already working correctly locally.
I think the reason it was originally ignored was probably because it relied on a couple of specific versions of toolchain.
Do you think we need to run a separate GitHub actions job for it?

@Rustin170506
Copy link
Member Author

I think the reason it was originally ignored was probably because it relied on a couple of specific versions of toolchain.
Do you think we need to run a separate GitHub actions job for it?

@ehuss Happy New Year! Could you please take a look?

@ehuss
Copy link
Contributor

ehuss commented Jan 3, 2022

I'd prefer not to change CI to accommodate these tests for now. It looks like index_cache_rebuild fundamentally needs 1.48 installed, so I would leave that with #[ignore] for now.

As for avoids_split_debuginfo_collision, that test needs two different toolchains. If the default toolchain is stable, then it won't work. I probably never tested that scenario. Also, I think maybe the test needs to be updated due to changes in #9653. I think one way to fix it is to check if the default toolchain is the same as "stable", just skip the test.

@Rustin170506

This comment was marked as outdated.

Sign in to view
@Rustin170506

This comment was marked as outdated.

Sign in to view
@Rustin170506
Copy link
Member Author

@ehuss
Could you please take a look? Thanks! 💚 💙 💜 💛 ❤️

@Rustin170506
Copy link
Member Author

nightly also work!

@Rustin170506
Copy link
Member Author

@rustbot ready

@rustbot
Copy link
Collaborator

rustbot commented Jan 16, 2022

Error: The feature shortcut is not enabled in this repository.
To enable it add its section in the triagebot.toml in the root of the repository.

Please let @rust-lang/release know if you're having trouble with this bot.

@Rustin170506 Rustin170506 mentioned this pull request Jan 16, 2022
Merged
bors added a commit that referenced this pull request Jan 16, 2022
@bors
Auto merge of #10298 - hi-rustin:rustin-patch-triagebot, r=ehuss
e778b3d 
Enable shortcut for triage bot

### What does this PR try to resolve?

Enable shortcut for triage bot. See: #10167 (comment)
### How should we test and review this PR?

We need to test it after the merge. But there should be no problem with the robot. I refer to the rust configuration file.

### Additional information

I don't know if it is accepted to open it, if we don't want to open it please feel free to close my PR.
@Rustin170506
Copy link
Member Author

@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 Jan 17, 2022
@Rustin170506
Copy link
Member Author

Friendly ping~ @ehuss

Could you please take a look? Thanks! 💚 💙 💜 💛 ❤️

ehuss
ehuss reviewed Feb 25, 2022
View reviewed changes
Copy link
Contributor

@ehuss ehuss left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Sorry about the delay.

It looks like new_features has fallen behind again. Starting with 1.60, namespaced-features has been stabilized. That means the validity checks will need to have three different checks:

  • < 1.51
  • >= 1.51 && < 1.60
  • >= 1.60

Can you add the new 1.60 behavior?

tests/testsuite/old_cargos.rs Show resolved Hide resolved
tests/testsuite/old_cargos.rs Outdated Show resolved Hide resolved
@Rustin170506
Use 1.51.0 in old cargos test
047edec 
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
@Rustin170506
Skip avoids_split_debuginfo_collision on stable
626bf2c 
Signed-off-by: hi-rustin <rustin.liu@gmail.com>

Address comments

Signed-off-by: hi-rustin <rustin.liu@gmail.com>
@Rustin170506
Check 1.60 behavior
9d93415 
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
@Rustin170506
Copy link
Member Author

I have tested on these tool chains:

➜  cargo git:(rustin-patch-test) rustup toolchain list          
stable-x86_64-apple-darwin
beta-x86_64-apple-darwin (default)
nightly-x86_64-apple-darwin
1.11.0-x86_64-apple-darwin
1.12.0-x86_64-apple-darwin
1.48.0-x86_64-apple-darwin
1.50.0-x86_64-apple-darwin
1.51.0-x86_64-apple-darwin
1.52.0-x86_64-apple-darwin
1.53.0-x86_64-apple-darwin
1.59.0-x86_64-apple-darwin

@ehuss
Copy link
Contributor

ehuss commented Feb 27, 2022

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Feb 27, 2022

📌 Commit 9d93415 has been approved by ehuss

@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 Feb 27, 2022
@bors
Copy link
Contributor

bors commented Feb 27, 2022

⌛ Testing commit 9d93415 with merge 188298f...

@bors
Copy link
Contributor

bors commented Feb 27, 2022

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing 188298f to master...

@bors bors merged commit 188298f into rust-lang:master Feb 27, 2022
@jonhoo jonhoo mentioned this pull request Feb 28, 2022
Merged
matthiaskrgr pushed a commit to matthiaskrgr/rust that referenced this pull request Mar 1, 2022
Update cargo
6b476fe 
11 changes in
d6cdde584a1f15ea086bae922e20fd27f7165431..3d6970d50e30e797b8e26b2b9b1bdf92dc381f34
2022-02-22 19:55:51 +0000 to 2022-02-28 19:29:07 +0000:

 - rust-lang/cargo#10395
 - rust-lang/cargo#10425
 - rust-lang/cargo#10428
 - rust-lang/cargo#10388
 - rust-lang/cargo#10167
 - rust-lang/cargo#10429
 - rust-lang/cargo#10426
 - rust-lang/cargo#10372
 - rust-lang/cargo#10420
 - rust-lang/cargo#10416
 - rust-lang/cargo#10417
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 1, 2022
@matthiaskrgr
Rollup merge of rust-lang#94459 - jonhoo:bump-cargo, r=Dylan-DPC
6aab954 
Update cargo

11 changes in
d6cdde584a1f15ea086bae922e20fd27f7165431..3d6970d50e30e797b8e26b2b9b1bdf92dc381f34
2022-02-22 19:55:51 +0000 to 2022-02-28 19:29:07 +0000:

 - rust-lang/cargo#10395
 - rust-lang/cargo#10425
 - rust-lang/cargo#10428
 - rust-lang/cargo#10388
 - rust-lang/cargo#10167
 - rust-lang/cargo#10429
 - rust-lang/cargo#10426
 - rust-lang/cargo#10372
 - rust-lang/cargo#10420
 - rust-lang/cargo#10416
 - rust-lang/cargo#10417
@ehuss ehuss added this to the 1.61.0 milestone Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ehuss ehuss Awaiting requested review from ehuss

Assignees

@ehuss ehuss

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.61.0
Development

Successfully merging this pull request may close these issues.
6 participants
@Rustin170506 @rust-highfive @ehuss @rustbot @bors @alexcrichton