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

Fix 2601 #2602

Merged
merged 3 commits into from
Dec 24, 2020
Merged

Fix 2601 #2602

merged 3 commits into from
Dec 24, 2020

Conversation

kinnison
Copy link
Contributor

@kinnison kinnison commented Dec 9, 2020

Previously we attempted to resolve a situation where a toolchain which had a wildcarded component installed as target-specific due to profiles would break things. Sadly it seems we missed one other mechanism by which such a component installation might occur - namely if someone requests a wildcarded component as an additional component during toolchain installation.

This PR resolves this by detecting such components during the building of the dist changeset. As such, it fixes #2601

@eddyb When this has built, assuming a green CI, if you're logged into Github then you should be able to download an artifact from the relevant CI run and try it out. Would you mind checking if this fixes your problem?

Signed-off-by: Daniel Silverstone <dsilvers@digital-scurf.org>
Sometimes someone might choose an additional component to add
during an install/update which is wildcarded.  We need to ensure
that we detect this and request the install with the wildcard
otherwise we can hit situations such as that identified in rust-lang#2601

Signed-off-by: Daniel Silverstone <dsilvers@digital-scurf.org>
@eddyb
Copy link
Member

eddyb commented Dec 9, 2020

I can't use the artifact myself directly, but I can do what I did in NixOS/nixpkgs#106443, only just locally, and pointing to your branch, then I get a "distro rustup" that I can do all my testing with.

I'll have to do that tomorrow though, but thanks for the fix!

@eddyb
Copy link
Member

eddyb commented Dec 10, 2020

I can confirm the sequence from #2601 behaves correctly, but that's because it starts by removing nightly.

But if I have rust-src-x86_64-unknown-linux-gnu in lib/rustlib/components from an older rustup, this PR makes both methods to install a component fail in the manner described in #2601 (comment) (to reproduce, run the first two commands in that comment with an older rustup).

So as I first suspected, this PR makes rustup toolchain install ... --component rust-src behave like rustup component add rust-src, which is an improvement, but rustup component add rust-src failed when rust-src-x86_64-unknown-linux-gnu was installed, and probably requires work elsewhere to trigger the self-repair.

Interestingly enough, installing any other component triggers the self-repair, but for rust-src I guess it triggers too late?

@kinnison
Copy link
Contributor Author

Oh crap yes, if you're already broken then it won't trigger the repair. Hmm, I'll need to decide how to handle this more cleanly. This was mostly to not break in the first place I hope.

When self-healing from a bad rust-src install as a side effect
of someone running `rustup component add rust-src` we would accidentally
double-add the rust-src component resulting in a conflict.

This fixes that.

Signed-off-by: Daniel Silverstone <dsilvers@digital-scurf.org>
@kinnison
Copy link
Contributor Author

I think this was being over-eager when self-healing rust-src misinstallation. We ended up double-installing rust-src which is what caused the actual conflict listed. Assuming CI goes green, would you give this branch a test for you. I think it's good now.

@eddyb
Copy link
Member

eddyb commented Dec 17, 2020

My bad, I haven't had a chance to look at this yet, it slipped through the cracks.

@kinnison
Copy link
Contributor Author

@eddyb That's quite alright, everyone's busy :D Just let me know when you can give it a go please?

@eddyb
Copy link
Member

eddyb commented Dec 24, 2020

The issue I described in #2602 (comment) (with a a toolchain from an older rustup) appears to be fixed, and the same-rustup behavior is still fixed.

Again apologies for the (extra one week) delay, I've been having a hard time doing software things lately.

@kinnison
Copy link
Contributor Author

No need to apologise, this time of year is tough for most people to get time, let alone to get time to deal with small side issues.

Thank you for checking it out, I'm going to merge this then.

@kinnison kinnison merged commit 373c808 into rust-lang:master Dec 24, 2020
@kinnison kinnison deleted the fix-2601 branch December 24, 2020 08:19
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.

rust-src component installed by rustup component add not seen by rustup toolchain install ... --component.
2 participants