HOST environment variable should not be cleared in all cases.

[thomcc]

Hi. We're getting the following error while building compiletest_rs under sccache on x86_64:

error: environment variable `HOST` not defined
   --> /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/compiletest_rs-0.9.0/src/common.rs:397:19
    |
397 |             host: env!("HOST").to_string(),
    |                   ^^^^^^^^^^^^
    |
    = note: this error originates in the macro `env` (in Nightly builds, run with -Z macro-backtrace for more info)

error: could not compile `compiletest_rs` due to previous error

This should be set by compiletest_rs's build script: https://github.com/Manishearth/compiletest-rs/blob/master/build.rs#L6 -- nothing turns on the rustc feature, but doesn't seem to be when building for x86_64 (perplexingly, it works for aarch64).

Potentially irrelevant background: In https://github.com/tcdi/plrust we're trying to introduce sccache into our build system (CC @BradyBonnette who is actually doing most of the work here). As part of that project, we have a custom rustc_driver-user that uses uses the compiletest_rs crate crate to test itself (usage is in https://github.com/tcdi/plrust/blob/main/plrustc/plrustc/tests/uitests.rs, in case that helps you find the actual code and such).

[drahnr]

#1663 is the root of this, it unconditionally removes the HOST env from the environment. It was not transplanted to the remote, to remove any accidental potential to create host dependent compile artifacts.

[thomcc]

Hmm, that's unfortunate. So compiletest-rs can't be used with sccache then, unless it's fixed.

[drahnr]

I think removing host might have been an overshoot, but I am not sure in which contexts it's used. I definitely could see potential for abuse though, especially in the C/CXX domain.

[drahnr]

Note that I have to double check, if i.e. cargo:rerun-if-env-changed=HOST would re-include it from a build.rs and if that'd be applicable in your case.

[thomcc]

HOST is one of the vars cargo sets for build scripts: https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-build-scripts, so generally rerun-if-env-changed shouldn't be used for it (but perhaps sccache changes this?)

[drahnr]

I think the patch originates from an environment whereHOST is used to denote the name of the host where a rpm package is built (I don't quite remember the context) and hence was a source of randomness. Now build.rs is a different story. It'd be good to be able to discriminate between build script and regular binary, but I did not investigate that.

[thomcc]

Yeah, so cargo sets HOST when running build.rs's. This build.rs then propagates it to the actual build of the crate, but it's not picked up (because you clear it, I guess).

[drahnr]

I'd be very happy to review a PR in that direction, it's non trivial though and definitely requires some digging in how cargo handles the different targets. I won't have time to investigate anytime soon.

[thomcc]

It's probably easier to fix compiletest-rs in this case: Manishearth/compiletest-rs#269

That said I would say that stripping HOST in this manner is almost certainly overeager, given that much more rust code will care about it in this case.

[thomcc]

I fixed this in compiletest-rs, so I renamed the issue.