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

Don't give a hard error if we can't determine the fingerprint for a custom toolchain #67

Merged
merged 1 commit into from
Dec 1, 2022

Conversation

jyn514
Copy link
Collaborator

@jyn514 jyn514 commented Jul 29, 2022

Previously, cargo sweep would unconditionally give an error when it couldn't determine the
fingerprint. But for custom toolchains, it makes sense for it to be linked but give an error when
you try to run it: it means the compiler hasn't yet been built.

Rather than aborting straight away, treat the toolchain as if it isn't installed and clean its artifacts.

This also gives a significantly better error message if cargo-sweep can't determine the fingerprint (which can still happen in practice if you've managed to really mess up one of your toolchains). Before:

[ERROR] Failed to clean "/home/jnelson/rust-community/cargo-sweep/target":

After:

[ERROR] Failed to clean "/home/jnelson/rust-community/cargo-sweep/target": failed to determine fingerprint for toolchain stage1: thread 'main' panicked at 'RUSTC_STAGE was not set: NotPresent', src/bootstrap/bin/rustc.rs:49:41

Fixes #65.

src/fingerprint.rs Outdated Show resolved Hide resolved
@jyn514 jyn514 added the S-waiting-on-review Status: Awaiting review from a maintainer label Aug 12, 2022
@elpiel
Copy link

elpiel commented Sep 29, 2022

@holmgr would it be possible to take a look at this PR? Ever since I've installed cargo-sweep I cannot use the -i option because of this issue.

@jyn514
Copy link
Collaborator Author

jyn514 commented Oct 2, 2022

@elpiel FYI I am the current maintainer of cargo-sweep. I plan to add tests for cargo-sweep before I merge this, since it has a lot of functionality and I've never used more than one or two flags. See #63 (comment).

@jyn514 jyn514 force-pushed the custom-toolchains branch 2 times, most recently from a8c4ba1 to dfb2e90 Compare November 26, 2022 22:44
…ustom toolchain

Previously, cargo sweep would unconditionally give an error when it couldn't determine the
fingerprint. But for custom toolchains, it makes sense for it to be linked but give an error when
you try to run it: it means the compiler hasn't yet been built.

Rather than aborting straight away, treat the toolchain as if it isn't installed and clean its artifacts.
@jyn514 jyn514 merged commit 57a7202 into holmgr:master Dec 1, 2022
@jyn514 jyn514 deleted the custom-toolchains branch December 1, 2022 18:36
@elpiel
Copy link

elpiel commented Dec 1, 2022

Awesome! 🚀

@marcospb19 marcospb19 mentioned this pull request Sep 12, 2023
@marcospb19 marcospb19 mentioned this pull request Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cargo-sweep should ignore custom toolchains
2 participants