Specific dependency configuration causes target to be rebuilt needlessly #6154
Comments
|
Thanks for making the repro. It's essentially the same as mentioned in #6143. Cargo tries to keep track of what is needed to be built for the host vs the target (for cross-compiling), and build scripts, their dependencies, compiler plugins, etc. all get lumped together as host-specific things all for the purpose of making sure I know how to fix this, it shouldn't be too difficult. It's just that the fix might cause some things to be built multiple times when they previously weren't (specifically when you set I'll try to tinker with it sometime soon and see what the implications are. |
Well, this issue is specifically a problem because of the extra compile time it causes. I certainly don't want a fix that would cause needless recompilations elsewhere. If this can be fixed with clever engineering without causing needless recompiles elsewhere, I'd rather wait for a proper fix, even if it means rewriting the whole system. |
|
Thanks for digging into this @ehuss! |
Track `panic` in Unit early. This is an alternate solution for #5445. It ensures that `panic` is cleared in the Profile for "for_host" targets (proc-macro/plugin/build-scripts) and dependencies. This helps avoid unnecessary recompiles in some situations (though does add extra units in some situations, see below). Some examples where extra rebuilds are now avoided: * A workspace with a dependency shared with normal and build-deps. `build --all` should build everything, and then `build -p foo` was causing a recompile because the shared dep was no longer in the `used_in_plugin` set. Now it should not recompile. * `panic=abort`, with a shared dependency in build and dev, `build` would cause that shared dependency to be built twice (exactly the same without panic set). Now it should only build once. Some examples where `panic` is now set correctly: * `panic=abort`, with a binary with a shared dependency in build and normal, `test` would cause that shared dependency to be built twice (exactly the same without panic set). Now it is still built twice, but the one for the normal (bin) dependency will correctly have `panic` set. Some examples where new units are now generated: * `panic=abort`, with shared dependency between normal and proc-macro or build. Previously the shared dependency was built once with `panic=unwind`. Now it is built separately (once with `panic`, once without). I feel like that this is more correct behavior — that now the normal dependency avoids adding landing pads. For `panic=abort` cross-compiling, this makes no difference in compile time since it was already built twice. Additional notes: - I left build-scripts with the old behavior where `panic` is cleared for it and all its dependencies. There could be arguments made to change that (#5436), but it doesn't seem important to me. - Renamed and refactored `ProfileFor` to `UnitFor`. I expect this API to continue to evolve in the future. Closes #6143, closes #6154.
So I took @ehuss 's advice in #6143 , and changed my plugins not to rely on
plugin = true.Turns out, my binary is still rebuilt needlessly, because there is something else that triggers this behavior besides
plugin = true. It looked like a plugin depending onhtml5evertriggered this issue, so I went down the rabbit hole, and tried to create a minimal test case that doesn't have external dependencies.The result is at https://github.com/crumblingstatue/cargo-rebuild-repro-2 .
The problem is still the same. Do
cargo build --all, see thefooprojectbinary intarget/debug, docargo run, seefooprojectand its dependency get needlessly rebuilt.The text was updated successfully, but these errors were encountered: