Skip to content

feat(self_update): add proxy sanity checks #4338

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

Merged
merged 1 commit into from
May 21, 2025

Conversation

manyinsects
Copy link
Member

@manyinsects manyinsects commented May 17, 2025

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

Copy link
Member

@rami3l rami3l left a comment

Choose a reason for hiding this comment

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

Not bad for the first PR, thanks a lot :)

The existing error messages might indicate that the default toolchain is unset in certain situations, so instead of calling <bin> --version, maybe call <bin> +<toolchain> --version?

@manyinsects manyinsects force-pushed the proxy-sanity-checks branch 3 times, most recently from 780a1e1 to eb1b8c8 Compare May 17, 2025 18:02
@manyinsects
Copy link
Member Author

this should be ready for re-review :)

Copy link
Member

@rami3l rami3l left a comment

Choose a reason for hiding this comment

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

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?

@rami3l rami3l self-assigned this May 19, 2025
@manyinsects manyinsects force-pushed the proxy-sanity-checks branch from eb1b8c8 to 94ed362 Compare May 19, 2025 18:02
@rami3l rami3l requested a review from ChrisDenton May 20, 2025 02:22
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.
@rami3l rami3l force-pushed the proxy-sanity-checks branch from 94ed362 to 3b58a8d Compare May 21, 2025 05:37
@rami3l rami3l enabled auto-merge May 21, 2025 05:37
@rami3l rami3l added this pull request to the merge queue May 21, 2025
Merged via the queue into rust-lang:master with commit ac249c8 May 21, 2025
29 checks passed
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.

Sanity check for proxies created during the installation phase
3 participants