Skip to content

run-make: do not use relative links to refer to compiler/stdlib sources, we should introduce some CHECKOUT_ROOT env var/helper. #126071

New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom
Closed
jieyouxu opened this issue Jun 6, 2024 · 3 comments · Fixed by #126081
Closed

run-make: do not use relative links to refer to compiler/stdlib sources, we should introduce some CHECKOUT_ROOT env var/helper. #126071

jieyouxu opened this issue Jun 6, 2024 · 3 comments · Fixed by #126081
Assignees
@jieyouxu
Labels
A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jieyouxu
Copy link
Member

jieyouxu commented Jun 6, 2024 •
edited
Loading

Some run-make tests refer to stdlib sources with "../../../library/alloc/src/lib.rs" which seems not ideal. @Kobzol suggested that we should introduce a CHECKOUT_ROOT helper to make this less fragile.
@jieyouxu jieyouxu converted this from a draft issue Jun 6, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jun 6, 2024
@jieyouxu jieyouxu self-assigned this Jun 6, 2024
@jieyouxu jieyouxu added C-cleanup Category: PRs that clean code up or issues documenting cleanup. 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. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jun 6, 2024
@jieyouxu jieyouxu changed the title Do not use relative links to refer to compiler/stdlib sources, we should introduce some CHECKOUT_ROOT env var/helper. run-make: do not use relative links to refer to compiler/stdlib sources, we should introduce some CHECKOUT_ROOT env var/helper. Jun 6, 2024
@bjorn3
Copy link
Member

bjorn3 commented Jun 6, 2024 •
edited
Loading

There is already the S env var set by compiletest for the source root.

@jieyouxu jieyouxu added the A-compiletest Area: The compiletest test runner label Jun 6, 2024
@jieyouxu
Copy link
Member Author

jieyouxu commented Jun 6, 2024

There is already the S env var set by compiletest for the source root.

I'm actually dumb, we even already have source_path in the support library:

rust/src/tools/run-make-support/src/lib.rs

Lines 90 to 92 in 50297bb

pub fn source_path() -> PathBuf {
env_var("S").into()
}

@ChrisDenton
Copy link
Member

That's a very descriptive environment variable name, lol

@Kobzol Kobzol mentioned this issue Jun 6, 2024
Merged
@bors bors closed this as completed in af229f5 Jun 7, 2024
@github-project-automation github-project-automation bot moved this from Ready / Needs Design to Done in Oxidizing run-make tests Jun 7, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jun 7, 2024
@rust-timer
Unrolled build for rust-lang#126081
97da297 
Rollup merge of rust-lang#126081 - Kobzol:run-make-relative-paths, r=jieyouxu

Do not use relative paths to Rust source root in run-make tests

Pre-requisite for rust-lang#126080.

Fixes: rust-lang#126071

r? `@jieyouxu`
@Kobzol Kobzol mentioned this issue Jun 7, 2024
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 7, 2024
@matthiaskrgr
Rollup merge of rust-lang#126112 - Kobzol:runmake-source-root, r=jiey…
5e143fa 
…ouxu

Clean up source root in run-make tests

The name `S` isn't exactly the most descriptive, and we also shouldn't need to pass it when building (actually I think that most of the env. vars that we pass to `cargo` here are probably not really needed).

Related issue: rust-lang#126071

r? `@jieyouxu`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 7, 2024
@matthiaskrgr
Rollup merge of rust-lang#126112 - Kobzol:runmake-source-root, r=jiey…
773f4a5 
…ouxu

Clean up source root in run-make tests

The name `S` isn't exactly the most descriptive, and we also shouldn't need to pass it when building (actually I think that most of the env. vars that we pass to `cargo` here are probably not really needed).

Related issue: rust-lang#126071

r? ``@jieyouxu``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 7, 2024
@matthiaskrgr
Rollup merge of rust-lang#126112 - Kobzol:runmake-source-root, r=jiey…
b8e28d1 
…ouxu

Clean up source root in run-make tests

The name `S` isn't exactly the most descriptive, and we also shouldn't need to pass it when building (actually I think that most of the env. vars that we pass to `cargo` here are probably not really needed).

Related issue: rust-lang#126071

r? ```@jieyouxu```
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jun 7, 2024
@rust-timer
Unrolled build for rust-lang#126112
e3fbfea 
Rollup merge of rust-lang#126112 - Kobzol:runmake-source-root, r=jieyouxu

Clean up source root in run-make tests

The name `S` isn't exactly the most descriptive, and we also shouldn't need to pass it when building (actually I think that most of the env. vars that we pass to `cargo` here are probably not really needed).

Related issue: rust-lang#126071

r? ```@jieyouxu```
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees

@jieyouxu jieyouxu

Labels
A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc C-cleanup Category: PRs that clean code up or issues documenting cleanup. 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.

Do not use relative paths to Rust source root in run-make tests Kobzol/rust
4 participants
@ChrisDenton @bjorn3 @jieyouxu @rustbot