Warnings for breaking changes to the configuration discovery of cargo install and cargo uninstall proposed in #11775

[jofas]

#11775 tries to streamline the configuration discovery of cargo uninstall with cargo install and make both more intuitive and in sync with how crate-local configuration discovery works. Unfortunately, these are breaking changes. So this PR adds warning messages to cargo install and cargo uninstall, informing the user that the configuration discovery will change in a future version of cargo.

[rustbot]

r? @epage

(rustbot has picked a reviewer for you, use r? to override)

[jofas]

Shoot, forgot to run the test suite locally and check if my changes broke any tests

[jofas]

Seemingly I broke all tests (>50) that run cargo install/cargo uninstall and check stderr output with .with_stderr, instead of .with_stderr_contains. Anyone any idea how I should proceed (CC @weihanglo)? Should I add the warning to each call of .with_stderr or maybe reduce all the panicking .with_stderr calls to .with_stderr_contains?

This test broke, for example:

cargo/tests/testsuite/install.rs

Lines 33 to 58 in e472ccc

	#[cargo_test]
	fn simple() {
	pkg("foo", "0.0.1");
	cargo_process("install foo")
	.with_stderr(
	"\
	[UPDATING] `[..]` index
	[DOWNLOADING] crates ...
	[DOWNLOADED] foo v0.0.1 (registry [..])
	[INSTALLING] foo v0.0.1
	[COMPILING] foo v0.0.1
	[FINISHED] release [optimized] target(s) in [..]
	[INSTALLING] [CWD]/home/.cargo/bin/foo[EXE]
	[INSTALLED] package `foo v0.0.1` (executable `foo[EXE]`)
	[WARNING] be sure to add `[..]` to your PATH to be able to run the installed binaries
	",
	)
	.run();
	assert_has_installed_exe(cargo_home(), "foo");
	cargo_process("uninstall foo")
	.with_stderr("[REMOVING] [CWD]/home/.cargo/bin/foo[EXE]")
	.run();
	assert_has_not_installed_exe(cargo_home(), "foo");
	}

because now we have an additional, unexpected [WARNING] ... in stderr of cargo install and cargo uninstall.

[weihanglo]

First of all, I am not sure if everyone in the Cargo team buys in this change. It may or may not change based on the consensus 😞.

Seemingly I broke all tests (>50) that run cargo install/cargo uninstall and check stderr output with .with_stderr, instead of .with_stderr_contains

I wish we could have turned trivial tests into UI tests style. So that we don't need to update each test manually.

Specifically to this fix, Cargo should strive to be as less verbose as it could (although it is already too verbose), so flushing messages to every user running cargo install is definitely no-go. I would probably limit the message to users that load configurations from locations other than ~/.cargo/config.toml. However, I feel even we have shipped this warning, it won't be landed in any near future, as it would highly possibly break a lot of automation processes, especially Docker builds which I've seen they were putting config on /.cargo/config.toml.

[jofas]

However, I feel even we have shipped this warning, it won't be landed in any near future, as it would highly possibly break a lot of automation processes, especially Docker builds which I've seen they were putting config on /.cargo/config.toml.

Very interesting. Well, I'll close this PR then, as the changes I've made aren't thought through well enough and I underestimated the complexity of what I was trying to implement. Maybe some other day 🤷‍♂️