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.

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

Make .cargo/config deprecation warnings silent #13667

Closed
ijackson opened this issue Mar 29, 2024 · 5 comments · Fixed by #13793
Closed

Make .cargo/config deprecation warnings silent #13667

ijackson opened this issue Mar 29, 2024 · 5 comments · Fixed by #13793
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself. C-bug Category: bug S-triage Status: This issue is waiting on initial triage.

Comments

@ijackson
Copy link
Contributor

Problem

I want to be able to run cargo 1.31 and have it respect my ,.cargo/config.

There seems to be no way to achieve this without causing recent cargo to issue a warning.

Steps

  1. Try: config is a file, config.toml doesn't exist
warning: `/home/rustcargo/.cargo/config` is deprecated in favor of `config.toml`
note: if you need to support cargo 1.38 or earlier, you can symlink `config` to `config.toml`

(Note that the phrase "symlink X to Y" is ambiguous. Does it mean make X a symlink to Y, or vice versa?)

  1. Try: config is a file; config.toml is a symlink to it
warning: both `/home/rustcargo/.cargo/config` and `/home/rustcargo/.cargo/config.toml` exist. Using `/home/rustcargo/.cargo/config`
  1. Try config.toml is a file, config is a symlink to it
warning: both `/home/rustcargo/.cargo/config` and `/home/rustcargo/.cargo/config.toml` exist. Using `/home/rustcargo/.cargo/config`

Possible Solution(s)

Check to see if one of the files is a link to the other. If so, don't issue the warning about both files existing.

The "one file is a link to the other" check should be done with stat and comparing st_dev and st_ino. (AIUI Windows doesn't have symlinks in the same way so we don't need to do this check there.)

Notes

This is a blocker for me upgrading the pinned nightly used by CI tests for derive-deftly, because I have UI tests that check for the precise stderr output. Right now, those tests always produce polluted output in my local environment.

This is happening to me with +nightly-2024-03-14. I can't easily try a more recent version because my rustup is completely broken due to rust-lang/rustup#3737

Version

rustcargo@zealot:~/.cargo$ cargo -vV
cargo 1.77.0-beta.5 (837c09f04 2024-02-16)
release: 1.77.0-beta.5
commit-hash: 837c09f04022352c26cf445f37492709922b24ef
commit-date: 2024-02-16
host: x86_64-unknown-linux-gnu
libgit2: 1.7.2 (sys:0.18.2 vendored)
libcurl: 8.5.0-DEV (sys:0.4.70+curl-8.5.0 vendored ssl:OpenSSL/1.1.1w)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Debian 10 (buster) [64-bit]
rustcargo@zealot:~/.cargo$
@ijackson ijackson added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Mar 29, 2024
@weihanglo
Copy link
Member

This is a blocker for me upgrading the pinned nightly used by CI tests for derive-deftly, because I have UI tests that check for the precise stderr output. Right now, those tests always produce polluted output in my local environment.

I would suggest not to check Cargo's stderr, as it is generally for human and subject to change at any time. That being said, #12235 is a way out, and we've landed the initial version of the linting system in #13621.

Check to see if one of the files is a link to the other. If so, don't issue the warning about both files existing.

Personally I wouldn't add more complexity to the logic there until we have a lint for it. I don't speak for others.

@weihanglo weihanglo changed the title config.toml: No way to support cargo 1.37 and also avoid warning Make .cargo/config deprecation warnings silent Mar 29, 2024
@weihanglo weihanglo added the A-diagnostics Area: Error and warning messages generated by Cargo itself. label Mar 29, 2024
@ijackson
Copy link
Contributor Author

Personally I wouldn't add more complexity to the logic there until we have a lint for it. I don't speak for others.

It seems to me that the current situation is simply wrong. cargo prints a warning, and suggests a resolution, but then it also prints a warning about the suggested resolution.

cargo should not suggest a resolution that isn't effective. If there is another resolution for this situation, cargo should suggest that.

I also don't think #12235 is going to be the answer because it's a year old and obviously isn't going to happen soon.

The current situation is a regression.

I would suggest not to check Cargo's stderr, as it is generally for human and subject to change at any time.

That's all very well, but I need to test the stderr output of a proc macro, which can't realistically be done without invoking cargo and rustc.

If I don't get a better resolution I guess I'm going to be writing horrible code to filter out this nuisance warning with regexps, or something. I would very much like to avoid that.

@ijackson
Copy link
Contributor Author

I went to fix this. UTSL, and I find

                // We don't want to print a warning if the version
                // without the extension is just a symlink to the version
                // WITH an extension, which people may want to do to
                // support multiple Cargo versions at once and not
                // get a warning.
                let skip_warning = if let Ok(target_path) = fs::read_link(&possible) {
                    target_path == possible_with_extension
                } else {
                    false
                };

which is buggy. I will fix it.

ijackson added a commit to ijackson/cargo that referenced this issue Apr 24, 2024
This is 100% reliable on Unix, and better on Windows.

(In this commit I avoid reindenting things to make review easier; the
formatting will be fixed in the next commit.)

Fixes rust-lang#13667
ijackson added a commit to ijackson/cargo that referenced this issue Apr 24, 2024
This is 100% reliable on Unix, and better on Windows.

(In this commit I avoid reindenting things to make review easier; the
formatting will be fixed in the next commit.)

Fixes rust-lang#13667
ijackson added a commit to ijackson/cargo that referenced this issue Apr 24, 2024
This is 100% reliable on Unix, and better on Windows.

(In this commit I avoid reindenting things to make review easier; the
formatting will be fixed in the next commit.)

Fixes rust-lang#13667
bors added a commit that referenced this issue Apr 24, 2024
Fix warning suppression for config.toml vs config compat symlinks

### What does this PR try to resolve?

Background: the cargo config file is being renamed from `.cargo/config` to `.cargo/config.toml`.  There's code in new cargo to look for both files (for compatibility), to issue a warning when onliy the old filename is found, and also to issue a warning if both files are found.  The warning suggests making a symlink if compatibility with old cargo is wanted.

An attempt was made to detect when both the old and new files exists, but one is a symlink to the other, but as reported in #13667, this code is not effective.  (It would work only if the symlink had the precise absolute pathname that cargo has decided to use for the lookup, which would be an unnatural way to make the link.)

Logically, the warning should appear when both files exist *but are different*.  That is the anomalous situation that will generate confusing behaviour.   By "different" we ought to mean "aren't the very same file".

That's what this MR implements, where possible.  On Unix, we use the information from stat(2).  That's not available on other platforms; on those, we arrange to also tolerate a symlink referring to precisely `config.toml` as a relative pathname, which is also fine, since by definition the target is then in the same directrory as `config`.

Fixes #13667.

### How should we test and review this PR?

I have interleaved the new tests with the commits that support them.  In each case, a functional commit is followed by a test which fails just beforehand.

(This can be observed by experimentally reordering the branch.)

I have also done ad-hoc testing.

### Additional information

I'm making the assumption that a symlink containing a relative path does something sane on Windows.  This assumption may be unwarranted.  If so, "Handle `config` -> `config.toml` (without full path)" needs to be dropped, and the test case needs to be `#[cfg(unix)]`.

But also, in this case, we should probably put some warnings in the stdlib docs!
bors added a commit that referenced this issue Apr 24, 2024
Fix warning suppression for config.toml vs config compat symlinks

### What does this PR try to resolve?

Background: the cargo config file is being renamed from `.cargo/config` to `.cargo/config.toml`.  There's code in new cargo to look for both files (for compatibility), to issue a warning when onliy the old filename is found, and also to issue a warning if both files are found.  The warning suggests making a symlink if compatibility with old cargo is wanted.

An attempt was made to detect when both the old and new files exists, but one is a symlink to the other, but as reported in #13667, this code is not effective.  (It would work only if the symlink had the precise absolute pathname that cargo has decided to use for the lookup, which would be an unnatural way to make the link.)

Logically, the warning should appear when both files exist *but are different*.  That is the anomalous situation that will generate confusing behaviour.   By "different" we ought to mean "aren't the very same file".

That's what this MR implements, where possible.  On Unix, we use the information from stat(2).  That's not available on other platforms; on those, we arrange to also tolerate a symlink referring to precisely `config.toml` as a relative pathname, which is also fine, since by definition the target is then in the same directrory as `config`.

Fixes #13667.

### How should we test and review this PR?

I have interleaved the new tests with the commits that support them.  In each case, a functional commit is followed by a test which fails just beforehand.

(This can be observed by experimentally reordering the branch.)

I have also done ad-hoc testing.

### Additional information

I'm making the assumption that a symlink containing a relative path does something sane on Windows.  This assumption may be unwarranted.  If so, "Handle `config` -> `config.toml` (without full path)" needs to be dropped, and the test case needs to be `#[cfg(unix)]`.

But also, in this case, we should probably put some warnings in the stdlib docs!
@bors bors closed this as completed in 23440c0 Apr 24, 2024
@0xbillw
Copy link

0xbillw commented May 17, 2024

I solved this problem by renaming .cargo/config to .cargo/config.toml.

@sj7510
Copy link

sj7510 commented Sep 30, 2024

I solved this problem by renaming .cargo/config to .cargo/config.toml.

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself. C-bug Category: bug S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants