Repository navigation
Conversation
This comment has been minimized.
This comment has been minimized.
f80987f to
0a7b6e1
Compare
rami3l
changed the title
15e2425 to
fce7233
Compare
| Ok(exe_dir.parent().unwrap().to_owned()) | ||
|
|
||
| let mut comps = curr_exe_dir.components().collect::<Vec<_>>(); | ||
| let retained_comps = match &comps[..] { |
There was a problem hiding this comment.
Can we do something that's less dependent on output directory layout? E.g. can we use CARGO_TARGET_TMPDIR or so?
There was a problem hiding this comment.
@ChrisDenton This is complicated, because neither CARGO_TARGET_TMPDIR nor CARGO_BIN_EXE_rustup-init is guaranteed to be set on both integration tests and unit tests, and this function is a common infrastructure of both sides, meaning if we need a fallback anyway on one side, it might just as well be the actual implementation :( Maybe you have a better idea?
There was a problem hiding this comment.
Well my better idea would be "maybe we should ask the cargo team" even if as a feature request. Because I know we aren't the only ones that need some scratch space for test infra.
But short of that, manually implementing it is fine. I just don't love it.
fce7233 to
32f410d
Compare
Continuation of #5211:
Status Quo
As stated above, the rustup test suite currently puts its temporary rustup installations under
./target/.This is of course very convenient, but it also has a practical reason: each installation is directly derived from
CARGO_BIN_EXE_rustup-initvia hard links to that file, otherwise each test will have its own actual copy ofrustup-init[EXE]which is, needless to say, unacceptable.The key point here is that hard links are not allowed to go across devices, otherwise we'll hit the
io::ErrorKind::CrossesDeviceserror and the whole test fixture will not work.Proposed Solution
The proposed solution is in two parts:
The first part restores the part that has been broken by Rust v1.100.0 WRT the
exe_test_dir()helper function. Before Rust v1.100, it used to return./targetor./target/<tuple>(which we will call<base>from now on), but now it points to<base>/(debug|release)/build/rustup/<unit-hash>/. This means that shared states that existed in the past are now separated by unit hash. With this fix, it regains the ability to find the same old<base>and thus we are out ofcargo's way.The second part evaluates whether the OS' tmp dir
<ostmp>can be used for testing, which is currently only possible withoutio::ErrorKind::CrossesDevices. To do so, it tries to create an empty "probe" directory under<base>andrename()it to<ostmp>/rustup-test. If this succeeds, that directory is used as the return value ofexe_test_dir()instead of the original<base>. This way, we can have the OS manage the test temporaries for us.