rust-osdev / cargo-xbuild

Automatically cross-compiles the sysroot crates core, compiler_builtins, and alloc.
Apache License 2.0
258 stars 25 forks source link

Doesn't work with build.target-dir set for Cargo #51

Closed upsuper closed 4 years ago

upsuper commented 4 years ago

I have build.target-dir in ~/.cargo/config set to some specific directory so that

However, this breaks cargo-xbuild because it seems to rely on that target directory is always in the project directory.

There are two possible way to fix as far as I can see:

  1. read the target directory from cargo metadata, or
  2. explicitly set CARGO_TARGET_DIR when it calls into cargo.

I would prefer the first so that the global configuration is respected.

reynoldsbd commented 4 years ago

This has been tripping me up, too.

For what it's worth (and to improve search results for anybody else hitting this issue), here's the error message I'm seeing:

> cargo xrustc --target=aarch64-unknown-none-softfloat --release
    Updating crates.io index
  Downloaded compiler_builtins v0.1.24
   Compiling core v0.0.0 (C:\Users\Bobby\.rustup\toolchains\nightly-2019-12-20-x86_64-pc-windows-msvc\lib\rustlib\src\rust\src\libcore)
   Compiling compiler_builtins v0.1.24
   Compiling rustc-std-workspace-core v1.99.0 (C:\Users\Bobby\.rustup\toolchains\nightly-2019-12-20-x86_64-pc-windows-msvc\lib\rustlib\src\rust\src\tools\rustc-std-workspace-core)
   Compiling alloc v0.0.0 (C:\Users\Bobby\AppData\Local\Temp\xargo.UCP7I5BJLghG)
    Finished release [optimized] target(s) in 14.67s
error: intermittent IO error while iterating directory `C:\Users\Bobby\AppData\Local\Temp\xargo.UCP7I5BJLghG\target\aarch64-unknown-none-softfloat\release\deps`
caused by: IO error for operation on C:\Users\Bobby\AppData\Local\Temp\xargo.UCP7I5BJLghG\target\aarch64-unknown-none-softfloat\release\deps: The system cannot find the path specified. (os error 3)
caused by: The system cannot find the path specified. (os error 3)

Happens to me on both Windows and Linux. Getting rid of the build.target-dir setting resolves the issue. Thanks, @upsuper!

phil-opp commented 4 years ago

Sorry for not replying earlier, this issue somehow slipped through.

Do I understand it correctly that the project is the following?:

I agree that solution 1 ("read the target directory from cargo metadata") is nicer. However, I don't think that sharing the sysroot between multiple crates currently works (it is rebuilt every time if I remember correctly). So maybe solution 2 ("explicitly set CARGO_TARGET_DIR when it calls into cargo") does lead to fewer problems. What do you think?

upsuper commented 4 years ago

A project specifies a build.target-dir key in a .cargo/config file.

It's not a project specifying so in this case. It's more a global settings. But yes, I think it's possible to put build.target-dir in project's .cargo/config and that should override the global settings.

I agree that solution 1 ("read the target directory from cargo metadata") is nicer. However, I don't think that sharing the sysroot between multiple crates currently works (it is rebuilt every time if I remember correctly). So maybe solution 2 ("explicitly set CARGO_TARGET_DIR when it calls into cargo") does lead to fewer problems. What do you think?

Given that the sysroot is built in a temporary directory, I guess it's reasonable to have the target directory enforced, as there doesn't seem to be lots of things to build, so sharing wouldn't really save much.

Actually your summary reminds me that maybe another way is to put .cargo/config in that temporary directory to override the global settings rather than using CARGO_TARGET_DIR, but either way would work alike I suppose.

phil-opp commented 4 years ago

You're completely right about the temporary directory.

So I think the best way to fix this is to change this line:

https://github.com/rust-osdev/cargo-xbuild/blob/9351d2c90c338526ac015a9fb1a443a03e48824c/src/sysroot.rs#L78

to cmd.env("CARGO_TARGET_DIR", td.join("target")). Unfortunatly I don't have the time to implement and test this myself right now, but I would be happy to merge a pull request for this.

Testing is simple: Just clone the repo, perform the change, and then install the local version through cargo install --path . --force.