Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests/ui/io-checks fail locally on master #110794

Closed
WaffleLapkin opened this issue Apr 25, 2023 · 6 comments · Fixed by #110801
Closed

tests/ui/io-checks fail locally on master #110794

WaffleLapkin opened this issue Apr 25, 2023 · 6 comments · Fixed by #110801
Assignees
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc

Comments

@WaffleLapkin
Copy link
Member

The tests/ui/io-checks/inaccessbile-temp-dir.rs test fails for me locally on the master branch (and was failing for a while now). I can of course ignore it, but it's annoying. tests/ui/io-checks/non-ice-error-on-worker-io-fail.rs also fails if run after/with inaccessbile-temp-dir.rs.

It looks like inaccessbile-temp-dir.rs successfully creates a temp directory even if it's parent does not exist:

output

:~/rust-b (master); git log -1 --oneline
91b61a4ad61 (HEAD -> master, upstream/master) Auto merge of #110389 - mazong1123:add-shortcut-for-grisu3, r=Mark-Simulacrum
:~/rust-b (master); ls /does-not-exist
ls: cannot access '/does-not-exist': No such file or directory
:~/rust-b (master) [2]; x t tests/ui/io-checks/inaccessbile-temp-dir.rs
Building bootstrap
    Finished dev [unoptimized] target(s) in 0.06s
Building stage0 library artifacts (aarch64-unknown-linux-gnu)
    Finished release [optimized + debuginfo] target(s) in 0.18s
Building compiler artifacts (stage0 -> stage1, aarch64-unknown-linux-gnu)
    Finished release [optimized + debuginfo] target(s) in 0.29s
Assembling stage1 compiler
Building stage1 library artifacts (aarch64-unknown-linux-gnu)
    Blocking waiting for file lock on package cache
    Blocking waiting for file lock on package cache
    Finished release [optimized + debuginfo] target(s) in 0.21s
Building stage0 tool compiletest (aarch64-unknown-linux-gnu)
    Finished release [optimized + debuginfo] target(s) in 0.19s
Check compiletest suite=ui mode=ui (aarch64-unknown-linux-gnu -> aarch64-unknown-linux-gnu)

running 1 tests
F

failures:

---- [ui] tests/ui/io-checks/inaccessbile-temp-dir.rs stdout ----

error: ui test compiled successfully!
status: exit status: 0
command: "/home/gh-WaffleLapkin/rust-b/build/aarch64-unknown-linux-gnu/stage1/bin/rustc" "/home/gh-WaffleLapkin/rust-b/tests/ui/io-checks/inaccessbile-temp-dir.rs" "-Zthreads=1" "--sysroot" "/home/gh-WaffleLapkin/rust-b/build/aarch64-unknown-linux-gnu/stage1" "--target=aarch64-unknown-linux-gnu" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Zdeduplicate-diagnostics=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/home/gh-WaffleLapkin/rust-b/build/aarch64-unknown-linux-gnu/test/ui/io-checks/inaccessbile-temp-dir" "-A" "unused" "-Crpath" "-Cdebuginfo=0" "-Lnative=/home/gh-WaffleLapkin/rust-b/build/aarch64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/home/gh-WaffleLapkin/rust-b/build/aarch64-unknown-linux-gnu/test/ui/io-checks/inaccessbile-temp-dir/auxiliary" "-Z" "temps-dir=/does-not-exist/output"
stdout: none
stderr: none



failures:
    [ui] tests/ui/io-checks/inaccessbile-temp-dir.rs

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 14869 filtered out; finished in 60.70ms

Some tests failed in compiletest suite=ui mode=ui host=aarch64-unknown-linux-gnu target=aarch64-unknown-linux-gnu
Build completed unsuccessfully in 0:00:03
:~/rust-b (master) [1]; ls /does-not-exist
output
:~/rust-b (master); 

This causes the failure (as the test is expected to not compile) and also creates the directory, which non-ice-error-on-worker-io-fail.rs then observes and also fails.

This happens for me on the dev-desktop-eu-1.

cc @fee1-dead iirc you had the same problems
cc @cuviper you changed those tests at some point, so maybe you know something

@WaffleLapkin WaffleLapkin added the A-testsuite Area: The testsuite used to check the correctness of rustc label Apr 25, 2023
@KittyBorgX
Copy link
Member

Summing up findings from discord, This test is wrong in its entirity since it gambles on permissions. The underlying rustc subcommand creates the dir if you have enough permission to create in / so it would fail. Otherwise, the directory is never created due to insufficient user permission, so the test passes.

To sum it up, the test passed (for me) since I did not have enough permission to create a new directory (/does-not-exist/output) in /:
image

@WaffleLapkin
Copy link
Member Author

The test assumes that the compiler uses create_dir and so it must fail to create /does-not-exist/another-dir, but this is actually not the case:

if let Some(ref dir) = sess.io.temps_dir {
if fs::create_dir_all(dir).is_err() {
sess.emit_fatal(errors::TempsDirError);
}
}

The test happens to pass in the CI and for many developers because you generally can't create a directory in /. However this is not always the case (for example I happen to run tests in chroot).

@jyn514 suggests that the proper fix for the test would be for the test to create an inaccessible directory, basically making a run-make analog to chmod 000 some-dir && rustc -Z temps-dir=some-dir/foo

@WaffleLapkin WaffleLapkin self-assigned this Apr 25, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 25, 2023
Fix `ui/io-checks/inaccessbile-temp-dir.rs` test

Fixes rust-lang#110794

r? `@jyn514`
@cuviper
Copy link
Member

cuviper commented Apr 26, 2023

the proper fix for the test would be for the test to create an inaccessible directory, basically making a run-make analog to chmod 000 some-dir && rustc -Z temps-dir=some-dir/foo

That still doesn't work if you're the root user, but I guess that's not what you're doing in your chroot?

You could use a path like /proc/... where even root can't create anything, but there's no OS-agnostic path like that.

@WaffleLapkin
Copy link
Member Author

That still doesn't work if you're the root user, but I guess that's not what you're doing in your chroot?

I guess my point is that inaccessbile-temp-dir.rs was broken on root anyway (because root has access to /, same as in chroot). The explanation in the test was because of the original non-ice-error-on-worker-io-fail.rs test it looks like, which works because it uses -o which requires at least the "parent must exist".

So change /dev/null -> /does-not-exist/output

  • non-ice-error-on-worker-io-fail.rs now works on root, but doesn't work on chroot anymore
  • inaccessbile-temp-dir.rs does not work on root/chroot, but also doesn't clobber /dev/null

My change:

  • non-ice-error-on-worker-io-fail.rs unchanged
  • inaccessbile-temp-dir.rs now works in chroot

Sooo, while inaccessbile-temp-dir.rs fails on root, since it doesn't have global effects (clobbering /dev/null as it originally did), I don't think it's a big deal. IMO you are not supposed to run tests on root (but it's also bad for tests to have global effects)...

@cuviper
Copy link
Member

cuviper commented Apr 26, 2023

Yeah, that's fair -- we don't have to support root, but it should at least do no harm.

While you're poking at that, maybe you can fix the filename spelling to "inaccessible"?

@WaffleLapkin
Copy link
Member Author

Fixed the spelling, thanks.

@bors bors closed this as completed in 9a3258f Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants