Skip to content

compiletest: Don't set TARGET for non run-make tests #147929

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
Merged
merged 2 commits into from
Oct 21, 2025
Merged

compiletest: Don't set TARGET for non run-make tests #147929

merged 2 commits into from
Oct 21, 2025

Conversation

@Zalathar
Copy link
Contributor

There are a few tests that were using TARGET to quietly do nothing on i586 targets, but it's cleaner to just add support for //@ ignore-i586 instead.

This lets us get rid of an unsafe env::set_var in compiletest, which really should have been setting the environment variable on individual build/run subprocess commands anyway.

@Zalathar
Add support for //@ ignore-i586
ff0c096 
There are a few tests that were trying to skip i586 targets via the `TARGET`
environment variable instead, so better to just add support for the directive.
@Zalathar
Don't set TARGET for non run-make tests
a581154 
If something like this is needed again in the future, please set environment
variables for the build and run subprocesses, instead of setting them on the
compiletest process.
@rustbot
Copy link
Collaborator

rustbot commented Oct 21, 2025

compiletest directives have been modified. Please add or update docs for the
new or modified directive in src/doc/rustc-dev-guide/.

Some changes occurred in src/tools/compiletest

cc @jieyouxu

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 21, 2025
@rustbot rustbot added 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. labels Oct 21, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 21, 2025

r? @jieyouxu

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@Zalathar
Copy link
Contributor Author

This was previously tested at #147896, but let's run some more try jobs on this PR just to be sure.

@bors try jobs=dist-i586-gnu-i586-i686-musl,i686-gnu-1,i686-msvc-1,test-various

rust-bors bot added a commit that referenced this pull request Oct 21, 2025
@rust-bors
Auto merge of #147929 - Zalathar:target, r=<try>
ca68066 
compiletest: Don't set `TARGET` for non run-make tests

try-job: dist-i586-gnu-i586-i686-musl
try-job: i686-gnu-1
try-job: i686-msvc-1
try-job: test-various
@rust-bors

This comment has been minimized.

Sign in to view
@rust-bors
Copy link

rust-bors bot commented Oct 21, 2025

☀️ Try build successful (CI)
Build commit: ca68066 (ca68066f4d5b47d9d0925fe95dbf77a0f5ea49f1, parent: c7a635f33c5fbd1ead110243a2f4a5f0561d79b0)

jieyouxu
jieyouxu approved these changes Oct 21, 2025
View reviewed changes
Copy link
Member

@jieyouxu jieyouxu left a comment •
edited by rustbot
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

View changes since this review

@jieyouxu
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Oct 21, 2025

📌 Commit a581154 has been approved by jieyouxu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 21, 2025
@matthiaskrgr matthiaskrgr mentioned this pull request Oct 21, 2025
Merged
bors added a commit that referenced this pull request Oct 21, 2025
@bors
Auto merge of #147943 - matthiaskrgr:rollup-rd0gb6y, r=matthiaskrgr
695857b 
Rollup of 3 pull requests

Successful merges:

 - #147929 (compiletest: Don't set `TARGET` for non run-make tests)
 - #147930 (motor: Add new `set_times` stubs)
 - #147934 (compiletest: More directive handling tweaks)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9671020 into rust-lang:master Oct 21, 2025
12 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Oct 21, 2025
rust-timer added a commit that referenced this pull request Oct 21, 2025
@rust-timer
Unrolled build for #147929
f35d90d 
Rollup merge of #147929 - Zalathar:target, r=jieyouxu

compiletest: Don't set `TARGET` for non run-make tests

There are a few tests that were using `TARGET` to quietly do nothing on `i586` targets, but it's cleaner to just add support for `//@ ignore-i586` instead.

This lets us get rid of an unsafe `env::set_var` in compiletest, which really should have been setting the environment variable on individual build/run subprocess commands anyway.

- The original code and tests were introduced way back in #39068
@Zalathar Zalathar deleted the target branch October 21, 2025 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jieyouxu jieyouxu jieyouxu approved these changes

Assignees

@jieyouxu jieyouxu

Labels

A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

Projects

None yet

Milestone

1.92.0

Development

Successfully merging this pull request may close these issues.

4 participants

@Zalathar @rustbot @jieyouxu @bors