Add Library API

[XAMPPRocky]

Hello, I'm working on EmbarkStudios/rust-gpu and we're currently looking at how we can build our own sysroot as part of our build process (EmbarkStudios/rust-gpu#48). Cargo's -Zbuild-std is currently not sufficient for us, as we want to be able to re-use the sysroot both for development and across multiple tests. Using cargo's -Zbuild-std either requires locking dependencies, redundantly compiling sysroot multiple times, or generate a complicated setup for tests so that they can re-use libcore. Ideally we'd like to just compile it once and re-use it throughout the build.

While investigating what was already available in this space I came across your crate, and found that it mostly does what we need for the project except for generating a .cargo/config.toml and being only available as a binary tool. Would you be interested in adding a lib.rs that allows a user to just build the sysroot? I would be willing to contribute the work for the change.

[DianaNites]

Yeah thats fine

[DianaNites]

Do these changes work?

[XAMPPRocky]

I'm going to try to build a proof of concept with it over the next couple of days, but it looks good to me just reviewing the code.

[XAMPPRocky]

So one thing I don't understand in the current API, is why does generate_alloc_cargo_toml take in a pre-existing manifest file and inherit its profile settings? Could this be optional so that it uses the default if you don't provide a manifest file?

cargo-sysroot/src/lib.rs

Lines 62 to 66 in 4a0c48b

	profile: {
	let toml: CargoToml =
	from_path(manifest).with_context(|| manifest.display().to_string())?;
	toml.profile
	},

[XAMPPRocky]

Another thing is that this currently builds every sysroot crate, but right now rust-gpu can really only compile some of core. Having an option to be able to compile alloc, core, and compiler_builtins as separate options would be immensely helpful.

[DianaNites]

generate_alloc_cargo_toml inherits profiles because it seemed to match carbo-xbuild behavior, and a project that worked with xbuild failed with mine until I added it, but I can make it optional.

[DianaNites]

Okay, hows it look now?

The mem feature of compiler_builtins is no longer on by default, and you can choose which sysroot crate to compile, core, alloc, or std.

Theres no option for compiler_builtins itself because, besides core, everything else depends on it. For core it simply isn't included at all, but I can change that if need be.

[XAMPPRocky]

Thank you so much for working on this! The API is much improved.

Theres no option for compiler_builtins itself because, besides core, everything else depends on it. For core it simply isn't included at all, but I can change that if need be.

That would be great, as it seems that those compiler_builtins are required for core, even if they aren't built. At least trying to build a module with just core available in the sysroot provides a error[E0463]: can't find crate for `compiler_builtins`. when running the following command.

Build

env::set_var("RUSTFLAGS", format!("-Zcodegen-backend={}", path_to_backend.display()));
let sysroot = cargo_sysroot::build_sysroot_with(
        None,
        &target_dir,
        "spirv-unknown-unknown".as_ref(),
        &src,
        cargo_sysroot::Sysroot::Core,
        false,
).map_err(|e| eyre!("Error building sysroot: {}", e))?;
env::remove_var("RUSTFLAGS");

Command

rustc lib.rs --target spirv-unknown-unknown --sysroot=<path_to_sysroot> -Zcodegen-backend=<path_to_rustc_codegen_spirv.dylib> -C target-feature=+spirv1.0

lib.rs

#![feature(lang_items)]
#![no_std]
#![feature(register_attr)]
#![register_attr(spirv)]

#[panic_handler]
fn panic(_: &core::panic::PanicInfo) -> ! {
    loop {}
}

#[lang = "eh_personality"]
extern "C" fn rust_eh_personality() {}

#[spirv(vertex)]
pub fn main() {
}

[DianaNites]

error[E0463]: can't find crate for compiler_builtins.

Thats odd, I wonder whats requiring it, I thought missing compiler_builtins usually caused linker errors. I tried to reproduce it and couldn't, though using a different target. May be specific to your target?

In light of that, instead of making Core bring in compiler_builtins, I've added Sysroot::CompilerBuiltins, which brings in both core and compiler_builtins. Does this work for you?

[XAMPPRocky]

Thank you so much again for working on this! I've now managed to create a working prototype that builds the spirv-unknown-unknown sysroot. I'm still working on the PR that uses it, but as some final feedback it would be nice if you could pass rustc flags as right now I need to build in two steps. First building the core sysroot, and then building the compiler builtins sysroot with the core sysroot available through --sysroot, and I'm manually setting a RUSTFLAGS environment variable before each invocation to do that. Related to that it would be nice if this function accepted a builder for all the options that you have graciously added. 😄

[DianaNites]

Can you elaborate on setting rustc flags? Why do you need to build in two steps right now? CompilerBuiltins implies Core, do they need different flags? Not quite sure how I could support that, they all depend on each other.

As for a builder-style interface, I can add that

[XAMPPRocky]

Sure, here's the relevant code. compiler_builtins is built using core, but the new sysroot isn't visible to rustc. So I need to pass the --sysroot flag to the second build with the location of the first.

fn rust_flags(backend: &Path, sysroot: Option<&Path>) -> String {
    use std::fmt::Write;
    let mut buf = String::new();
    let _ = write!(&mut buf, "-Zcodegen-backend={}", backend.display());
    if let Some(sysroot) = sysroot {
        let _ = write!(&mut buf, " --sysroot={}", sysroot.display());
    }

    buf
}

// Build core without `--sysroot`
env::set_var("RUSTFLAGS", rust_flags(&backend, None));
let sysroot = cargo_sysroot::build_sysroot_with(
    None,
    &target_dir,
    TARGET_NAME.as_ref(),
    &src,
    cargo_sysroot::Sysroot::Core,
    false,
).map_err(|e| eyre!("{}", e))?;

// Build compiler_builtins with `spirv_unknown_unknown` sysroot
env::set_var("RUSTFLAGS", rust_flags(&backend, Some(&sysroot)));
let sysroot = cargo_sysroot::build_sysroot_with(
    None,
    &target_dir,
    TARGET_NAME.as_ref(),
    &src,
    cargo_sysroot::Sysroot::CompilerBuiltins,
    false,
).map_err(|e| eyre!("{}", e))?;
env::remove_var("RUSTFLAGS");

[DianaNites]

I'm more confused. Why are you building core explicitly? CompilerBuiltins implies Core. It builds core itself, because it depends on core? It can't be built separately?

[XAMPPRocky]

Apparently, because if I comment out the second one and change Core to CompilerBuiltins I get the following error. I should have a PR that you can try out by Friday.

    Updating crates.io index
   Compiling compiler_builtins v0.1.36
   Compiling core v0.0.0 (/Users/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core)
error[E0463]: can't find crate for `core`
  |
  = note: the `spirv-unknown-unknown` target may not be installed

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error: could not compile `compiler_builtins`

To learn more, run the command again with --verbose.

[DianaNites]

Well, that shouldn't happen.. I'll see whats causing that

[DianaNites]

Okay, that should fix it. Thats embarrassing. A simple mixed conditional.

[XAMPPRocky]

I'm now getting a Couldn't copy host tools to sysroot error when building any Sysroot using the same code as above. These are the components I have installed.

cargo-x86_64-apple-darwin
clippy-x86_64-apple-darwin
llvm-tools-preview-x86_64-apple-darwin
rust-docs-x86_64-apple-darwin
rust-src
rust-std-x86_64-apple-darwin
rustc-x86_64-apple-darwin
rustc-dev-x86_64-apple-darwin
rustfmt-x86_64-apple-darwin

[DianaNites]

Thats.. never happened before. No idea how to reproduce that.

Is that the entire error message? It's not supposed to be..

[DianaNites]

Don't think I can do anything about this without more details on the error, so the above commit should help with that. Maybe it's something apple specific? But I have no way to test that..

Maybe Rust changed some layouts? What nightly are you on?

I'm on rustc 1.49.0-nightly (1773f60ea 2020-11-08), and the latest seems to be 1.49.0-nightly (5404efc28 2020-11-11), but I can't update since it, and last two versions, are missing the miri component.

[DianaNites]

Hmm but wait, it worked for you before didnt it? But I shouldn't have changed anything that could break it..

[DianaNites]

Not seeing anything that could've affected it between 2adf1bd and now..

[XAMPPRocky]

It is the full error, I'll try some different nightlies on my end and get back to you.

Compiling core v0.0.0 (/Users/.rustup/toolchains/nightly-2020-11-09-x86_64-apple-darwin/lib/rustlib/src/rust/library/core)
Compiling Sysroot v0.0.0 (/Users/src/rust-gpu2/examples/runners/wgpu/target/debug/build/example-runner-wgpu-fa478400e7da9d34/out/spirv/sysroot)
    Finished release [optimized] target(s) in 15.81s
Error: Couldn't copy host tools to sysroot

[DianaNites]

Thats.. extremely odd. Are you using the latest master, b8b0c4d added some more context hopefully to improve it

[XAMPPRocky]

Okay it seems cargo update fixed it. Thank you again so much for your help with this! 😄

[XAMPPRocky]

Actually I found the issue. If I have a rust-toolchain with a specific nightly (e.g. nightly-2020-11-09) it fails to copy the tools, using just nightly works.

[DianaNites]

Oh great! With that I should be able to reproduce and fix it to.. not fail.

[DianaNites]

And with the above commit, it should be fixed.

[DianaNites]

Added a builder API in 9d61d0b, including the ability to set rustc flags.

[XAMPPRocky]

@DianaNites Thank you again for all this work! It's very stable now, would you be up for releasing this version on crates.io?

[DianaNites]

I can do a crates.io release, yeah! But what about #6?

[DianaNites]

Published as v0.8.0

[XAMPPRocky]

I can do a crates.io release, yeah! But what about #6?

Great, the library API itself is still good, and for now we can just not run clippy on those examples in my PR. We can track progress in more specific issues.