-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The new per-package-target
feature does not work with -Zbuild-std
#9451
Comments
Doing some further testing and debugging revealed that the problem has nothing to do with the way So I tried instead to see what would happen if I commented out the check in question and then tested the compiler that way. You'd expect |
Looks like another possible lead on Line 955:
This might be interfering with the ability to pass custom target files as arguments to the |
For the record, here's what happens when I attempt a build with both the problematic test commented out and while using Cargo.toml instead of the command line to specify the
If @phil-opp was correct about this check being the problem, this wouldn't be happening. This panic message, however, does at least give a better hint as to what the actual problem is: "no entry found for key" ― this means that FWIW this might be the exact same issue as #9521, given that the lack of information denoted in the panic message is something you'd expect if |
Opening
Line 151 in this case is the
It appears that |
Looks like I finally found the real culprit at lines 450-454:
The |
Yet another update: turns out that wasn't the issue either. Even this change wasn't enough to solve the problem:
Update: Same panic message but from a different source file:
This is part of this function:
That it's still panicking with the same error (albeit in a different file) after this change shows that it's probably a TOML parsing issue, for 2 main reasons:
So what needs to happen is that someone on the |
Trying this again with pull-request #10108 which is a partial fix ― needs some further work to merge though; hopefully by opening it up to the open-source community at large, it'll help get the problem solved faster. The new |
@kennystrawnmusic Thank you very much for your investigation! Here are some comments about your findings:
I think this is not the case: #9521 is not about misidentifying the target specified, but about compiling crates with too many features. Basically, the core of the issue is that cargo assumes additive features, so when having a workspace with two dependencies on foo with different features it compiles it just once with all the features. The issue of #9521 is that it still does this if the two dependencies on foo are with different architectures, which in practice leads to broken code because not all features are available on all architectures and so features aren't actually really additive. The fix to #9521 would be to instead of having one crate build graph with all the features, have one crate build graph per architecture… either that, or have one crate build graph per crate instead of per workspace, which to me would make the most sense so that
Yes,
AFAICT there's no per-package-target team, I just wrote the initial implementation and then didn't have enough time to get back to this :( that said, if you have a small reproducer with an unpatched cargo, would you be able to share it with me as eg. a tarball or git repo along with the exact commands to be used to reproduce? I may have a bit of time in the upcoming weeks, and can try looking at this :) |
Thinking some more about it: what would be the semantics of I'm halfway thinking |
Sorry for the late response — been very busy myself because it is almost Christmas after all. One already-existing set of instructions to reproduce this bug is the set of instructions that @phil-opp posted to his blog. Whenever the “-Zbuild-std” option is invoked, it fails to check if “per-package-target” is specified in the package manifest file before throwing an error. That’s why I attempted to contribute that Obviously it is possible to work around this bug by simply specifying the custom target .json file with As Phil also suggested here, exposing |
Alright, new plan: going to delete my fork and re-fork from scratch, then rewrite the |
Rewritten with a whole new approach: kennystrawnmusic@590a5aa Knowing that this check uses |
@Ekleog Update: nope, that didn’t work either because the whole code needs to be refactored in that case due to the change in mutability. That’s what makes this such a challenge: change just one variable in one function in one corner of a project from immutable to mutable and suddenly you get cascading errors across multiple files that even the most powerful refactoring tools imaginable cannot fix. |
If I try to create a new local variable (say,
|
Yeah, I'm getting this issue with my Rust bootloader. I use the |
@kennystrawnmusic I've forked your fork, rebased against upstream master, and on top of your last commit (757267c662f77dc155feb6dc4ec5a7386549884), fixed a borrow error (22d9e06473de7094078e90425a7479f3e41090ea), and solved an unknown attribute access on the We still have an error that is pasted below, and I'm not sure how to approach fixing it. What do you think?
|
I’ve decided to endorse the #10308 changes anyway. Tested using the following project file layout and can confirm that it does indeed fix this: build-std-test/Cargo.toml
build-std-test/custom.json
build-std-test/src/main.rs
Command line output
|
Problem
I just tried to use the new
per-package-target
feature implemented in #9030 (tracking issue) together with-Zbuild-std
, but it results in anerror: -Zbuild-std requires --target
.Steps
default-target
orforced-target
key inCargo.toml
.cargo build -Zbuild-std=core
.Possible Solution(s)
I think the issue is that the target check already happens here:
cargo/src/cargo/ops/cargo_compile.rs
Lines 394 to 399 in db741ac
But the
default-target
/forced-target
fields are only considered later ingenerate_targets
:cargo/src/cargo/ops/cargo_compile.rs
Lines 925 to 943 in db741ac
The
generate_targets
function is invoked here:cargo/src/cargo/ops/cargo_compile.rs
Lines 470 to 488 in db741ac
Notes
Output of
cargo version
:cc @Ekleog
The text was updated successfully, but these errors were encountered: