Cargo wrongly links std to no_std crate with conditional dependency on std, even when that dependency is not selected
#6571
Comments
|
@dwijnand I don't see that any of those tickets has to do with my issue -- they are all about features, and there are no cfg features in my example, I am using This issue specifically has to do with this syntax: Cargo is giving me This has nothing to do with default features, or features being unioned across the build plan. |
|
Let me premise by saying we can reopen if I'm wrong, but I think that's already covered by these comments in those issues: |
|
So, I'm skeptical that this is really the same issue, because when we change it from using I can try to add such a demonstration in the minimal verifiable example. This suggests that there is more to the story than e.g. @alexcrichton 's response to this comment: #2589 (comment)
otherwise why doesn't the same issue occur when we branch on features instead of Contrary to what is written there in 2016, It seems that cargo is successfully able to avoid building |
|
A minimal verificable example would be lovely, thank you. |
|
A little bit of digging into it shows the problem seems to be cargo decided to pass --cfg 'feature="std"' when compiling rand_core, see the following different calls of rustc(you can generate then with cargo build -vv)
(the first one is the correct one). |
|
I think the problem is, when cargo computes Resolve(a data-structure contains which package are dependent by which, and which version and features), it treat take targets-specific dependencies as they are always needed. So rand's mere appearance (even through not later linked in) cause it's std feature, and therefore rand_core's std feature to be included, which cause problem when rand_core is linked with myenslave. and the project compiles. |
|
@lijinpei thank you for digging into this! So my takeaway from this is that making something Do you think that this a "bug" or a design decision of the cargo team? (Or simply not clear which?) |
|
First, theoretically, whether core_rand's std feature enabled or not, it meets all the dependency requirements. A requirements problem may be satisfied by multiple version of a set of packages, or multiple sets of feature of packages, I don't know if cargo guarantees to return a specific set. Secondly, currently, when computing dependencies, cargo treat all platform-specific dependencies as if they are always needed, it's only when the actual build starts the platform-specifics are filtered out. I personally thought this is a bug. But that needs to be cleared be cargo team. (I can see one reason for cargo to work this way is it needs to support cross-build while share the same Cargo.lock. But maybe constraints for different platforms may not be satisfied together?) (A third problem would be features like std needs consensus between rustc and cargo, but cargo won't look into a .rs file's contents, it even doesn't know which set of files are needed to build a crate without rustc's help, while rustc doesn't know package dependencies, it needs cargo to provides them on the command line) |
|
Might be related to #2524? |
|
@lijinpei in any other build system like, scons, cmake, make, if unnecessary stuff is being built, that is a bug. All of these other systems are highly configurable so that you can make it build exactly what you need with exactly the flags you need. It's crazy that in cargo you can have bugs where the wrong stuff is built, it takes hours to track down the reason, and then there's no fix, or maybe a hack requiring changes to everything, and the cargo devs question whether it's a bug! Building unnecessary stuff can mean slow builds or broken builds. Bugs like this in cargo (and there are many of them!), and the difficulty of debugging them, are a serious reason not to use rust at all, esp. in embedded development. In our company we have worked around it with a hack but it's pretty dirty. You can choose not to define it as a bug, but this just means you have set the bar for cargo way below other similar tools.
I don't see a reason for this constraint, any more than it would make sense to require Cargo.lock to be the same no matter what features are enabled. |
|
I'm going to close this as I believe it is resolved by #7914. |
many years ago, we encountered this bug: rust-lang/cargo#6571 This bug meant that if we wanted to have a single Rng type that worked on all platforms, including SGX, then it could not use the standard library on any platform, because cargo would evaluate dependencies without respect to what platform you are on. However, it was really important for our code that we had such an abstraction layer. This was important for writing enclave impl code that could run in SGX and also in unit tests for instance. The way we solved this issue was that, we took the current version of `ThreadRng` which is the generically recommendable cryptographic RNG type from `rand` crate, and made the smallest possible changes to it until it would build without the standard library. The main change was replacing `std::thread_local!` with the `#[thread_local]` attribute, which turned out to be straightforward. However, it creates an annoying maintenance burden on us, and there has been a lot of churn in the rand crate since then. Since the `resolver = 2` stuff was stabilized, the original problem is no longer the case. It's fine to have crates that pull in `std` in one platform but not another. So we can now just use `ThreadRng` as the no-RDRAND fallback, like we wanted all along.
many years ago, we encountered this bug: rust-lang/cargo#6571 This bug meant that if we wanted to have a single Rng type that worked on all platforms, including SGX, then it could not use the standard library on any platform, because cargo would evaluate dependencies without respect to what platform you are on. However, it was really important for our code that we had such an abstraction layer. This was important for writing enclave impl code that could run in SGX and also in unit tests for instance. The way we solved this issue was that, we took the current version of `ThreadRng` which is the generically recommendable cryptographic RNG type from `rand` crate, and made the smallest possible changes to it until it would build without the standard library. The main change was replacing `std::thread_local!` with the `#[thread_local]` attribute, which turned out to be straightforward. However, it creates an annoying maintenance burden on us, and there has been a lot of churn in the rand crate since then. Since the `resolver = 2` stuff was stabilized, the original problem is no longer the case. It's fine to have crates that pull in `std` in one platform but not another. So we can now just use `ThreadRng` as the no-RDRAND fallback, like we wanted all along.
…2503) * delete some terrible code that was written to work around cargo bugs many years ago, we encountered this bug: rust-lang/cargo#6571 This bug meant that if we wanted to have a single Rng type that worked on all platforms, including SGX, then it could not use the standard library on any platform, because cargo would evaluate dependencies without respect to what platform you are on. However, it was really important for our code that we had such an abstraction layer. This was important for writing enclave impl code that could run in SGX and also in unit tests for instance. The way we solved this issue was that, we took the current version of `ThreadRng` which is the generically recommendable cryptographic RNG type from `rand` crate, and made the smallest possible changes to it until it would build without the standard library. The main change was replacing `std::thread_local!` with the `#[thread_local]` attribute, which turned out to be straightforward. However, it creates an annoying maintenance burden on us, and there has been a lot of churn in the rand crate since then. Since the `resolver = 2` stuff was stabilized, the original problem is no longer the case. It's fine to have crates that pull in `std` in one platform but not another. So we can now just use `ThreadRng` as the no-RDRAND fallback, like we wanted all along. * fixup from review comments * remove unnecessary dependencies
and others
Problem
In the minimal test case (https://github.com/cbeck88/rust-cargo-bug), a library is marked
no_stdand which has no dependency onstdis nevertheless linked tostdby cargo, wrongly. This breaks the build, because it conflicts with provided lang items.Yet, when a conditional dependency which is not even selected or built by cargo is commented out in
Cargo.toml,cargostops linkingstdto the first target, and the build succeeds.This indicates that the logic that determines whether a crate transitively depends on
stdis broken and doesn't take into account conditional dependencies properly.Steps
To reproduce the bug, use the minimal verifiable example provided here: https://github.com/cbeck88/rust-cargo-bug
clonethe repositorycargo build, orcargo build --target=x86_64-unknown-linux-gnuif you are not on an x86_64 machine. Observe build failuregit applythe patch, which comments out conditional dependency onrandPossible Solution(s)
I suspect that the logic that determines whether
stdis needed, ignores, or doesn't properly take into account, whether or not a conditional dependency was actually selected.Notes
Output of
cargo version:Using stable cargo and rust on Ubuntu 18.04:
The text was updated successfully, but these errors were encountered: