-
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
Linker and links are ignored with target-applies-to-host and no --target flag #14195
Comments
@rustbot label: Z-target-applies-to-host |
I am not @rustbot :P |
I should say I intend to fix this since it's a natural continuation of what I already fixed, but thought that I should at least create an issue before doing so instead of completely ignoring process. |
Alright, went ahead and tracked down the root cause of the panic for the links-override half of this issue, tl;dr it is a side effect of failing to properly apply the link overrides flags.
Shortly-therafter Much later in the build process Finally we get around to running the custom build script ( The proposed fix should completely fix this error case, by storing link overrides in the unit instead of looking it up via kind when needed it should no longer be possible to get inconsistent results on whether or not overrides exist. |
Just re-read this issue. I guess this table is what we are going to fix?
|
If that is the case, then we don't really need to ping the entire team. |
Yes, exactly that, only modifying behaviour in the top right box. For completeness that table is missing links-overrides, but it follows the same pattern as linker (or would except that in the top right box it actually just panics cargo right now). |
Fix passing of links-overrides with target-applies-to-host and an implicit target ### What does this PR try to resolve? This fixes the link-overrides half of #14195, both the panic, and the fact that the field is being discarded, the latter of which caused the former as discussed in [the issue](#14195 (comment)). It does so following the blueprint laid out in #13900 - which is also in my opinion the current best summary of the broader context. ### How should we test and review this PR? For reviewing, comparing to the changes in #13900 might be useful. ### Additional information I'm pushing a PR for the other half of #14195 simultaneously. I thought it better to keep the PRs small since they're independent, though if merged simultaneously there will be a conflict over the ordering of fields in `Unit`.
Problem
This is the exact same bug as #10744 except with regards to
linker
andlinks
instead ofrustflags
andrustdocflags
. When building withtarget-applies-to-host=false
and not passing a--target
flaglinker
andlinks
are ignored because the host-values of those are set to the default and then the artifact units end up picking up the host values (see "How it is implemented" in #13900 for an explanation of how that happens).Steps
Running on x86_64-unknown-linux-gnu (if you have a different host substitute in the correct architecture)
Expected behavior:
error: linker `nope` not found
Actual behavior: It builds without error
Expected behavior: It builds without error
Expected bug: It fails to find mylib
Actual behavior:
I.e. cargo crashes as a whole instead of just failing to apply the flags. This only happens in the case where target-applies-to-host=false and no --target flag is passed so it's likely a side effect of failing to apply the flags, but that should be confirmed. I am confident it does fail to apply the flags as well from having been working on the code.
Possible Solution(s)
Extend the fix in #13900 to
linker
and link overrides. That should fix the expected bugs, but the actual behavior of link overrides causing cargo to panic is a surprise to me and might need something further.The text was updated successfully, but these errors were encountered: