panic not always tracked in built dependencies

[ehuss]

Whether or not panic was used is not always tracked in the metadata hash. This can cause cargo to mistakenly reuse a lib with the wrong settings.

In particular, build-dependencies, plugins, and proc-macros and their dependencies do not encode that they were compiled without panic.

This can creep up in a variety of ways, but here is a relatively small repro:

mkdir ws
cd ws
cargo new --lib a
cargo new --lib pm
cat > Cargo.toml <<EOL
[workspace]
members = ["a", "pm"]
[profile.dev]
panic = "abort"
EOL

cat >> pm/Cargo.toml <<EOL
a = {path="../a"}
[lib]
proc-macro = true
EOL

echo "extern crate a;" >> pm/src/lib.rs

# This builds `a` with panic.
cargo build -p a -v
# This fails with wrong panic strategy since it reuses `a`.
cargo build -p pm -v
# Note: Compiling these in the other order works.

Fixing this is partially blocked by #5444, and possibly other issues.

[alexcrichton]

Hm this is somewhat surprising, the fingerprint hash contains the entire Profile, right? which contains the panic setting?

[ehuss]

Panic is set in the profile for proc-macros. It is only cleared at the end when the args are constructed here:

cargo/src/cargo/core/compiler/mod.rs

Lines 910 to 914 in 181de52

	if let Some(panic) = panic.as_ref() {
	if !cx.used_in_plugin.contains(unit) {
	cmd.arg("-C").arg(format!("panic={}", panic));
	}
	}

Nothing in the hash indicates that a dependency is used by a proc-macro.

[alexcrichton]

Ah ok makes sense! I think we should probably just include cx.used_in_plugin.contains(unit) in the fingerprint, right?

[alexcrichton]

This should be fixed at #5458

[ehuss]

I was thinking about clearing the panic flag when the profile is constructed. That would fix dependencies shared with proc-macro dependencies (currently shared dependencies are built without panic set).

[alexcrichton]

@ehuss ah yes I think that'd probably be the best solution, I was hoping for a quick fix as this is a pretty serious bug and beta is being branched this weekend. That sounds like a good follow-up though!

[ehuss]

@alexcrichton kk, i'm working on it now, will hopefully get the bugs nailed down before then.

[alexcrichton]

Awesome, thanks!

[alexcrichton]

@ehuss btw do you have an example which still fails to build after #5458? If so I can reopen

[ehuss]

@alexcrichton Shared dependencies will be built w/o panic:

cargo new --bin foo
cd foo
cargo new --lib shared
cargo new --lib pm

cat >> pm/Cargo.toml <<EOF
shared = {path = "../shared"}
[lib]
proc-macro = true
EOF

cat >> Cargo.toml <<EOF
pm = {path = "pm"}
shared = {path = "shared"}

[profile.dev]
panic = "abort"
EOF

# `shared` will be built without panic.
cargo build -v

This doesn't cause any errors since linking with unwind dependencies doesn't fail.

I can't remember if I asked already, but what are the specific problems with linking with a lib built with unwind? It sure would be easier if we could just set panic for bins.

[alexcrichton]

@ehuss oh that's the previous behavior of Cargo, right?

It's totally ok to link a panic=abort final artifact to intermediate libraries that are compiled as panic=unwind, but you just can't do the reverse.

I do think though that it'd be best to compile the lib twice, once with unwind and once with abort, but if it's not a regression then that at least buys us some more time!