Apply rustflags when probing rustc

[roblabla]

We should apply the RUSTFLAGS environment variable when probing with rustc. The RUSTFLAGS might get important flags, such as the sysroot, when doing cross-platform building with tools such as cargo-xbuild or Xargo.

Without this PR, autocfg will fail when used in concert with xargo/cargo-xbuild, with the following problem:

error[E0463]: can't find crate for `std`
  |
  = note: the `i386-unknown-none-user` target may not be installed

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error[E0463]: can't find crate for `core`
  |
  = note: the `i386-unknown-none-user` target may not be installed

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
warning: autocfg could not probe for `std`

The reason for this error is that autocfg is not providing the sysroot to rustc, and as such it can't find the core/std crates. To fix this, we should pass the sysroot, which is usually found in the RUSTFLAGS.

[cuviper]

CI failed only on the rustfmt check.

It would be nice to have some test for this though. I think you could do this with an integration test under tests/, and use autocfg itself as a probed "sysroot" crate with something like RUSTFLAGS="-L target/debug/deps.

It would be nice to see the Vec<String> cached in AutoCfg.

At a higher level, I think we need to be careful not to over-apply these. Here's what Cargo says:

https://github.com/rust-lang/cargo/blob/69aea5b6f69add7c51cca939a79644080c0b0ba0/src/cargo/core/compiler/build_context/target_info.rs#L379-L395

/// Acquire extra flags to pass to the compiler from various locations.
///
/// The locations are:
///
///  - the `RUSTFLAGS` environment variable
///
/// then if this was not found
///
///  - `target.*.rustflags` from the manifest (Cargo.toml)
///  - `target.cfg(..).rustflags` from the manifest
///
/// then if neither of these were found
///
///  - `build.rustflags` from the manifest
///
/// Note that if a `target` is specified, no args will be passed to host code (plugins, build
/// scripts, ...), even if it is the same as the target.

First, note that we're only covering the RUSTFLAGS environment variable here, but at least that does have the highest priority. It would be a lot more challenging to cover the other ways rustflags are set. I want to keep autocfg highly compatible itself, which makes me wary of adding a toml dependency or the like, nevermind the add complexity of finding the right config file at all. The comment says from the manifest, but I think they mean .cargo/config.

But that final note is the real concern -- if autocfg is used to build something for the host (like another build-dependency or proc-macro plugin), then we shouldn't be applying RUSTFLAGS. We can't see whether Cargo had --target specified though! I think the best we might do is approximate by seeing if env TARGET is different than HOST at all, and only apply RUSTFLAGS then.

So I don't think we have enough information to perfectly copy what Cargo does, but good enough? Maybe we can also add a method AutoCfg::rustflags to give explicit control to the user, but we need the implicit behavior to Do The Right Thing as much as possible for broad crate coverage.

[cuviper]

But that final note is the real concern -- if autocfg is used to build something for the host (like another build-dependency or proc-macro plugin), then we shouldn't be applying RUSTFLAGS. We can't see whether Cargo had --target specified though! I think the best we might do is approximate by seeing if env TARGET is different than HOST at all, and only apply RUSTFLAGS then.

A more implicit idea is to look at OUT_DIR. With --target, it will be something like target/$TARGET/debug/build/..., otherwise just target/debug/build/...

The "debug" might also be "release" or in the future a custom profile name. It also might not be called target/ if CARGO_TARGET_DIR says otherwise. Windows also makes / annoying.

Anyway, maybe we can combine these for a heuristic: apply RUSTFLAGS if TARGET != HOST or OUT_DIR contains $CARGO_TARGET_DIR/$TARGET.

[roblabla]

Thanks for the review! I fixed the rustfmt, cached the flags on AutoCfg struct creation, and added a simple test. Which is failing on 1.10 and earlier for some reason ?_?.

I want to keep autocfg highly compatible itself, which makes me wary of adding a toml dependency or the like, nevermind the add complexity of finding the right config file at all. The comment says from the manifest, but I think they mean .cargo/config.

Yeah that's definitely the .cargo/config file they're talking about, the comment is wrong ^^'. I hoped there would be something similar to cargo metadata we could use to get information about the config, but it doesn't seem like there is. So, I guess for now, using the RUSTFLAGS environment variable only is the most strategic thing to do.

Maybe in the long run, we could get cargo to add a CARGO_RUSTFLAGS env variable for build scripts that contain the accumulated flags that cargo would use? This would also fix the following concern.

Anyway, maybe we can combine these for a heuristic: apply RUSTFLAGS if TARGET != HOST or OUT_DIR contains $CARGO_TARGET_DIR/$TARGET.

Will try implementing this.

[roblabla]

Hmm, it doesn't seem to be possible to detect when --target is specified with those heuristics. When autocfg is used in a build dep, the build dep sees the following env variables:

host=x86_64-unknown-linux-gnu, target=x86_64-unknown-linux-gnu, out_dir=/home/roblabla/Dropbox/dev/src/rust/hello_world/target/release/build/cratename-c4c54b59052b8613/out, cargo_target_dir=target

Which is exactly the same as when we run it directly with cargo build.

(Also, note that cargo_target_dir is not passed to build scripts, see rust-lang/cargo#7325, so I had to approximate it).

[roblabla]

Looks like somewhere between 1.10.0 and 1.15.0, rustc changed the location of test dependencies - on older versions they're in target/debug directly. I pushed a fix.

WRT --target detection, I'm unsure how to proceed. I checked the environment differences between cargo build and cargo build --target=thumbv6m-none-eabi when building a build dependency. They're exactly the same. So I don't think it's doable at all. We should probably open an issue in cargo to make this possible in the future.

[cuviper]

Hmm. There's really only one case where cargo doesn't apply RUSTFLAGS, which is compiling builds scripts and plugins when --target is specified. I think it would be a real problem if we accidentally do apply flags here. Cargo does apply the flags for target code, even if the target matches the host, and for all code when target is not specified. So yeah, I think we can't really tell between "host code with target" or "any code without target".

So I think the best incremental improvement we can make is to just use the flags when we see TARGET != HOST. That should be correct for cross-compiling situations like xargo, and current native compilations will be unaffected.