Skip to content
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

-Zbuild-std support #197

Closed
daxpedda opened this issue Jan 27, 2024 · 6 comments · Fixed by #235
Closed

-Zbuild-std support #197

daxpedda opened this issue Jan 27, 2024 · 6 comments · Fixed by #235

Comments

@daxpedda
Copy link
Contributor

daxpedda commented Jan 27, 2024

Passing -Zbuild-std to Config::dependency_builder does the right thing, but unfortunately Config::program requires more help to register the right dependencies.

The call from Cargo adds these args which are missing from ui_test:

--extern 'noprelude:alloc=/home/rust/test-ui_test/target/debug/deps/liballoc-b1a1c1cd8f555a51.rlib'
--extern 'noprelude:compiler_builtins=/home/rust/test-ui_test/target/debug/deps/libcompiler_builtins-644694813e268ea1.rlib'
--extern 'noprelude:core=/home/rust/test-ui_test/target/debug/deps/libcore-5c3d328d2c56cef9.rlib'
--extern 'noprelude:panic_abort=/home/rust/test-ui_test/target/debug/deps/libpanic_abort-b22430a779820425.rlib'
--extern 'noprelude:panic_unwind=/home/rust/test-ui_test/target/debug/deps/libpanic_unwind-d0ae2f1d6268e79a.rlib'
--extern 'noprelude:proc_macro=/home/rust/test-ui_test/target/debug/deps/libproc_macro-c9c7a2404ad05f6b.rlib'
--extern 'noprelude:std=/home/rust/test-ui_test/target/debug/deps/libstd-1aa5d2ec05eb93ba.rlib'
--extern 'noprelude:test=/home/rust/test-ui_test/target/debug/deps/libtest-eed65dbc0bf35809.rlib'

Cargo has a bit of machinery in place to make this happen:
https://github.com/rust-lang/cargo/blob/3a72bf343b41da9d1676ad8f7b5daea2824d760e/src/cargo/core/compiler/unit_dependencies.rs#L150-L198

So basically it boils down to:

  • Knowing that we did/want to build Std.
  • Finding the Std root folder (installed via Rustup as the rust-src component).
  • Calculating the dependencies (which should be the same as what ui_test already does).
  • Add them correctly to the rustc call.

I'm currently looking into porting some of this code into ui_test and wanted to know if this in scope.

@oli-obk
Copy link
Owner

oli-obk commented Jan 27, 2024

It's def in scope, but please hide it behind an unstable/nightly cargo feature.

Bonus points if we can share code with cargo by putting the logic into a separate crate

@daxpedda
Copy link
Contributor Author

daxpedda commented Jan 27, 2024

So I went in circles for a while trying different solutions which all don't really work.

Cargo does the following:

  1. Find the path to sysroot (done).
  2. Parse -Zbuild-std and build a list of crates we have to include (done).
  3. Create a virtual workspace so we can use the sysroot Cargo.lock and extract the dependencies from the Cargo.toml of each crate we need to include (EDIT: source) (we can't do any of this reliably without actually depending on cargo).

The way we extract dependencies uses cargo metadata, but because the sysroot Cargo.lock is freestanding (doesn't have a corresponding Cargo.toml (workspace or not)), the dependencies it extracts have the wrong version.
E.g. Cargo.toml of std says compiler_builtins = "^0.1.105", so when we run cargo metadata on top we generate a lockfile saying compiler_builtins = "0.1.106", but the disjoint Cargo.lock file says compiler_builtins = "0.1.105".

Ergo cargo metadata just can't work here, unless we get something like rust-lang/cargo#5707.

I believe the best way forward is to directly parse the Cargo.lock file, which doesn't give us proper PackageIds, but we can make a best-effort match pretty reliably.

Alternatively we could also do some upstream changes. E.g. create a proper workspace, or pin versions in Cargo.toml? This isn't an area I'm familiar with at all.

Happy to receive any further guidance!
In the meantime I will go ahead and parse the lockfile.

@oli-obk
Copy link
Owner

oli-obk commented Jan 27, 2024

Uh... maybe we should talk to cargo folk 😅 i'll talk to some next week, but if you wanna reach out on the cargo zulip stream before that, don't let me stop you

@daxpedda
Copy link
Contributor Author

I guess this would solve it as well: rust-lang/wg-cargo-std-aware#20.
So maybe that's just the right answer here.

@daxpedda
Copy link
Contributor Author

In my fork I made it work so far by detecting -Zbuild-std and then match names: daxpedda@c001707.

I'm happy to upstream this behind a crate feature or a Config toggle, so let me know if this is an acceptable solution for now.

@oli-obk
Copy link
Owner

oli-obk commented May 22, 2024

Yea that seems fine to me, no need to hide it behind a crate feature, we'll fix it as it actually sees usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants