warning: output filename collision.

Wyvern commented 1 month ago

Problem

Got lots of warnings like below

warning: output filename collision.
The lib target `cfg_if` in package `cfg-if v1.0.0` has the same output filename as the lib target `cfg_if` in package `cfg-if v1.0.0`.
Colliding filename is: target/debug/deps/libcfg_if-b07c5e93d2d15f7e.rlib
The targets should have unique names.
Consider changing their names to be unique or compiling them separately.
This may become a hard error in the future; see <https://github.com/rust-lang/cargo/issues/6313>.

Steps

no warnings:

stable
cargo 1.81.0-nightly (a515d4634 2024-07-02)

warnings:

cargo 1.81.0-nightly (154fdac39 2024-07-07) start to pop up warnings.

Possible Solution(s)

No response

Notes

Version

nightly-x86_64-apple-darwin

epage commented 1 month ago

Could you provide reproduction steps?

Wyvern commented 1 month ago

I don't know what lead to this warnings, compiling almost all of my rust projects using latest cargo version cargo buildwill pop up some warnings.

For example for one project, the full list of warnings is:

warning: output filename collision.
The lib target `cfg_if` in package `cfg-if v1.0.0` has the same output filename as the lib target `cfg_if` in package `cfg-if v1.0.0`.
Colliding filename is: /debug/deps/libcfg_if-b07c5e93d2d15f7e.rlib
The targets should have unique names.
Consider changing their names to be unique or compiling them separately.
This may become a hard error in the future; see <https://github.com/rust-lang/cargo/issues/6313>.
warning: output filename collision.
The lib target `cfg_if` in package `cfg-if v1.0.0` has the same output filename as the lib target `cfg_if` in package `cfg-if v1.0.0`.
Colliding filename is: /debug/deps/libcfg_if-b07c5e93d2d15f7e.rmeta
The targets should have unique names.
Consider changing their names to be unique or compiling them separately.
This may become a hard error in the future; see <https://github.com/rust-lang/cargo/issues/6313>.
warning: output filename collision.
The lib target `phf_shared` in package `phf_shared v0.10.0` has the same output filename as the lib target `phf_shared` in package `phf_shared v0.10.0`.
Colliding filename is: /debug/deps/libphf_shared-e4838e974e16f5df.rlib
The targets should have unique names.
Consider changing their names to be unique or compiling them separately.
This may become a hard error in the future; see <https://github.com/rust-lang/cargo/issues/6313>.
warning: output filename collision.
The lib target `phf_shared` in package `phf_shared v0.10.0` has the same output filename as the lib target `phf_shared` in package `phf_shared v0.10.0`.
Colliding filename is: /debug/deps/libphf_shared-e4838e974e16f5df.rmeta
The targets should have unique names.
Consider changing their names to be unique or compiling them separately.
This may become a hard error in the future; see <https://github.com/rust-lang/cargo/issues/6313>.
warning: output filename collision.
The lib target `siphasher` in package `siphasher v0.3.11` has the same output filename as the lib target `siphasher` in package `siphasher v0.3.11`.
Colliding filename is: /debug/deps/libsiphasher-f5cde03e2e9cf64a.rlib
The targets should have unique names.
Consider changing their names to be unique or compiling them separately.
This may become a hard error in the future; see <https://github.com/rust-lang/cargo/issues/6313>.
warning: output filename collision.
The lib target `siphasher` in package `siphasher v0.3.11` has the same output filename as the lib target `siphasher` in package `siphasher v0.3.11`.
Colliding filename is: /debug/deps/libsiphasher-f5cde03e2e9cf64a.rmeta
The targets should have unique names.
Consider changing their names to be unique or compiling them separately.
This may become a hard error in the future; see <https://github.com/rust-lang/cargo/issues/6313>.

As you see, every warning duplicate itself; and why only compiling cfg_if, phf_shared, siphasher generate warning but others not.

epage commented 1 month ago

Also, on top of reproduction steps, can you reproduce this on stable Rust? If only testing with nightlies, happen to know what was the last nightly you used without these warnings?

Wyvern commented 1 month ago

Stable Rust is fine.

As noted in the original post,

cargo 1.81.0-nightly (https://github.com/rust-lang/cargo/commit/a515d463427b3912ec0365d106791f88c1c14e1b 2024-07-02) works fine without any warnings

cargo 1.81.0-nightly (https://github.com/rust-lang/cargo/commit/154fdac39ae9629954e19e9986fd2cf2cdd8d964 2024-07-07) start to pop up warnings.

Wyvern commented 1 month ago

After some investigation, found this warning is relate to Cargo's

[unstable]
target-applies-to-host = true
host-config = true

comment them out warning disappears.

aeubanks commented 2 weeks ago

We're also seeing this after https://github.com/rust-lang/rust/pull/128083, the exact symptoms mentioned in #7859. We also have a similar cargo.config, and not using that config makes the build succeed.

I could reproduce this by setting the CARGO_HOME env var to a directory containing the following config.toml:

[unstable]
target-applies-to-host = true
host-config = true

and running CARGO_HOME="/tmp/asdf" ./x.py build

weihanglo commented 2 weeks ago

That behavior might come from https://github.com/rust-lang/cargo/pull/13900. I need to dig deeper.

and not using that config makes the build succeed.

@aeubanks Do you mean that you are blocked due to this "bug"? Could you provide a minimal reproducion so I can look into?

aeubanks commented 2 weeks ago

Yes we are blocked on upgrading Rust due to this. I don't have a more minimal repro than what I described above. I suspect that it will require building the standard library anyway, as opposed to some smaller standalone project.

aeubanks commented 2 weeks ago

Sorry, to clarify, this is building https://github.com/rust-lang/rust

chbaker0 commented 2 weeks ago

+1 to the build issue @aeubanks mentions. The repro steps are:

Make an empty directory, create file config.toml in it with the following contents:

[unstable]
target-applies-to-host = true
host-config = true

In a checkout of https://github.com/rust-lang/rust run CARGO_HOME=<dir from above> ./x.py build --stage 0 library/std.

I'm observing this error on x86_64-unknown-linux-gnu. In fact, just about any invocation of x.py will work because the error occurs when building bootstrap.

I will try to create a smaller repro that doesn't rely on an entire Rust checkout.

chbaker0 commented 2 weeks ago

From inside the checkout, in the package that fails to build:

~/repos/rust/src/bootstrap$ cargo tree -i cc
cc v1.0.97
├── bootstrap v0.0.0 (~/repos/rust/src/bootstrap)
└── cmake v0.1.48
    └── bootstrap v0.0.0 (~/repos/rust/src/bootstrap)
[build-dependencies]
└── lzma-sys v0.1.20
    └── xz2 v0.1.7
        └── bootstrap v0.0.0 (~/repos/rust/src/bootstrap)

cc is depended on as both a (transitive) build-dependency and as a regular dependency in the same build. I can't help but wonder if this is a regression related to target-applies-to-host

chbaker0 commented 2 weeks ago

@rustbot label: Z-target-applies-to-host

alecmocatta commented 2 weeks ago

We're also seeing it with this config:

target-applies-to-host = false

[unstable]
bindeps = true
host-config = true
target-applies-to-host = true

weihanglo commented 2 weeks ago

The error can be reproduced with the follow steps in rust-lang/rust.

Check out https://github.com/rust-lang/rust/commit/53676730146e38e4697b6204c2ee61a9fd6b7e51

Set ~/.cargo/config.toml with the follow configuration:

[unstable]
target-applies-to-host = true
host-config = true

Run ./x.py in the root of rust-lang/rust repo. The first invocation should pass.

Run again ./x.py. This time it should fail with the output:

Building bootstrap
warning: output filename collision.
The lib target `cc` in package `cc v1.0.97` has the same output filename as the lib target `cc` in package `cc v1.0.97`.
Colliding filename is: /Users/user/dev/rust/build/bootstrap/debug/deps/libcc-0ac4eaa3c8400ccd.rlib
The targets should have unique names.
Consider changing their names to be unique or compiling them separately.
This may become a hard error in the future; see <https://github.com/rust-lang/cargo/issues/6313>.
warning: output filename collision.
The lib target `cc` in package `cc v1.0.97` has the same output filename as the lib target `cc` in package `cc v1.0.97`.
Colliding filename is: /Users/user/dev/rust/build/bootstrap/debug/deps/libcc-0ac4eaa3c8400ccd.rmeta
The targets should have unique names.
Consider changing their names to be unique or compiling them separately.
This may become a hard error in the future; see <https://github.com/rust-lang/cargo/issues/6313>.
  Compiling cc v1.0.97
  Compiling lzma-sys v0.1.20
  Compiling xz2 v0.1.7
  Compiling bootstrap v0.0.0 (/Users/user/dev/rust/src/bootstrap)
error[E0460]: found possibly newer version of crate `cc` which `cmake` depends on
  --> src/core/build_steps/llvm.rs:306:23
   |
306 |         let mut cfg = cmake::Config::new(builder.src.join(root));
   |                       ^^^^^
   |
   = note: perhaps that crate needs to be recompiled?
   = note: the following crate versions were found:
           crate `cc`: /Users/user/dev/rust/build/bootstrap/debug/deps/libcc-0ac4eaa3c8400ccd.rmeta
           crate `cc`: /Users/user/dev/rust/build/bootstrap/debug/deps/libcc-0ac4eaa3c8400ccd.rlib
           crate `cmake`: /Users/user/dev/rust/build/bootstrap/debug/deps/libcmake-c88aa06d9a5ce765.rmeta

error[E0463]: can't find crate for `cmake`
  --> src/core/build_steps/llvm.rs:594:15
   |
594 |     cfg: &mut cmake::Config,
   |               ^^^^^ can't find crate

error[E0463]: can't find crate for `cmake`
  --> src/core/build_steps/llvm.rs:817:77
   |
817 | ...ection, cfg: &mut cmake::Config) {
   |                      ^^^^^ can't find crate

error[E0463]: can't find crate for `cmake`
  --> src/core/build_steps/llvm.rs:896:23
   |
896 | ...   let mut cfg = cmake::Config::new(builder.src.join("src/l...
   |                     ^^^^^ can't find crate

error[E0463]: can't find crate for `cmake`
   --> src/core/build_steps/llvm.rs:1019:23
    |
1019 |         let mut cfg = cmake::Config::new(&compiler_rt_dir);
    |                       ^^^^^ can't find crate

Some errors have detailed explanations: E0460, E0463.
For more information about an error, try `rustc --explain E0460`.
error: could not compile `bootstrap` (lib) due to 5 previous errors
failed to run: /Users/user/dev/rust/build/aarch64-apple-darwin/stage0/bin/cargo build --manifest-path /Users/user/dev/rust/src/bootstrap/Cargo.toml

If you run ./x.py with stage0 cargo prior to this commit https://github.com/rust-lang/cargo/commit/8457356c31a4cf7fd16d7ed03877645637cb0535, it succeeds. So clearly there is something bad in 8457356c31a4cf7fd16d7ed03877645637cb0535.

weihanglo commented 2 weeks ago

@alecmocatta Are you also bootstrapping rustc, or can you share you project so we can look into what has happened?

weihanglo commented 2 weeks ago

Alright. Just narrowed bootstrap down to this real-world reproduction:

[package]
name = "cargo-14253"
version = "0.1.0"
edition = "2021"

[dependencies]
cc = "1.0.97"
lzma-sys = "0.1"

[profile.dev]
debug = 0

Run with

RUSTFLAGS="--cfg foo" cargo +beta -Zhost-config -Ztarget-applies-to-host build

and you'll get this compile error (it is not deterministic, sometimes succeeds):

warning: output filename collision.
The lib target `cc` in package `cc v1.1.7` has the same output filename as the lib target `cc` in package `cc v1.1.7`.
Colliding filename is: /home/user/projects/cargo-14253/target/debug/deps/libcc-abd516538b38f015.rlib
The targets should have unique names.
Consider changing their names to be unique or compiling them separately.
This may become a hard error in the future; see <https://github.com/rust-lang/cargo/issues/6313>.
warning: output filename collision.
The lib target `cc` in package `cc v1.1.7` has the same output filename as the lib target `cc` in package `cc v1.1.7`.
Colliding filename is: /home/user/projects/cargo-14253/target/debug/deps/libcc-abd516538b38f015.rmeta
The targets should have unique names.
Consider changing their names to be unique or compiling them separately.
This may become a hard error in the future; see <https://github.com/rust-lang/cargo/issues/6313>.
   Compiling libc v0.2.155
   Compiling cc v1.1.7
   Compiling pkg-config v0.3.30
   Compiling lzma-sys v0.1.20
error: failed to build archive at `/home/user/projects/cargo-14253/target/debug/deps/libcc-abd516538b38f015.rlib`: No such file or directory

error: could not compile `cc` (lib) due to 1 previous error
warning: build failed, waiting for other jobs to finish...

Note that RUSTFLAGS="--cfg foo" and [profile.dev] debug=0 are essential. They forced build (host) dependencies to be shared with normal dependencies.

For further investigation in Cargo here is a Cargo-flavored UI test reproduction:

click to expand

```rust #[cargo_test] fn host_config_shared_build_dep() { // rust-lang/cargo#14253 Package::new("cc", "1.0.0").publish(); Package::new("lzma-sys", "0.1.0") .file("src/lib.rs", "") .file("build.rs", "fn main() {}") .build_dep("cc", "1.0.0") .publish(); let p = project() .file( "Cargo.toml", r#" [package] name = "bootstrap" edition = "2021" [dependencies] cc = "1.0.0" lzma-sys = "0.1.0" [profile.dev] debug = 0 "#, ) .file("src/lib.rs", "") .build(); p.cargo("build") .masquerade_as_nightly_cargo(&["target-applies-to-host", "host-config"]) .arg("-Zhost-config") .arg("-Ztarget-applies-to-host") .env("RUSTFLAGS", "--cfg foo") .with_status(101) // Sometimes it compiles. Not deterministic... .with_stderr_data(str![r#" [UPDATING] `dummy-registry` index [LOCKING] 3 packages to latest compatible versions [DOWNLOADING] crates ... [DOWNLOADED] lzma-sys v0.1.0 (registry `dummy-registry`) [DOWNLOADED] cc v1.0.0 (registry `dummy-registry`) [WARNING] output filename collision. The lib target `cc` in package `cc v1.0.0` has the same output filename as the lib target `cc` in package `cc v1.0.0`. Colliding filename is: [ROOT]/foo/target/debug/deps/libcc-[HASH].rlib The targets should have unique names. Consider changing their names to be unique or compiling them separately. This may become a hard error in the future; see . [WARNING] output filename collision. The lib target `cc` in package `cc v1.0.0` has the same output filename as the lib target `cc` in package `cc v1.0.0`. Colliding filename is: [ROOT]/foo/target/debug/deps/libcc-[HASH].rmeta The targets should have unique names. Consider changing their names to be unique or compiling them separately. This may become a hard error in the future; see . [COMPILING] cc v1.0.0 [ERROR] failed to build archive: No such file or directory [ERROR] could not compile `cc` (lib) due to 1 previous error "#]) .run(); } ```

weihanglo commented 2 weeks ago

@alecmocatta @Wyvern Just I'd like to confirm my observation applies to your cases. Is there any extra profile setting in your package, such as [profile.dev] debug = 0 or [profile.dev.build-override] debug = true?

@chbaker0 @aeubanks Before the bad behavior is fixed or reverted, I think removing the debug = 0 profile setting in bootstrap should unblock you.

chbaker0 commented 2 weeks ago

Ah, I was struggling to make a self-contained repro and thought there was something unique to how bootstrap was built. Nice!

Wyvern commented 2 weeks ago

@weihanglo Yeah, [profile.dev] debug = 0 was set, and remove it can fix the warning for now even with cargo's

[unstable]
target-applies-to-host = true
host-config = true

settings.

But, if -Cdebuginfo=0 was set there is no problem; I think rustflags -Cdebuginfo=0 and profile based debug = 0 should be equivalent.

alecmocatta commented 2 weeks ago

@weihanglo

@alecmocatta Are you also bootstrapping rustc, or can you share you project so we can look into what has happened?

This occurred building a private repo that I unfortunately can't share.

@alecmocatta @Wyvern Just I'd like to confirm my observation applies to your cases. Is there any extra profile setting in your package, such as [profile.dev] debug = 0 or [profile.dev.build-override] debug = true?

debug is indeed in our .cargo/config.toml:

[profile.dev]
debug = "line-tables-only"

Reproduced most of the rest here in case anything else is helpful.

```toml target-applies-to-host = false [host] rustflags = [ "-Zoom=panic", "-Zproc-macro-backtrace", "-Zmacro-backtrace", "-Zthreads=8", ] [target.'cfg(all())'] rustflags = [ "-Zoom=panic", "-Zproc-macro-backtrace", "-Zmacro-backtrace", "-Zthreads=8", ] [target.x86_64-unknown-linux-gnu] linker = "clang" rustflags = ["-Clink-arg=-fuse-ld=mold"] [unstable] bindeps = true host-config = true target-applies-to-host = true [profile.dev] debug = "line-tables-only" [profile.dev.package."foo"] opt-level = 3 [profile.dev.package."bar"] opt-level = 3 [profile.dev.package."baz"] debug-assertions = false ```

zmodem commented 6 days ago

If you run ./x.py with stage0 cargo prior to this commit 8457356, it succeeds. So clearly there is something bad in 8457356.

Should 8457356 be considered for reverting while this is being investigated? It seems to be affecting a number of people.

nico commented 1 day ago

We haven't been able to uprev Rust in Chromium for weeks due to this bug.

@gmorenz: Is there a fix on the horizon? If not, can we revert 8457356c31a4cf7fd16d7ed03877645637cb0535 for now to keep HEAD green?

gmorenz commented 1 day ago

@nico This is the first I was alerted about this...

I can set some time aside to work on this immediately. I'll update later today on whether or not I think I can quickly fix this.

Generally speaking I don't see any issue with temporarily reverting https://github.com/rust-lang/cargo/commit/8457356c31a4cf7fd16d7ed03877645637cb0535 if a fix isn't likely to be very fast. We've all lived with the bug that fixed for long enough for it to not be high priority to the project.

gmorenz commented 1 day ago

This is slightly tentative, but here's what I think is happening here so anyone can correct me if they know I'm wrong:

-Ztarget-applies-to-host --config target-applies-to-host=false* make it possible to build two units (in this case cc) for the same output directory with different rustflags (one with artifact rustflags, one with none/host rust flags). Those flags aren't included in the hash which is used to disambiguate different versions of crates, resulting in a conflict. The actual error being produced** appears to be the result of two invocations of rustc racing and clobbering eachother, but even when the error isn't produced we shouldn't expect the right behavior since two different artifacts have the same name and only one of them can sit there afterwards.

This wasn't caught because this is usually masked by the fact that the artifact (non-build-dep) version is passed either -C debuginfo=2 in dev or -C opt-level=3 in release builds. Both of which are included in the relevant hash.

I haven't invested time into verifying this yet, but I believe that this should only occur in cases where the behavior pre 8457356 was already wrong. Specifically as I currently understand it this only occurs when

no --target flag is passed. Otherwise host and target artifacts would be in different directories and we shouldn't hit this bug.
rustflags for non-host dependencies are passed, or with -Zhost-config rustflags for host-dependencies are passed (and if both are passed they are not identical). Otherwise the units would be deduplicated and we shouldn't hit this bug.
-Ztarget-applies-to-host --config target-applies-to-host=false (or equivalent) is passed, otherwise we just pass target rustflags to host deps and we wouldn't hit this bug.

And the combination of those bullet points mean that pre 8457356 we are passing the rustflags for host-deps to target artifacts (i.e. whatever is in the host table if -Zhost-config is set, or nothing if it isn't), which is very surprising (and wrong) behavior.

This obviously needs to be fixed, but if I'm correct that this should only occur in cases where we would otherwise pass the wrong rustflags I don't think reverting 8457356 is much of an improvement. Reverting would replace inconsistent wrong behavior which sometimes errors with consistent wrong behavior that never does.

I'm hopeful the fix is as simple as updating that hashing function and can be out quickly.

* -Zhost-config is a substitute for --config target-applies-to-host=false because it changes the default for that config value to false.

** Incidentally there's another variant of it, with [ERROR] failed to build archive: No such file or directory replaced with [ERROR] failed to create encoded metadata from file: [NOT_FOUND].

PS. Refined version of @weihanglo's test (`lzma-sys` crate as not necessary, `host-config` feature replace with `--config`)

```rust #[cargo_test] fn host_config_shared_build_dep() { // rust-lang/cargo#14253 Package::new("cc", "1.0.0").publish(); let p = project() .file( "Cargo.toml", r#" [package] name = "bootstrap" edition = "2021" [dependencies] cc = "1.0.0" [build-dependencies] cc = "1.0.0" [profile.dev] debug = 0 "#, ) .file("src/lib.rs", "") .file("build.rs", "fn main() {}") .build(); p.cargo("build") .masquerade_as_nightly_cargo(&["target-applies-to-host"]) .arg("-Ztarget-applies-to-host") .arg("--config=target-applies-to-host=false") .env("RUSTFLAGS", "--cfg foo") .with_status(101) // Sometimes it compiles. Not deterministic... .with_stderr_data(str![[r#" [UPDATING] `dummy-registry` index [LOCKING] 2 packages to latest compatible versions [DOWNLOADING] crates ... [DOWNLOADED] cc v1.0.0 (registry `dummy-registry`) [WARNING] output filename collision. The lib target `cc` in package `cc v1.0.0` has the same output filename as the lib target `cc` in package `cc v1.0.0`. Colliding filename is: [ROOT]/foo/target/debug/deps/libcc-[HASH].rlib The targets should have unique names. Consider changing their names to be unique or compiling them separately. This may become a hard error in the future; see . [WARNING] output filename collision. The lib target `cc` in package `cc v1.0.0` has the same output filename as the lib target `cc` in package `cc v1.0.0`. Colliding filename is: [ROOT]/foo/target/debug/deps/libcc-[HASH].rmeta The targets should have unique names. Consider changing their names to be unique or compiling them separately. This may become a hard error in the future; see . [COMPILING] cc v1.0.0 [ERROR] failed to build archive: No such file or directory [ERROR] could not compile `cc` (lib) due to 1 previous error "#]]) .run(); } ```

weihanglo commented 1 day ago

-Ztarget-applies-to-host --config target-applies-to-host=false* make it possible to build two units (in this case cc) for the same output directory with different rustflags (one with artifact rustflags, one with none/host rust flags). Those flags aren't included in the hash which is used to disambiguate different versions of crates, resulting in a conflict.

Thanks for the investigation. Yes. I am just about to post the same observation. RUSTFLAGS is not included in -C metadata hash. Here is the reason — flags may contain absolute paths that affect reproducibility.

gmorenz commented 21 hours ago

Here is the reason — flags may contain absolute paths that affect reproducibility.

The PGO argument against including them in the hash is also very compelling.

In solving #10744 (the issue behind the PR causing this) I treated as design constraints that:

We can't regress on compile time. In particular by continuing to de-duplicate host and target dependencies when not building with a --target flag or rustflags (or non-identical rustflags in the case of -Zhost-config).
We have different rustflags for host and target dependencies.

Those constraints put rustflags firmly in the category of "In general this must include all things that need to be distinguished in different parts of the same build" from the same comment. If we're sharing the artifacts then they must be part of the same build. If we're using different rustflags we need to distinguish that.

Brainstorming approaches:

An effective but very inelegant approach would be to hash where rustflags come from instead of what they are. I.e. hashing a enum RustflagsSource { FlagsAreTheSameBetweenHostAndTarget, Host, Target }. I think this solves the reproducibility problem because changing the prefix in absolute paths shouldn't change whether or not the rust flags are the same between host and target. Likewise with PGO target flags should never be the same as host flags (unless you're applying PGO to host artifacts for some reason).

A more elegant approach, also suggested in #8716, is to "go back to hashing RUSTFLAGS, but only in the -C extra-filename, and not -C metadata". As stated in that issue it would solve the PGO issue but likely not fully solve the reproducibility issue (artifact names are included in debugging information). It would solve our issue here as well since we're just dealing with a filesystem collision.

We could abandon the first design constraint (which I don't think anyone other than myself ever made explicit), always compile host and target dependencies separately when target-applies-to-host=false and add whether it's a host or target dependency to the hash.

gmorenz commented 21 hours ago

@weihanglo I happened to click on the commit you just pushed (https://github.com/rust-lang/cargo/commit/a15807eecc281ba941f9174b94a5f1b20819555e), it doesn't fully fix this issue because in the -Zhost-config case both target and host could have different non-empty rustflags.

weihanglo commented 21 hours ago

Thanks and yes I just realized that.

weihanglo commented 21 hours ago

enum RustflagsSource { FlagsAreTheSameBetweenHostAndTarget, Host, Target }

@gmorenz This is not too bad and safer than -C extra-filename approach at this moment. a15807eecc281ba941f9174b94a5f1b20819555e as an experiment shows your proposal could work. If that implementation is too complidated, we can simplify the logic to NoFlags|Host|Target so we don't need to deal with rustflags comparsion. It might have build time regression when both host and target flags are the same though.

weihanglo commented 20 hours ago

We could abandon the first design constraint (which I don't think anyone other than myself ever made explicit), always compile host and target dependencies separately when target-applies-to-host=false and add whether it's a host or target dependency to the hash.

IIRC, Cargo has been doing this since #11252. Unless the purpose of people doing debug = 0 is for sharing build time and runtime dependencies, this shouldn't be too much a burden. With target-applies-to-host and host-config on, it is less likely people want to share those flags.

Anyhow, we can do a tiny optimization that only when rustflags is empty does Cargo share them. This is nearly useless IMO, but easier in terms of implementation.

gmorenz commented 20 hours ago

enum RustflagsSource { FlagsAreTheSameBetweenHostAndTarget, Host, Target }

@gmorenz This is not too bad

Refining that proposal slightly, I don't think we actually need the whole enum. E.g. rustflags == host.rustflags should be sufficient to disambiguate units. Or in https://github.com/rust-lang/cargo/commit/a15807eecc281ba941f9174b94a5f1b20819555e unit.rustflags == build_runner.bcx.target_data.host_info.rustflags, rustdocflags works the same. links_overrides is unit.links_overrides == build_runner.bcx.target_data.host_config.links_overrides (all untested but I think just substituting that in to the existing commit should work).

IIRC, Cargo has been doing this since https://github.com/rust-lang/cargo/pull/11252.

I'm pretty sure my testing shows differently... I believe there's actually code to check if you can deduplicate host dependencies by changing debug info to match non-host dependencies though I'd need to dig that up again.

gmorenz commented 20 hours ago

There's code in #11252 that ensures deduplication happens: https://github.com/rust-lang/cargo/pull/11252/files#diff-195c704b6ca429e9c366aec7aa05d5c3f429ad41acdd6602a4ce946eb03e4f74R441

With target-applies-to-host and host-config on, it is less likely people want to share those flags.

Today, yes. I'd really like to see these become the default some day (maybe tied to an edition). That's a discussion for somewhere else, but it's guiding the design choices I'm advocating for.

weihanglo commented 20 hours ago

IIRC, Cargo has been doing this since #11252.

I'm pretty sure my testing shows differently... I believe there's actually code to check if you can deduplicate host dependencies by changing debug info to match non-host dependencies though I'd need to dig that up again.

Yes. Only when criteria is met. See this

Refining that proposal slightly…

Feel free to start the work if you're willing to. I won't be available to code in a few hours for a day or two.

gmorenz commented 20 hours ago

I'll push a PR tomorrow

rust-lang / cargo