Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/cargo/util/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1678,13 +1678,17 @@ impl GlobalContext {
if let Some(path) = self.get_file_path(&config_root, "config", true)? {
walk(&path)?;
}
seen_dir.insert(config_root);

let canonical_root = config_root.canonicalize().unwrap_or(config_root);
seen_dir.insert(canonical_root);
}

let canonical_home = home.canonicalize().unwrap_or(home.to_path_buf());

// Once we're done, also be sure to walk the home directory even if it's not
// in our history to be sure we pick up that standard location for
// information.
if !seen_dir.contains(home) {
if !seen_dir.contains(&canonical_home) && !seen_dir.contains(home) {
if let Some(path) = self.get_file_path(home, "config", true)? {
walk(&path)?;
}
Expand Down
60 changes: 60 additions & 0 deletions tests/testsuite/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2589,3 +2589,63 @@ fn mixed_type_array() {
}
);
}

#[cargo_test]
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the update!

FWIW, for bugfixes, we usually follow a variant of atomic commit pattern:

  1. Commit a test that asserts the current buggy behavior (passes).
  2. In the next commit, fix the bug and update the test/snapshot.

Every commit passes, and the test/snapshot diff shows the behavior change. And the first commit could be seen as a minimal reproducible example, to ensure we are fixing a real issue.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for clarifying :) This should be fine now?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah that is pretty much what we want. Here is a new suggestion I think we could make it better: #16325 (comment).

fn config_symlink_home_duplicate_load() {
// Test that when CARGO_HOME is accessed via a symlink that points to a directory
// already in the config search path, the config file is not loaded twice.

use cargo_test_support::basic_manifest;

#[cfg(unix)]
use std::os::unix::fs::symlink;

#[cfg(windows)]
use std::os::windows::fs::symlink_dir as symlink;

let p = project()
.file("Cargo.toml", &basic_manifest("foo", "0.1.0"))
.file("src/lib.rs", "")
.build();

// Create directory structure a/b/ and symlink c -> a
let a_dir = p.root().join("a");
let b_dir = a_dir.join("b");
let c_symlink = p.root().join("c");

fs::create_dir_all(&b_dir).unwrap();
symlink(&a_dir, &c_symlink).unwrap();

// Create config file in a/.cargo/
let cargo_config_dir = a_dir.join(".cargo");
fs::create_dir(&cargo_config_dir).unwrap();
let config_path = cargo_config_dir.join("config.toml");
fs::write(
&config_path,
r#"
[build]
rustdocflags = ["--default-theme=dark"]
"#,
)
.unwrap();

// Move the project into a/b/
let project_in_b = b_dir.join("foo");
fs::create_dir(&project_in_b).unwrap();
fs::write(
project_in_b.join("Cargo.toml"),
&basic_manifest("foo", "0.1.0"),
)
.unwrap();
fs::create_dir(project_in_b.join("src")).unwrap();
fs::write(project_in_b.join("src/lib.rs"), "").unwrap();

// Set CARGO_HOME to ../../c/.cargo (which is really a/.cargo via symlink)
let cargo_home = c_symlink.join(".cargo");

// If config is loaded twice, rustdocflags will be duplicated and cause an error
p.cargo("doc")
.cwd(&project_in_b)
.env("CARGO_HOME", &cargo_home)
.run();
}