cargo icon indicating copy to clipboard operation
cargo copied to clipboard

Pass rustflags to artifacts built with implicit targets when using target-applies-to-host

Open gmorenz opened this issue 2 months ago • 5 comments

What this PR does

This fixes #10744, a long-standing bug with target-applies-to-host=false where RUSTFLAGS are not being passed to artifact-units when built with cargo build (as opposed to cargo build --target <host>).

It doesn't fix a similar problem with linker and links. If the architecture in this PR is accepted, I expect I will be able to fix linker and links in the same way in a subsequent PR.

Below is a hopefully useful table of what flags are passed when, with new behavior bolded (without these changes the flag is not passed). I've only changed values in the top right cell, I've included the whole table for completeness and to hopefully make clear what exactly this feature is doing (which took me awhile to understand).

The table was generated with a host of x86_64-unknown-linux-gnu. "Flag" refers to values in RUSTFLAGS. "Linker" refers to the value of --config target.<host>.linker . The table was built with this repo and then hand modify to bold changed values.

target_applies_to_host=true target_applies_to_host=false
no --target flag Flag passed to bin
Flag passed to shared dep from bin
Linker passed to bin

Flag passed to proc macro
Flag passed to shared dep from proc macro
Linker passed to proc macro

Built with 5 invocations
Without rustflags, built with 5 invocations
Flag passed to bin
Flag passed to shared dep from bin
Linker not passed to bin

Flag not passed to proc macro
Flag not passed to shared dep from proc macro
Linker not passed to proc macro

Built with 6 invocations
Without rustflags, built with 5 invocations
--target x86_64-unknown-linux-gnu Flag passed to bin
Flag passed to shared dep from bin
Linker passed to bin

Flag not passed to proc macro
Flag not passed to shared dep from proc macro
Linker passed to proc macro

Built with 6 invocations
Without rustflags, built with 6 invocations
Flag passed to bin
Flag passed to shared dep from bin
Linker passed to bin

Flag not passed to proc macro
Flag not passed to shared dep from proc macro
Linker not passed to proc macro

Built with 6 invocations
Without rustflags, built with 6 invocations
--target i686-unknown-linux-gnu Flag passed to bin
Flag passed to shared dep from bin
Linker not passed to bin

Flag not passed to proc macro
Flag not passed to shared dep from proc macro
Linker passed to proc macro

Built with 6 invocations
Without rustflags, built with 6 invocations
Flag passed to bin
Flag passed to shared dep from bin
Linker not passed to bin

Flag not passed to proc macro
Flag not passed to shared dep from proc macro
Linker not passed to proc macro

Built with 6 invocations
Without rustflags, built with 6 invocations

How it is implemented

There are two stages in the UnitGraph's life. It gets built, with correct CompileKind: CompileKind::Host for host units, CompileKind::Target(HostTriple) for artifact units. Then it gets rebuilt with artifact units that are intended for the host having their kind changed to CompileKind::Host.

This rebuilding step serves to de-duplicate host and artifact units. A step that we want to maintain where possible. Because de-duplicating host and artifact dependencies is only possible if their rustflags don't differ we must calculate rustflags for units before this step, and this step necessarily throws away the information necessary to calculate them.

The possible rustflags have already been determined before creating units. "Calculating rustflags for units" just means determining which set of rustflags go with a particular unit, and storing that somewhere. The obvious place to do that is in the unit itself, so that's what this PR does, which has the convenient side affect of also handing the de-duplication logic for us. If the rustflags are the same, two units can be merged, if they differ, they cannot.

In the above table that's why it takes 5 invocations to build the repo without rustflags, but 6 with them. The shared_dependency merges when it is built without rustflags, and not when it is built with rustflags.

Related PRs, comments and issues

fixes #10744

#9453 - Tracking issue

#9753 - Stabilization PR

#9634 - I believe this was another attempt (going down another implementation route) to fix the same issue

Comments from Alex Crichton noting that this must be fixed 1 2

My own comment describing exactly how I ran into this

Documentation for the feature

gmorenz avatar May 11 '24 03:05 gmorenz