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

More principled tempdir usage by and between rustc, rustdoc and compiletest #138475

Open
scottmcm opened this issue Mar 14, 2025 · 4 comments
Open
Labels
A-compiletest Area: The compiletest test runner A-test-infra Area: test infrastructure (may span bootstrap/compiletest/more) A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. E-needs-investigation Call for partcipation: This issues needs some investigation to determine current status T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@scottmcm
Copy link
Member

"Specifically, it's somewhere here:

if outputs.outputs.should_link() {
let tmpdir = TempFileBuilder::new()
.prefix("rustc")
.tempdir()
.unwrap_or_else(|error| sess.dcx().emit_fatal(errors::CreateTempDir { error }));
let path = MaybeTempDir::new(tmpdir, sess.opts.cg.save_temps);

"but yeah, in any case, we probably should override that, to point it under build/test/$test_suite_name/$test_suite_revisioned_compare_moded/__temp/
"(E.g. by setting TMP_DIR/TEMP/TMP or whatever the env vars were called)
"Can you open an E-needs-investigation issue?" ~ @jieyouxu https://discord.com/channels/273534239310479360/957720175619215380/1349912066772963469

Spotted in #138157 (comment), the bors job failed with

--- stderr -------------------------------
error: couldn't create a temp dir: Access is denied. (os error 5) at path "C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\rustcfzdxGW"

error: aborting due to 1 previous error

It would be nice if those stayed in the build/target directory, which is more likely to have a defender exclusion (and thus more likely to not have that access error). It would also be helpful to stay on the Dev Drive on Windows 11 if people have that set up, rather than use C: which typically has the most extra filesystem access costs (from filters and such).

And it'd just be nice not to have a bazillion of these left over after running tests a bunch :)

@scottmcm scottmcm added the E-needs-investigation Call for partcipation: This issues needs some investigation to determine current status label Mar 14, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 14, 2025
@jieyouxu
Copy link
Member

jieyouxu commented Mar 14, 2025

Yeah, this behavior is definitely a bit strange and sub-optimal. Addressing this properly would need fixes from two directions:

  1. The compiler itself needs to be modified to consistently use temp dirs that can be controlled by the user (through flags and whatever).
  2. compiletest should also specify temp dirs to be somewhere under the build directory, and not random places e.g. under an entirely different drive,

@jieyouxu jieyouxu added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-compiletest Area: The compiletest test runner C-bug Category: This is a bug. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Mar 14, 2025
@jieyouxu jieyouxu added A-testsuite Area: The testsuite used to check the correctness of rustc A-test-infra Area: test infrastructure (may span bootstrap/compiletest/more) labels Mar 14, 2025
@scottmcm
Copy link
Member Author

scottmcm commented Mar 14, 2025

Which reminds me I should go clean this out again. I have pages and pages of these:

Image

(All of them empty, at least of the ones I randomly checked.)

EDIT: I checked with find, apparently 10390 empty rust* directories in temp.

@jieyouxu
Copy link
Member

@scottmcm that might not be rustc, that might be rustdoc:

fn get_doctest_dir() -> io::Result<TempDir> {
TempFileBuilder::new().prefix("rustdoctest").tempdir()
}

@jieyouxu
Copy link
Member

jieyouxu commented Mar 14, 2025

I'm going to tag this with rustdoc too, because while we also should control the tempdir env vars explicitly from compiletest, rustdoc may need to be fixed itself like rustc (I haven't looked deeply, lmw if this is some compiletest-only issue).

@jieyouxu jieyouxu added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Mar 14, 2025
@jieyouxu jieyouxu changed the title Avoid writing to AppData\Local\Temp in compiletest (maybe ever?) More principled tempdir usage by and between rustc, rustdoc and compiletest Mar 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner A-test-infra Area: test infrastructure (may span bootstrap/compiletest/more) A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. E-needs-investigation Call for partcipation: This issues needs some investigation to determine current status T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
Development

No branches or pull requests

3 participants