Skip to content

Commit

Permalink
Auto merge of #13349 - epage:config, r=weihanglo
Browse files Browse the repository at this point in the history
fix(config): Deprecate non-extension files

### What does this PR try to resolve?

In #7295 (released in 1.39), we said we'd want to warn on use of
`.cargo/config` after about 6 months.  Over 4 years later, we are now
getting that warning.

This is important for addressing user confusion, like in
https://www.reddit.com/r/rust/comments/19fd5q2/cargoconfig/

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

It'll be important to look at the individual commits as one updates tests from using `.cargo/config` to `.cargo/config.toml` which touches a lot of code.

I added a test for `.cargo/config` in a separate commit so you can see how the output changes.

### Additional information

Discussed on [zulip](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Deprecating.20credential.20provider.20default.3F)
  • Loading branch information
bors committed Jan 26, 2024
2 parents f4bafa7 + 6eb2dde commit 3a72bf3
Show file tree
Hide file tree
Showing 45 changed files with 359 additions and 330 deletions.
4 changes: 2 additions & 2 deletions crates/cargo-test-support/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ impl RegistryBuilder {
/// Initializes the registry.
#[must_use]
pub fn build(self) -> TestRegistry {
let config_path = paths::home().join(".cargo/config");
let config_path = paths::home().join(".cargo/config.toml");
t!(fs::create_dir_all(config_path.parent().unwrap()));
let prefix = if let Some(alternative) = &self.alternative {
format!("{alternative}-")
Expand Down Expand Up @@ -1195,7 +1195,7 @@ impl Package {
/// Creates a new package builder.
/// Call `publish()` to finalize and build the package.
pub fn new(name: &str, vers: &str) -> Package {
let config = paths::home().join(".cargo/config");
let config = paths::home().join(".cargo/config.toml");
if !config.exists() {
init();
}
Expand Down
24 changes: 17 additions & 7 deletions src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1507,7 +1507,7 @@ impl Config {
let possible_with_extension = dir.join(format!("{}.toml", filename_without_extension));

if possible.exists() {
if warn && possible_with_extension.exists() {
if warn {
// 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
Expand All @@ -1520,12 +1520,22 @@ impl Config {
};

if !skip_warning {
self.shell().warn(format!(
"Both `{}` and `{}` exist. Using `{}`",
possible.display(),
possible_with_extension.display(),
possible.display()
))?;
if possible_with_extension.exists() {
self.shell().warn(format!(
"Both `{}` and `{}` exist. Using `{}`",
possible.display(),
possible_with_extension.display(),
possible.display()
))?;
} else {
self.shell().warn(format!(
"`{}` is deprecated in favor of `{filename_without_extension}.toml`",
possible.display(),
))?;
self.shell().note(
format!("If you need to support cargo 1.38 or earlier, you can symlink `{filename_without_extension}` to `{filename_without_extension}.toml`"),
)?;
}
}
}

Expand Down
12 changes: 6 additions & 6 deletions tests/testsuite/alt_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ You may press ctrl-c to skip waiting; the crate should be available shortly.
fn passwords_in_registries_index_url_forbidden() {
registry::alt_init();

let config = paths::home().join(".cargo/config");
let config = paths::home().join(".cargo/config.toml");

fs::write(
config,
Expand All @@ -638,7 +638,7 @@ fn passwords_in_registries_index_url_forbidden() {
.with_status(101)
.with_stderr(
"\
error: invalid index URL for registry `alternative` defined in [..]/home/.cargo/config
error: invalid index URL for registry `alternative` defined in [..]/home/.cargo/config.toml
Caused by:
registry URLs may not contain passwords
Expand Down Expand Up @@ -1153,7 +1153,7 @@ fn unknown_registry() {
.publish();

// Remove "alternative" from config.
let cfg_path = paths::home().join(".cargo/config");
let cfg_path = paths::home().join(".cargo/config.toml");
let mut config = fs::read_to_string(&cfg_path).unwrap();
let start = config.find("[registries.alternative]").unwrap();
config.insert(start, '#');
Expand Down Expand Up @@ -1296,7 +1296,7 @@ fn unknown_registry() {
#[cargo_test]
fn registries_index_relative_url() {
registry::alt_init();
let config = paths::root().join(".cargo/config");
let config = paths::root().join(".cargo/config.toml");
fs::create_dir_all(config.parent().unwrap()).unwrap();
fs::write(
&config,
Expand Down Expand Up @@ -1343,7 +1343,7 @@ fn registries_index_relative_url() {
#[cargo_test]
fn registries_index_relative_path_not_allowed() {
registry::alt_init();
let config = paths::root().join(".cargo/config");
let config = paths::root().join(".cargo/config.toml");
fs::create_dir_all(config.parent().unwrap()).unwrap();
fs::write(
&config,
Expand Down Expand Up @@ -1379,7 +1379,7 @@ fn registries_index_relative_path_not_allowed() {
error: failed to parse manifest at `{root}/foo/Cargo.toml`
Caused by:
invalid index URL for registry `relative` defined in [..]/.cargo/config
invalid index URL for registry `relative` defined in [..]/.cargo/config.toml
Caused by:
invalid url `alternative-registry`: relative URL without a base
Expand Down
64 changes: 32 additions & 32 deletions tests/testsuite/bad_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ fn bad1() {
let p = project()
.file("src/lib.rs", "")
.file(
".cargo/config",
".cargo/config.toml",
r#"
[target]
nonexistent-target = "foo"
Expand All @@ -21,7 +21,7 @@ fn bad1() {
.with_stderr(
"\
[ERROR] expected table for configuration key `target.nonexistent-target`, \
but found string in [..]/config
but found string in [..]/config.toml
",
)
.run();
Expand All @@ -32,7 +32,7 @@ fn bad2() {
let p = project()
.file("src/lib.rs", "")
.file(
".cargo/config",
".cargo/config.toml",
r#"
[http]
proxy = 3.0
Expand All @@ -46,7 +46,7 @@ fn bad2() {
[ERROR] could not load Cargo configuration
Caused by:
failed to load TOML configuration from `[..]config`
failed to load TOML configuration from `[..]config.toml`
Caused by:
failed to parse key `http`
Expand All @@ -67,7 +67,7 @@ fn bad3() {
let p = project()
.file("src/lib.rs", "")
.file(
".cargo/config",
".cargo/config.toml",
r#"
[http]
proxy = true
Expand All @@ -84,7 +84,7 @@ fn bad3() {
error: failed to update registry [..]
Caused by:
error in [..]config: `http.proxy` expected a string, but found a boolean
error in [..]config.toml: `http.proxy` expected a string, but found a boolean
",
)
.run();
Expand All @@ -94,7 +94,7 @@ Caused by:
fn bad4() {
let p = project()
.file(
".cargo/config",
".cargo/config.toml",
r#"
[cargo-new]
vcs = false
Expand All @@ -108,7 +108,7 @@ fn bad4() {
[ERROR] Failed to create package `foo` at `[..]`
Caused by:
error in [..]config: `cargo-new.vcs` expected a string, but found a boolean
error in [..]config.toml: `cargo-new.vcs` expected a string, but found a boolean
",
)
.run();
Expand All @@ -120,7 +120,7 @@ fn bad6() {
let p = project()
.file("src/lib.rs", "")
.file(
".cargo/config",
".cargo/config.toml",
r#"
[http]
user-agent = true
Expand All @@ -137,7 +137,7 @@ fn bad6() {
error: failed to update registry [..]
Caused by:
error in [..]config: `http.user-agent` expected a string, but found a boolean
error in [..]config.toml: `http.user-agent` expected a string, but found a boolean
",
)
.run();
Expand All @@ -158,7 +158,7 @@ fn invalid_global_config() {
foo = "0.1.0"
"#,
)
.file(".cargo/config", "4")
.file(".cargo/config.toml", "4")
.file("src/lib.rs", "")
.build();

Expand Down Expand Up @@ -788,7 +788,7 @@ or workspace dependency to use. This will be considered an error in future versi
#[cargo_test]
fn invalid_toml_historically_allowed_fails() {
let p = project()
.file(".cargo/config", "[bar] baz = 2")
.file(".cargo/config.toml", "[bar] baz = 2")
.file("src/main.rs", "fn main() {}")
.build();

Expand Down Expand Up @@ -880,7 +880,7 @@ use `rev = \"foo\"` in the dependency declaration.
fn bad_source_config1() {
let p = project()
.file("src/lib.rs", "")
.file(".cargo/config", "[source.foo]")
.file(".cargo/config.toml", "[source.foo]")
.build();

p.cargo("check")
Expand All @@ -906,7 +906,7 @@ fn bad_source_config2() {
)
.file("src/lib.rs", "")
.file(
".cargo/config",
".cargo/config.toml",
r#"
[source.crates-io]
registry = 'http://example.com'
Expand Down Expand Up @@ -952,7 +952,7 @@ fn bad_source_config3() {
)
.file("src/lib.rs", "")
.file(
".cargo/config",
".cargo/config.toml",
r#"
[source.crates-io]
registry = 'https://example.com'
Expand Down Expand Up @@ -997,7 +997,7 @@ fn bad_source_config4() {
)
.file("src/lib.rs", "")
.file(
".cargo/config",
".cargo/config.toml",
r#"
[source.crates-io]
replace-with = 'bar'
Expand Down Expand Up @@ -1046,7 +1046,7 @@ fn bad_source_config5() {
)
.file("src/lib.rs", "")
.file(
".cargo/config",
".cargo/config.toml",
r#"
[source.crates-io]
registry = 'https://example.com'
Expand Down Expand Up @@ -1120,7 +1120,7 @@ fn bad_source_config6() {
)
.file("src/lib.rs", "")
.file(
".cargo/config",
".cargo/config.toml",
r#"
[source.crates-io]
registry = 'https://example.com'
Expand All @@ -1133,10 +1133,10 @@ fn bad_source_config6() {
.with_status(101)
.with_stderr(
"\
[ERROR] error in [..]/foo/.cargo/config: could not load config key `source.crates-io.replace-with`
[ERROR] error in [..]/foo/.cargo/config.toml: could not load config key `source.crates-io.replace-with`
Caused by:
error in [..]/foo/.cargo/config: `source.crates-io.replace-with` expected a string, but found a array
error in [..]/foo/.cargo/config.toml: `source.crates-io.replace-with` expected a string, but found a array
"
)
.run();
Expand Down Expand Up @@ -1207,7 +1207,7 @@ fn bad_source_config7() {
)
.file("src/lib.rs", "")
.file(
".cargo/config",
".cargo/config.toml",
r#"
[source.foo]
registry = 'https://example.com'
Expand Down Expand Up @@ -1241,7 +1241,7 @@ fn bad_source_config8() {
)
.file("src/lib.rs", "")
.file(
".cargo/config",
".cargo/config.toml",
r#"
[source.foo]
branch = "somebranch"
Expand All @@ -1253,7 +1253,7 @@ fn bad_source_config8() {
.with_status(101)
.with_stderr(
"[ERROR] source definition `source.foo` specifies `branch`, \
but that requires a `git` key to be specified (in [..]/foo/.cargo/config)",
but that requires a `git` key to be specified (in [..]/foo/.cargo/config.toml)",
)
.run();
}
Expand Down Expand Up @@ -1532,7 +1532,7 @@ fn bad_target_cfg() {
// the message.
let p = project()
.file(
".cargo/config",
".cargo/config.toml",
r#"
[target.'cfg(not(target_os = "none"))']
runner = false
Expand All @@ -1545,17 +1545,17 @@ fn bad_target_cfg() {
.with_status(101)
.with_stderr(
"\
[ERROR] error in [..]/foo/.cargo/config: \
[ERROR] error in [..]/foo/.cargo/config.toml: \
could not load config key `target.\"cfg(not(target_os = \\\"none\\\"))\".runner`
Caused by:
error in [..]/foo/.cargo/config: \
error in [..]/foo/.cargo/config.toml: \
could not load config key `target.\"cfg(not(target_os = \\\"none\\\"))\".runner`
Caused by:
invalid configuration for key `target.\"cfg(not(target_os = \\\"none\\\"))\".runner`
expected a string or array of strings, but found a boolean for \
`target.\"cfg(not(target_os = \\\"none\\\"))\".runner` in [..]/foo/.cargo/config
`target.\"cfg(not(target_os = \\\"none\\\"))\".runner` in [..]/foo/.cargo/config.toml
",
)
.run();
Expand All @@ -1571,7 +1571,7 @@ fn bad_target_links_overrides() {
// currently is designed with serde.
let p = project()
.file(
".cargo/config",
".cargo/config.toml",
&format!(
r#"
[target.{}.somelib]
Expand All @@ -1587,12 +1587,12 @@ fn bad_target_links_overrides() {
.with_status(101)
.with_stderr(
"[ERROR] Only `-l` and `-L` flags are allowed in target config \
`target.[..].rustc-flags` (in [..]foo/.cargo/config): `foo`",
`target.[..].rustc-flags` (in [..]foo/.cargo/config.toml): `foo`",
)
.run();

p.change_file(
".cargo/config",
".cargo/config.toml",
&format!(
"[target.{}.somelib]
warning = \"foo\"
Expand All @@ -1611,7 +1611,7 @@ fn redefined_sources() {
// Cannot define a source multiple times.
let p = project()
.file(
".cargo/config",
".cargo/config.toml",
r#"
[source.foo]
registry = "https://github.com/rust-lang/crates.io-index"
Expand All @@ -1632,7 +1632,7 @@ note: Sources are not allowed to be defined multiple times.
.run();

p.change_file(
".cargo/config",
".cargo/config.toml",
r#"
[source.one]
directory = "index"
Expand Down
Loading

0 comments on commit 3a72bf3

Please sign in to comment.