prefer CARGO_TARGET_TMPDIR as location for temporary build files

[RalfJung]

Based on discussion at rust-lang/cargo#14125 (comment).

I wasn't sure whether to use build-time or run-time env var lookup. The docs only mention CARGO_TARGET_TMPDIR being set at build time for tests. Does ui_test support being built outside of tests? The docs don't mention CARGO_TARGET_DIR being set at all so no idea who sets that or when, I figured I'd just keep it as a fallback.

[RalfJung]

Looking at it again, cargo indeed never sets CARGO_TARGET_DIR. So this logic seems to just have been an attempt at replicating cargo's logic for finding the target dir: CARGO_TARGET_DIR with a fallback to ./target/. This will be incorrect if the user uses --config or sets the target dir via config.toml.

I think ideally we'd just remove that logic entirely and rely on CARGO_TARGET_TMPDIR instead.

[oli-obk]

So this logic seems to just have been an attempt at replicating cargo's logic for finding the target dir: ``

Correct

I think ideally we'd just remove that logic entirely and rely on CARGO_TARGET_TMPDIR instead

Sounds right

[RalfJung]

Hm, thinking about it, I don't know if we can use CARGO_TARGET_TMPDIR. I assume this is only set for the test crate, not for its dependencies? Cc @epage

[epage]

Could you help me understand how this is run? Is this a test runner, similar to cargo test? Is it called by cargo in some way or does it only call into cargo?

[RalfJung]

This crate is typically a dev-dependency of some other crate, and then that other crate will call us in its tests. See for instance https://github.com/rust-lang/miri/blob/master/tests/ui.rs as a typical user of this crate.

[epage]

Yeah, testing libraries are a bit of a pain.

In #t-cargo > cargo_bin_exe and tests @ 💬, I'm exploring whether to make some of these available at runtime to make things easier. Otherwise, it'll just require always passing it in like you currently do.

I guess runtime access would also be good for not rebuilding just because the target directory moved though I'm unsure how often that happens in practice. CI seems like the most likely culprit.

[epage]

In the mean time, you have to put the caller in charge of providing these paths, similar to CARGO_BIN_EXE_miri

[RalfJung]

Providing it for dependencies or at runtime seem like good options; passing it in is kind if annoying. In our case it requires a sember bump. Oli, any preference for what to do? Abort this PR or have the user pass in the value?

[oli-obk]

requiring everyone to pass sth in that is almost always just ./target doesn't seem very nice. I think it's fine to require those folk to manually adjust their Config struct instead of requiring everyone to do so.

[RalfJung]

It's different people though -- the user who sets if a different target-dir and build-dir, vs the author of the ui_test-using test suite.

If someone configures the target-dir in config.toml, I think currently ui_test will just create a ./target dir anyway, disregarding the user's choice. If they set CARGO_TARGET_DIR to a global directory, we'll have a single shared target dir for all ui_test instances, which sounds like it could have "fun" interactions.

[oli-obk]

oh, I didn't think about config.toml further up the directory graph than the crate itself...

yea that's annoying. I guess we could change the Config constructors to be macros instead so they get evaluated at the testing crate site instead of in the ui_test dependency where the env vars aren't set