rust-lang / rustwide

Execute your code on the Rust ecosystem.
Apache License 2.0
180 stars 44 forks source link

Fetch dependencies for `-Zbuild-std` before entering the sandbox #66

Closed jyn514 closed 1 year ago

jyn514 commented 2 years ago

This allows running doc -Zbuild-std from within the sandbox. Previously, it would error out because cargo tried to download the standard library's dependencies:

[2021-11-27T19:57:24Z INFO  rustwide::cmd] running `Command { std: "docker" "create" "-v" "/home/joshua/src/rust/docs.rs/.workspace/builds/gba-0.5.2/target:/opt/rustwide/target:rw,Z" "-v" "/home/joshua/src/rust/docs.rs/.workspace/builds/gba-0.5.2/source:/opt/rustwide/workdir:ro,Z" "-v" "/home/joshua/src/rust/docs.rs/.workspace/cargo-home:/opt/rustwide/cargo-home:ro,Z" "-v" "/home/joshua/src/rust/docs.rs/.workspace/rustup-home:/opt/rustwide/rustup-home:ro,Z" "-e" "SOURCE_DIR=/opt/rustwide/workdir" "-e" "CARGO_TARGET_DIR=/opt/rustwide/target" "-e" "DOCS_RS=1" "-e" "CARGO_HOME=/opt/rustwide/cargo-home" "-e" "RUSTUP_HOME=/opt/rustwide/rustup-home" "-w" "/opt/rustwide/workdir" "-m" "3221225472" "--user" "1000:1000" "--network" "none" "ghcr.io/rust-lang/crates-build-env/linux-micro" "/opt/rustwide/cargo-home/bin/cargo" "+nightly" "rustdoc" "--lib" "-Zrustdoc-map" "-Z" "unstable-options" "--config" "build.rustdocflags=[\"--cfg\", \"docs_rs\", \"-Z\", \"unstable-options\", \"--emit=invocation-specific\", \"--resource-suffix\", \"-20211126-1.58.0-nightly-6d246f0c8\", \"--static-root-path\", \"/\", \"--cap-lints\", \"warn\", \"--disable-per-crate-search\"]" "-Zunstable-options" "--config=doc.extern-map.registries.crates-io=\"https://docs.rs/{pkg_name}/{version}/thumbv4t-none-eabi\"" "-Zbuild-std" "--target" "thumbv4t-none-eabi", kill_on_drop: false }`
[2021-11-27T19:57:24Z INFO  rustwide::cmd] [stdout] fe2773f8a17ab13ce7b66c7221f91883a7b92b3bdeef7b30bf2ed55aa6b3d511
[2021-11-27T19:57:24Z INFO  rustwide::cmd] running `Command { std: "docker" "start" "-a" "fe2773f8a17ab13ce7b66c7221f91883a7b92b3bdeef7b30bf2ed55aa6b3d511", kill_on_drop: false }`
[2021-11-27T19:57:24Z INFO  rustwide::cmd] [stderr]  Downloading crates ...
[2021-11-27T19:57:24Z INFO  rustwide::cmd] [stderr] warning: spurious network error (2 tries remaining): [6] Couldn't resolve host name (Could not resolve host: crates.io)
[2021-11-27T19:57:24Z INFO  rustwide::cmd] [stderr] error: failed to download from `https://crates.io/api/v1/crates/libc/0.2.106/download`
[2021-11-27T19:57:24Z INFO  rustwide::cmd] [stderr]
[2021-11-27T19:57:24Z INFO  rustwide::cmd] [stderr] Caused by:
[2021-11-27T19:57:24Z INFO  rustwide::cmd] [stderr]   [6] Couldn't resolve host name (Could not resolve host: crates.io)

This is currently blocked on https://github.com/rust-lang/cargo/pull/10129 to make -Zbuild-std actually have an effect.

pietroalbini commented 2 years ago

I know this is not working yet due to the Cargo bug, but could you:

pietroalbini commented 2 years ago

Also, this should be toggleable in WorkspaceBuilder (with a method like enable_build_std_support(enable: bool)), and the method should be gated by the unstable crate feature (as -Zbuild-std is nightly-only).

jyn514 commented 2 years ago

the method should be gated by the unstable crate feature (as -Zbuild-std is nightly-only).

This makes sense to me, but why make it toggleable? It should be very cheap to do compared to all the other setup rustwide does, and fewer knobs to tweak means less complexity IMO.

jyn514 commented 2 years ago

why make it toggleable?

Oh ... it turns out rustwide uses these features unconditionally, even on stable toolchains :upside_down_face: yeah that makes sense why it needs to be under an unstable feature then.

jyn514 commented 2 years ago

How can I test this? I tried modifying the hello world: https://github.com/rust-lang/rustwide/blob/326694e5575aacfb45213f4951e33955c044935b/tests/buildtest/mod.rs#L10-L27 but there's no way to configure the workspace using runner::run, it doesn't have a way to pass in options for workspace: https://github.com/rust-lang/rustwide/blob/326694e5575aacfb45213f4951e33955c044935b/tests/utils/mod.rs#L19-L32

saethlin commented 2 years ago

I would really like to have this so that I can evaluate how much code was broken by https://github.com/rust-lang/rust/pull/92686. Is there anything I can do to help here?

jyn514 commented 2 years ago

@saethlin the cargo side of this was actually merged recently, so this should be unblocked. It's been a very long time since I've thought about this PR but judging by my past comments I think it just needs tests? Happy for you to take it over if you have time.

SeniorMars commented 2 years ago

Hello, I will attempt to do these tests. I will keep you updated on how it goes!

jyn514 commented 1 year ago

@pietroalbini it's been a very long time since I've looked at this PR ... if you don't have time to help me get the tests working, do you mind if we merge without tests? This would unblock building for tier 3 targets in docs.rs.

pietroalbini commented 1 year ago

@jyn514 ok, so I took a look at the code again, and what I would do is:

  1. In tests/utils/mod.rs, add a new prepare_named_workspace that does everything init_named_workspace does except calling builder.init(). Then init_named_workspace can be changed to call prepare_named_workspace(...).init()
  2. In tests/buildtest/runner.rs, change Runner::new to also accept the workspace as the argument, rather than initializing it inside. Move the init_workspace() call to the run standalone function.
  3. In the test, call your own prepare_named_workspace, customize it, and pass it to Runner::new.
jyn514 commented 1 year ago

Done! Thank you for making me write the test, it caught several bugs :smile:

jyn514 commented 1 year ago

Looks like it's failing to mount the source directory, or something?

[INFO  rustwide::cmd] running `Command { std: "docker" "create" "-v" "/home/runner/work/rustwide/rustwide/.workspaces/integration/builds/hello-world/target:/opt/rustwide/target:rw,Z" "-v" "/home/runner/work/rustwide/rustwide/.workspaces/integration/builds/hello-world/source:/opt/rustwide/workdir:ro,Z" "-v" "/home/runner/work/rustwide/rustwide/.workspaces/integration/cargo-home:/opt/rustwide/cargo-home:ro,Z" "-v" "/home/runner/work/rustwide/rustwide/.workspaces/integration/rustup-home:/opt/rustwide/rustup-home:ro,Z" "-e" "SOURCE_DIR=/opt/rustwide/workdir" "-e" "CARGO_TARGET_DIR=/opt/rustwide/target" "-e" "RUSTC_BOOTSTRAP=1" "-e" "CARGO_HOME=/opt/rustwide/cargo-home" "-e" "RUSTUP_HOME=/opt/rustwide/rustup-home" "-w" "/opt/rustwide/workdir" "--user" "1001:123" "--network" "none" "ghcr.io/rust-lang/crates-build-env/linux-micro@sha256:8ccaa386dd87fbd89917ce35effac8d5c8b22ea6981f511e336859b38fb07f26" "/opt/rustwide/cargo-home/bin/cargo" "+stable" "run" "-Zbuild-std" "--target" "x86_64-unknown-linux-gnu", kill_on_drop: false }`
[INFO  rustwide::cmd] [stderr]    Compiling hello-world v0.1.0 (/opt/rustwide/workdir)
[INFO  rustwide::cmd] [stderr] error: Unable to proceed. Could not locate working directory.: No such file or directory (os error 2)
[INFO  rustwide::cmd] [stderr] error: could not compile `hello-world`
jyn514 commented 1 year ago

Ah, apparently the build directory gets wiped before each test: https://github.com/rust-lang/rustwide/blob/5c579a5246863f0523cfab3b68132a01d52ddf58/tests/buildtest/runner.rs#L38-L44 I think this is going wrong because I set multiple tests to use the same source crate? I'll see if I can randomize the name of the build directory so they don't overlap.

jyn514 commented 1 year ago

Ah, this isn't actually ready to merge - docs.rs needs the ability to fetch dependencies for multiple targets. Also I think I'll need to move this from the WorkspaceBuilder to the BuildBuilder to allow it to be configurable per-crate.

jyn514 commented 1 year ago

Now this is actually ready, I tested it works correctly in docs.rs.

Putting the fetch function on Build is a little strange, but necessary for docs.rs because we need to parse the metadata in Cargo.toml before we know the targets to fetch, and Cargo.toml isn't available until Prepare runs.

jyn514 commented 1 year ago

👋 @pietroalbini do you know when you'll get a chance to look at this?

pietroalbini commented 1 year ago

Released rustwide 0.16.0 with this PR.