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.

Sign up for GitHub

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

Jump to bottom

run-make-support: rename tmp_dir() and clarify its semantics #125726

Closed
jieyouxu opened this issue May 29, 2024 · 1 comment · Fixed by #126097
Closed

run-make-support: rename tmp_dir() and clarify its semantics #125726

jieyouxu opened this issue May 29, 2024 · 1 comment · Fixed by #126097
Assignees
@jieyouxu
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jieyouxu
Copy link
Member

Most command wrappers assume output artifacts will be emitted to tmp_dir() (which calculates TMPDIR as is used in tools.mk), which is a scratchpad directory that is intended to be unique for each run-make test so they don't interfere with each other, and that output artifact emission is contained to the directory. Typically command wrappers will have --out-dir=tmp_dir() presets.

The naming of tmp_dir() however is pretty bad -- it being a "temporary directory" is not to be confused with env::temp_dir.

Do note, however, that tools.mk setting TMPDIR has other effects:

On Unix, returns the value of the TMPDIR environment variable if it is set

I don't know if this is intentional, but if it is intended to even control where e.g. codegen artifacts are emitted, then setting TMPDIR does nothing on Windows: we also need to set TMP/TEMP on Windows.
@jieyouxu jieyouxu added A-testsuite Area: The testsuite used to check the correctness of rustc T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools C-bug Category: This is a bug. labels May 29, 2024
@jieyouxu jieyouxu self-assigned this May 29, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 29, 2024
@jieyouxu jieyouxu removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 29, 2024
@jieyouxu jieyouxu mentioned this issue May 29, 2024
Merged
@jieyouxu
Copy link
Member Author

jieyouxu commented Jun 3, 2024

In #125827 we named the output artifacts directory rmake_out, so reasonable names could be rmake_out_dir and rmake_out_path, for example.

@jieyouxu jieyouxu mentioned this issue Jun 3, 2024
Merged
@jieyouxu jieyouxu moved this to In progress in Oxidizing run-make tests Jun 6, 2024
@Kobzol Kobzol mentioned this issue Jun 7, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 8, 2024
@bors
Auto merge of rust-lang#126097 - Kobzol:runmake-change-cwd, r=jieyouxu
99c5a83 
Change how runmake v2 tests are executed

This PR makes execution of v2 runmake tests more sane, by executing each test in a temporary directory by default, rather than running it inside `tests/run-make`. This will have.. a lot of conflicts.

Fixes: rust-lang#126080
Closes rust-lang#125726, because it removes `tmp_dir`, lol.

r? `@jieyouxu`
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 8, 2024
@bors
Auto merge of rust-lang#126097 - Kobzol:runmake-change-cwd, r=jieyouxu
a5bdab9 
Change how runmake v2 tests are executed

This PR makes execution of v2 runmake tests more sane, by executing each test in a temporary directory by default, rather than running it inside `tests/run-make`. This will have.. a lot of conflicts.

Fixes: rust-lang#126080
Closes rust-lang#125726, because it removes `tmp_dir`, lol.

r? `@jieyouxu`
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 8, 2024
@bors
Auto merge of rust-lang#126097 - Kobzol:runmake-change-cwd, r=
2c5647e 
Change how runmake v2 tests are executed

This PR makes execution of v2 runmake tests more sane, by executing each test in a temporary directory by default, rather than running it inside `tests/run-make`. This will have.. a lot of conflicts.

Fixes: rust-lang#126080
Closes rust-lang#125726, because it removes `tmp_dir`, lol.

r? `@jieyouxu`

try-job: x86_64-msvc
@bors bors closed this as completed in cfdb617 Jun 8, 2024
@github-project-automation github-project-automation bot moved this from In progress to Done in Oxidizing run-make tests Jun 8, 2024
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Jun 28, 2024
@bors
Auto merge of #126097 - Kobzol:runmake-change-cwd, r=jieyouxu
bb0f5fb 
Change how runmake v2 tests are executed

This PR makes execution of v2 runmake tests more sane, by executing each test in a temporary directory by default, rather than running it inside `tests/run-make`. This will have.. a lot of conflicts.

Fixes: rust-lang/rust#126080
Closes rust-lang/rust#125726, because it removes `tmp_dir`, lol.

r? `@jieyouxu`

try-job: x86_64-msvc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jieyouxu jieyouxu

Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
No open projects
Oxidizing run-make tests
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Change how runmake v2 tests are executed Kobzol/rust
2 participants
@jieyouxu @rustbot