-
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
Fix logic for determining prefer-dynamic for a dylib. #9252
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
@alexcrichton, before moving forward with this, I'd like to better understand the motivation for why it works this way. Can you explain what the reasoning is? |
Honestly at this point I have no idea. I barely know of any use cases of dylib crate types much less intermediate dependencies. AFAIK there's no real story about dylib dependencies in Rust right now so changing this seems fine by me. r=me if you're ok landing |
OK, understood. Since this is clearly broken, I'll go ahead and merge, hopefully it doesn't break anyone relying on it. @bors r=alexcrichton |
📌 Commit 48e0306 has been approved by |
⌛ Testing commit 48e0306 with merge c43e485eb0d7c97ae7db97821c4c9e735c965fd2... |
💔 Test failed - checks-actions |
The lincheck script is a little out-of-sync with the latest nightly (rust-lang/rust#82950), it should resolve itself after the next nightly comes out (~7 hours from now). I'll wait for that and try again later. |
@bors retry |
☀️ Test successful - checks-actions |
Update cargo 7 commits in 970bc67c3775781b9708c8a36893576b9459c64a..32da9eaa5de5be241cf8096ca6b749a157194f77 2021-03-07 18:09:40 +0000 to 2021-03-13 01:18:40 +0000 - Fix logic for determining prefer-dynamic for a dylib. (rust-lang/cargo#9252) - Fix issue with filtering exclusive target dependencies. (rust-lang/cargo#9255) - Update pkgid-spec docs. (rust-lang/cargo#9249) - Wordsmith the edition documentation a bit more (rust-lang/cargo#9233) - Package ID specification urls must contain a host (rust-lang/cargo#9188) - Add documentation for JSON message_path. (rust-lang/cargo#9247) - Fix filter_platform to run on targets other than x86. (rust-lang/cargo#9246)
Update cargo 7 commits in 970bc67c3775781b9708c8a36893576b9459c64a..32da9eaa5de5be241cf8096ca6b749a157194f77 2021-03-07 18:09:40 +0000 to 2021-03-13 01:18:40 +0000 - Fix logic for determining prefer-dynamic for a dylib. (rust-lang/cargo#9252) - Fix issue with filtering exclusive target dependencies. (rust-lang/cargo#9255) - Update pkgid-spec docs. (rust-lang/cargo#9249) - Wordsmith the edition documentation a bit more (rust-lang/cargo#9233) - Package ID specification urls must contain a host (rust-lang/cargo#9188) - Add documentation for JSON message_path. (rust-lang/cargo#9247) - Fix filter_platform to run on targets other than x86. (rust-lang/cargo#9246)
The logic for determining if a dylib should use
prefer-dynamic
used to be something like "do not use prefer-dynamic if it is the current package".The current logic has a strange behavior where it works as intended if there is only one package in the workspace, but a workspace with multiple packages will always use
prefer-dynamic
.Instead of using
current_opt
, which isn't a good concept to use in a workspace, I switched this to be "primary" (a package selected on the command-line).History
--all
flag tocargo test
#3221 changed to the faulty logic (see comments at Add--all
flag tocargo test
#3221 (comment)). I think there was some confusion there.