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.

Sign up for GitHub

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

Jump to bottom

Avoid rustup invocation for non-rustup rust installation #4219

Merged
merged 1 commit into from
Apr 30, 2020
Merged

Avoid rustup invocation for non-rustup rust installation #4219

merged 1 commit into from
Apr 30, 2020

Conversation

oxalica
Copy link
Contributor

@oxalica oxalica commented Apr 30, 2020

Fix #4218 and #3243.

bjorn3
bjorn3 reviewed Apr 30, 2020
View reviewed changes
xtask/src/lib.rs
Ok(())
Ok(output)
if output.status.success()
&& std::str::from_utf8(&output.stdout)?.contains(RUSTFMT_TOOLCHAIN) =>
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't check that it is the latest version. rustup toolchain install updates if the toolchain isn't the latest version.

Copy link
Contributor Author

@oxalica oxalica Apr 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Does outdated clippy really matter? I also think it's developer's responsibility to choose the correct version of clippy.

Note that the aim of this PR is to make tests and utilities work with bare-installation of rust with components, without rustup. Components may be managed by other package manager, like nix. We should not assume it.

@matklad
Copy link
Member

matklad commented Apr 30, 2020

bors r+

yup, that's a much better approach to doing this. After this PR, I'd like to tweak run! macro to maybe handle env as well, but I'll do this myself as I am not yet sure how it should look.

@bors
Copy link
Contributor

bors bot commented Apr 30, 2020

Build succeeded:

@bors bors bot merged commit 36775ef into rust-lang:master Apr 30, 2020
@oxalica oxalica deleted the fix/rustup-cmd branch April 30, 2020 12:17
@polyzen
Copy link
Contributor

polyzen commented May 5, 2020

Still getting the test failures with rust-analyzer 2020-05-04 and rustfmt provided by the Arch Linux rust package:

> rustfmt --version
rustfmt 1.4.12-
thread 'cli::check_code_formatting' panicked at 'Failed to run rustfmt from toolchain 'stable'. ...

For

  • cli::check_code_formatting
  • cli::generated_assists_are_fresh
  • cli::generated_grammar_is_fresh

@lnicola lnicola mentioned this pull request Jan 22, 2021
Closed
@oxalica oxalica mentioned this pull request Aug 28, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bjorn3 bjorn3 bjorn3 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Avoid calling rustup in tests
4 participants
@oxalica @matklad @polyzen @bjorn3