rust-lang / wg-cargo-std-aware

Repo for working on "std aware cargo"
133 stars 8 forks source link

Usage of `--sysroot` may still be racy and/or allow false dependencies #49

Closed alexcrichton closed 4 years ago

alexcrichton commented 4 years ago

I'm realizing now that after we've implemented --sysroot that we may still be susceptible races where you can depend on a sysroot crate without actually declaring that you depend on it. Let's say that https://github.com/rust-lang/wg-cargo-std-aware/issues/5 has already been implemented, and you say you only depend on core, I think you could also depend on alloc via the following:

I think the problem here is that we're giving, via --sysroot, crates access to anything in the sysroot. Cargo doesn't actually have any control over what can be loaded from the sysroot. While this will probably only rarely come up in practice, I think we may want to strive to have Cargo have a more intrusive understanding of what crates are where in the sysroot.

Assuming that #5 is implemented I think we'll have some sort of distinction between crates that have sysroot dependencies or not. For example your crate will either declare its sysroot dependencies explicitly, or you won't say anything (like all crates do today) and we'll have some default behavior (like assuming you can use up to proc_macro). That puts us in one of two buckets:

The invented --extern-sysroot flag here would take the form --extern-sysroot name=path. In all cases Cargo would pass -L dependency=... for the sysroot, and the --extern flags would configure which crates can be loaded from the sysroot. The only reason for --extern-sysroot to exist is to match the name resolution behavior of sysroot crates today since there are subtle differences. Alternatively we could fix rustc so there are no differences!

ehuss commented 4 years ago

I have implemented a branch that removes --sysroot, and instead adds "options" to the --extern flag. In this case, it is --extern noprelude:alloc=/path/to/deps/liballoc.rlib. The "noprelude" option tells rustc not to add to the prelude which addresses the original concern in #40 that made us switch. It also is more careful about which crates it passes to rustc via --extern. It seems to work well.

Any objections to moving in that direction?

If that is OK, the next question I have: Should I prototype a new flag in rustc to disable crate searches in the sysroot? This would help with avoiding the dreaded "duplicate lang item" error if you have an errant extern crate for something you shouldn't have access to (and instead show E0463 or E0433 failed to resolve error). I think it might be a good idea, and would also ensure that things like rust-lld are still found. However, I recognize that it's a minor error message improvement, so may not be important enough. I'm not sure how careful rustc is about the proliferation of random flags. If that sounds ok, any opinion on the flag name? Maybe --no-sysroot-search?

alexcrichton commented 4 years ago

Any objections to moving in that direction?

👍 from me!

Should I prototype a new flag in rustc to disable crate searches in the sysroot?

👍 from me as well

The name of --no-sysroot-search sounds good to me!

Ericson2314 commented 4 years ago

I think this should be closed after https://github.com/rust-lang/cargo/pull/7699?

ehuss commented 4 years ago

I've been leaving it open because --no-sysroot-search still needs to be implemented.

Ericson2314 commented 4 years ago

Isn't that what #31 is, or am I missing something?

ehuss commented 4 years ago

Oh, good point, closing for #31.