Skip to content

Commit 445fe4a

Browse files
authored
fix(depinfo): prevent invalid trailing backslash on Windows (#16223)
### What does this PR try to resolve? Fixes #16096. Finally found some time to trace the issue to its roots: cargo tries to convert dep paths to relative ones. If a path is either 1. `pkg_root` or `build_root` itself (see `DepInfoPathType::PackageRootRelative` and `DepInfoPathType::BuildRootRelative`), causing the `EncodedDepInfo` to store an empty path, or 2. an explicitly empty path is provided (e.g. via `println!("cargo::rerun-if-changed=");`), then cargo will join the respecive root paths with an empty path. Joining with an empty path [adds a trailing path separator](https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=d36571ed84f4cd4476097050ff167f34). On systems with a `/` main separator, this works fine. On Windows, however, this adds a trailing backslash. Trailing backslashes are incompatible with `.d` dep file paths. This PR adds the necessary checks and ensures that instead of `foo.join("")` we return `foo` (instead of effectively `foo + std::path::MAIN_SEPARATOR_STR`). Importantly, this PR does not change the behavior for any other paths passed in (e.g. paths explicitly ending in a backslash), and only focuses on unintentional backslashes outside of the user's control. ### How to test and review this PR? The first commit shows the unintended behavior in two tests; the second commit fixes the issue and alters the tests to reflect the new behavior.
2 parents 77f0841 + b4b38e1 commit 445fe4a

File tree

3 files changed

+124
-7
lines changed

3 files changed

+124
-7
lines changed

src/cargo/core/compiler/fingerprint/dep_info.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -503,11 +503,19 @@ fn make_absolute_path(
503503
build_root: &Path,
504504
path: PathBuf,
505505
) -> PathBuf {
506-
match ty {
507-
DepInfoPathType::PackageRootRelative => pkg_root.join(path),
508-
// N.B. path might be absolute here in which case the join will have no effect
509-
DepInfoPathType::BuildRootRelative => build_root.join(path),
506+
let relative_to = match ty {
507+
DepInfoPathType::PackageRootRelative => pkg_root,
508+
// N.B. path might be absolute here in which case the join below will have no effect
509+
DepInfoPathType::BuildRootRelative => build_root,
510+
};
511+
512+
if path.as_os_str().is_empty() {
513+
// Joining with an empty path causes Rust to add a trailing path separator. On Windows, this
514+
// would add an invalid trailing backslash to the .d file.
515+
return relative_to.to_path_buf();
510516
}
517+
518+
relative_to.join(path)
511519
}
512520

513521
/// Some algorithms are here to ensure compatibility with possible rustc outputs.

src/cargo/core/compiler/output_depinfo.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,18 @@ fn add_deps_for_unit(
8484
.get(metadata)
8585
{
8686
for path in &output.rerun_if_changed {
87-
// The paths we have saved from the unit are of arbitrary relativeness and may be
88-
// relative to the crate root of the dependency.
89-
let path = unit.pkg.root().join(path);
87+
let package_root = unit.pkg.root();
88+
89+
let path = if path.as_os_str().is_empty() {
90+
// Joining with an empty path causes Rust to add a trailing path separator.
91+
// On Windows, this would add an invalid trailing backslash to the .d file.
92+
package_root.to_path_buf()
93+
} else {
94+
// The paths we have saved from the unit are of arbitrary relativeness and
95+
// may be relative to the crate root of the dependency.
96+
package_root.join(path)
97+
};
98+
9099
deps.insert(path);
91100
}
92101
}

tests/testsuite/dep_info.rs

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -552,3 +552,103 @@ fn non_local_build_script() {
552552
"#]],
553553
);
554554
}
555+
556+
#[cargo_test]
557+
fn no_trailing_separator_after_package_root_build_script() {
558+
let p = project()
559+
.file(
560+
"Cargo.toml",
561+
r#"
562+
[package]
563+
name = "foo"
564+
version = "0.0.1"
565+
authors = []
566+
"#,
567+
)
568+
.file("src/main.rs", "fn main() {}")
569+
.file(
570+
"build.rs",
571+
r#"
572+
fn main() {
573+
println!("cargo::rerun-if-changed=");
574+
}
575+
"#,
576+
)
577+
.build();
578+
579+
p.cargo("build").run();
580+
let contents = p.read_file("target/debug/foo.d");
581+
582+
assert_e2e().eq(
583+
&contents,
584+
str![[r#"
585+
[ROOT]/foo/target/debug/foo[EXE]: [ROOT]/foo [ROOT]/foo/build.rs [ROOT]/foo/src/main.rs
586+
587+
"#]],
588+
);
589+
}
590+
591+
#[cargo_test(nightly, reason = "proc_macro::tracked_path is unstable")]
592+
fn no_trailing_separator_after_package_root_proc_macro() {
593+
let p = project()
594+
.file(
595+
"Cargo.toml",
596+
r#"
597+
[package]
598+
name = "foo"
599+
version = "0.0.1"
600+
authors = []
601+
edition = "2018"
602+
603+
[dependencies]
604+
pm = { path = "pm" }
605+
"#,
606+
)
607+
.file(
608+
"src/main.rs",
609+
"
610+
pm::noop!{}
611+
fn main() {}
612+
",
613+
)
614+
.file(
615+
"pm/Cargo.toml",
616+
r#"
617+
[package]
618+
name = "pm"
619+
version = "0.1.0"
620+
edition = "2018"
621+
622+
[lib]
623+
proc-macro = true
624+
"#,
625+
)
626+
.file(
627+
"pm/src/lib.rs",
628+
r#"
629+
#![feature(track_path)]
630+
extern crate proc_macro;
631+
use proc_macro::TokenStream;
632+
633+
#[proc_macro]
634+
pub fn noop(_item: TokenStream) -> TokenStream {
635+
proc_macro::tracked_path::path(
636+
std::env::current_dir().unwrap().to_str().unwrap()
637+
);
638+
"".parse().unwrap()
639+
}
640+
"#,
641+
)
642+
.build();
643+
644+
p.cargo("build").run();
645+
let contents = p.read_file("target/debug/foo.d");
646+
647+
assert_e2e().eq(
648+
&contents,
649+
str![[r#"
650+
[ROOT]/foo/target/debug/foo[EXE]: [ROOT]/foo [ROOT]/foo/pm/src/lib.rs [ROOT]/foo/src/main.rs
651+
652+
"#]],
653+
);
654+
}

0 commit comments

Comments
 (0)