feat(cli/rustup-mode): warn about auto-installation in some subcommands#4895
feat(cli/rustup-mode): warn about auto-installation in some subcommands#4895rami3l wants to merge 3 commits into
Conversation
rami3l
added
the
backport
|
Hmmm the blast radius is too large. Lemme think: maybe we should just print the warning as an additional step for |
rami3l
force-pushed
the
feat/backport-auto-install-rustup-warn
branch
3 times, most recently
from
41ff1f3 to
191ea78
Compare
rami3l
changed the title
rustup subcommands
rami3l
force-pushed
the
feat/backport-auto-install-rustup-warn
branch
from
191ea78 to
7375543
Compare
rami3l
force-pushed
the
feat/backport-auto-install-rustup-warn
branch
from
7375543 to
5f10e42
Compare
| } | ||
|
|
||
| // NOTE: Special behavior for rustup v1.29 | ||
| warn!("auto-installation of the active toolchain is enabled when running `rustup`"); |
There was a problem hiding this comment.
Seems weird to have both the warning above and this warning shown at the same time?
There was a problem hiding this comment.
@djc Good catch!
How about showing the existing warning only when the deprecation notice is not shown?
| } | ||
|
|
||
| pub(crate) fn should_auto_install(&self) -> Result<bool> { | ||
| if !self.allow_auto_install { |
There was a problem hiding this comment.
What is this trying to achieve?
There was a problem hiding this comment.
@djc Thanks for asking!
In c037408 (#4840) which is what this PR is based on, self.allow_auto_install is used to reject auto install from happening which is achieved by passing false instead of true for some rustup subcommands which will cause this function to reject auto installation right away.
However, since it is decided that in 1.29 we should only warn instead of rejecting directly, this commit replaces it with a warning.
It should be noted that the warning should have existed in this function but since this will cause too many false positives (we should only show it when an auto-installation is actually triggered, not whenever we are in an auto-install deprecated subcommand), it is moved to another module.
There was a problem hiding this comment.
Please don't hesitate to suggest any changes if you think that will make your review easier.
2 participants
Adapted from
c037408(#4840); closes #4894.