feat(self_update): add proxy sanity checks #4338
Open
+31
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rami3l
reviewed
May 17, 2025
rami3l
requested changes
May 17, 2025
manyinsects
force-pushed
the
proxy-sanity-checks
branch
3 times, most recently
from
780a1e1 to
eb1b8c8
Compare
|
this should be ready for re-review :) |
rami3l
reviewed
May 19, 2025
rami3l
reviewed
May 19, 2025
rami3l
reviewed
May 19, 2025
rami3l
approved these changes
May 19, 2025
There was a problem hiding this comment.
Thanks a lot! I've left a few tiny nits but the overall it looks pretty nice!
cc @ChrisDenton I don't currently think we need to add tests for this particular PR, so modulo the comments I think it's okay to merge it as-is and see how it goes?
Sometimes, proxies can be incorrectly installed for whatever reason. This adds a new check after initial component installation to call all proxies with --version, and to fail if one of them doesn't report success.
manyinsects
force-pushed
the
proxy-sanity-checks
branch
from
eb1b8c8 to
94ed362
Compare
djc
reviewed
May 20, 2025
| help: this might be a bug in rustup, please open a new issue here:\n\ | ||
| help: https://github.com/rust-lang/rustup/issues/new" | ||
| )] | ||
| InvalidProxyInstall, |
There was a problem hiding this comment.
I'd avoid this wording and variant name since technically the proxy install completed without error -- it's just that the installed proxy does not actually work.
There was a problem hiding this comment.
@djc How about:
Suggested change
| InvalidProxyInstall, | |
| BrokenProxy, |
... and
"the rustup executable proxies don't seem to work"
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sometimes, proxies can be incorrectly installed for whatever reason. This adds a new check after initial component installation to call all proxies with --version, and to fail if one of them doesn't report success.
Closes #4336 (but there should probably be a followup issue for potentially self-repairing broken proxies?).
There are two test failures in my local testing:this has been resolved (for now...?) by limiting to only core components