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

Forbid unprefixed SHAs for toolchains #723

Merged
merged 1 commit into from
Mar 31, 2024
Merged

Conversation

lqd
Copy link
Member

@lqd lqd commented Mar 18, 2024

This PR forbids unprefixed SHAs for toolchain arguments, to avoid crater hanging when they appear.

I'm not sure this is fully valid though: I don't know if SHAs could appear in a Toolchain's name (and RustwideToolchain::dist()) without being an error 😓. This commonly happens with rustup-toolchain-install-master but I don't know if a similar situation could arise within common crater usage.

CI will surely fail: some of the used dependencies are broken on nightly right now, and clippy emits warnings -- both issues are fixed in #722.

(this is for @oli-obk and for @compiler-errors)

Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

Thanks!

@Mark-Simulacrum
Copy link
Member

@bors r+

This seems reasonable to me.

@bors
Copy link
Contributor

bors commented Mar 31, 2024

📌 Commit aedec6d has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Mar 31, 2024

⌛ Testing commit aedec6d with merge a00f6b3...

bors added a commit that referenced this pull request Mar 31, 2024
Forbid unprefixed SHAs for toolchains

This PR forbids unprefixed SHAs for toolchain arguments, to avoid crater hanging when they appear.

I'm not sure this is fully valid though: I don't know if SHAs could appear in a `Toolchain`'s name (and `RustwideToolchain::dist()`) without being an error 😓. This commonly happens with `rustup-toolchain-install-master` but I don't know if a similar situation could arise within common crater usage.

CI will surely fail: some of the used dependencies are broken on nightly right now, and clippy emits warnings -- both issues are fixed in #722.

<sub>(this is [for](rust-lang/rust#122230 (comment)) `@oli-obk` and [for](rust-lang/rust#122502 (comment)) `@compiler-errors)</sub>`
@bors
Copy link
Contributor

bors commented Mar 31, 2024

💔 Test failed - checks-actions

@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Mar 31, 2024

💡 This pull request was already approved, no need to approve it again.

  • This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.

@bors
Copy link
Contributor

bors commented Mar 31, 2024

📌 Commit aedec6d has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Mar 31, 2024

⌛ Testing commit aedec6d with merge f2a10b4...

@bors
Copy link
Contributor

bors commented Mar 31, 2024

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing f2a10b4 to master...

1 similar comment
@bors
Copy link
Contributor

bors commented Mar 31, 2024

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing f2a10b4 to master...

@bors bors merged commit f2a10b4 into rust-lang:master Mar 31, 2024
2 of 5 checks passed
@bors
Copy link
Contributor

bors commented Mar 31, 2024

👀 Test was successful, but fast-forwarding failed: 422 Changes must be made through a pull request.

@lqd lqd deleted the unprefixed-shas branch March 31, 2024 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants