Fetch dependencies for -Zbuild-std before entering the sandbox

[jyn514]

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 rust-lang/cargo#10129 to make -Zbuild-std actually have an effect.

[pietroalbini]

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

• Add a comment explaining why -Zbuild-std is provided there
• Add a test building a -Zbuild-std crate to make sure we don't regress that
• Add a changelog line

[pietroalbini]

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]

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]

why make it toggleable?

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

[jyn514]

How can I test this? I tried modifying the hello world:

rustwide/tests/buildtest/mod.rs

Lines 10 to 27 in 326694e

	fn test_hello_world() {
	runner::run("hello-world", |run| {
	run.build(SandboxBuilder::new().enable_networking(false), |build| {
	let storage = rustwide::logging::LogStorage::new(LevelFilter::Info);
	rustwide::logging::capture(&storage, || -> Result<_, Error> {
	build.cargo().args(&["run"]).run()?;
	Ok(())
	})?;
	assert!(storage.to_string().contains("[stdout] Hello, world!\n"));
	assert!(storage
	.to_string()
	.contains("[stdout] Hello, world again!\n"));
	Ok(())
	})?;
	Ok(())
	});
	}

but there's no way to configure the workspace using runner::run, it doesn't have a way to pass in options for workspace:

rustwide/tests/utils/mod.rs

Lines 19 to 32 in 326694e

	let mut builder = WorkspaceBuilder::new(&workspace_path, USER_AGENT).fast_init(true);
	if std::env::var("RUSTWIDE_TEST_INSIDE_DOCKER").is_ok() {
	builder = builder.running_inside_docker(true);
	}
	// Use the micro image when running tests on Linux, speeding them up.
	if cfg!(target_os = "linux") {
	builder = builder.sandbox_image(SandboxImage::remote(
	"ghcr.io/rust-lang/crates-build-env/linux-micro",
	)?);
	}
	builder.init()

[saethlin]

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

[jyn514]

@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]

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

[jyn514]

@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]

@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]

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

[jyn514]

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]

Ah, apparently the build directory gets wiped before each test:

rustwide/tests/buildtest/runner.rs

Lines 38 to 44 in 5c579a5

	pub(crate) fn build<T>(
	&self,
	sandbox: SandboxBuilder,
	f: impl FnOnce(BuildBuilder) -> Result<T, Error>,
	) -> Result<T, Error> {
	let mut dir = self.workspace.build_dir(&self.crate_name);
	dir.purge()?;

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]

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]

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]

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

[pietroalbini]

Released rustwide 0.16.0 with this PR.