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

Compiletest should not inherit all host RUSTFLAGS #136960

Merged
merged 1 commit into from
Feb 14, 2025

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Feb 13, 2025

I told @rhelmot to do this in #134913. But it's not correct; compiletest shouldn't inherit RUSTFLAGS at all.

Pass a single new --host-rustcflags to compiletest instead, without overwriting any existing arguments.

Fixes the following failure, which only happens when building llvm from source and then running x test --stage 1 ui-fulldeps:

diff --git a/tests/ui-fulldeps/fluent-messages/test.stderr b/tests/ui-fulldeps/fluent-messages/test.stderr
index 0b3bb14ce51..978ac46c5a2 100644
--- a/tests/ui-fulldeps/fluent-messages/test.stderr
+++ b/tests/ui-fulldeps/fluent-messages/test.stderr
@@ -1,3 +1,8 @@
+warning[E0602]: unknown lint: `linker_messages`
+   |
+   = note: requested on the command line with `-A linker_messages`
+   = note: `#[warn(unknown_lints)]` on by default

See https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/.E2.9C.94.20unknown.20lint.3A.20.60linker_messages.60.20when.20blessing.20tests.20on.20.2E.2E.2E for more context.

I told rhelmot to do this in rust-lang#134913. But it's not correct; compiletest
shouldn't inherit RUSTFLAGS at all.

Pass a single new --host-rustcflags to compiletest instead, without overwriting any
existing arguments.

Fixes the following failure, which only happens when building llvm from
source and then running `x test --stage 1 ui-fulldeps`:
```
diff --git a/tests/ui-fulldeps/fluent-messages/test.stderr b/tests/ui-fulldeps/fluent-messages/test.stderr
index 0b3bb14ce51..978ac46c5a2 100644
--- a/tests/ui-fulldeps/fluent-messages/test.stderr
+++ b/tests/ui-fulldeps/fluent-messages/test.stderr
@@ -1,3 +1,8 @@
+warning[E0602]: unknown lint: `linker_messages`
+   |
+   = note: requested on the command line with `-A linker_messages`
+   = note: `#[warn(unknown_lints)]` on by default
```
@rustbot
Copy link
Collaborator

rustbot commented Feb 13, 2025

r? @Kobzol

rustbot has assigned @Kobzol.
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

@rustbot rustbot added 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. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Feb 13, 2025
@jieyouxu
Copy link
Member

r? jieyouxu

@rustbot rustbot assigned jieyouxu and unassigned Kobzol Feb 13, 2025
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Hm, right. Didn't catch this, but not inheriting all host RUSTFLAGS does seem more correct. Thanks!

@jieyouxu jieyouxu added the A-compiletest Area: The compiletest test runner label Feb 13, 2025
@jieyouxu
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 13, 2025

📌 Commit 66ebee4 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 Feb 13, 2025
@jyn514
Copy link
Member Author

jyn514 commented Feb 13, 2025

@shepmaster tested this and found it didn't actually solve his problem. so that's unfortunate. but i still think the new code is more correct.

workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Feb 14, 2025
Compiletest should not inherit all host RUSTFLAGS

I told `@rhelmot` to do this in rust-lang#134913. But it's not correct; compiletest shouldn't inherit RUSTFLAGS at all.

Pass a single new --host-rustcflags to compiletest instead, without overwriting any existing arguments.

Fixes the following failure, which only happens when building llvm from source and then running `x test --stage 1 ui-fulldeps`:
```
diff --git a/tests/ui-fulldeps/fluent-messages/test.stderr b/tests/ui-fulldeps/fluent-messages/test.stderr
index 0b3bb14ce51..978ac46c5a2 100644
--- a/tests/ui-fulldeps/fluent-messages/test.stderr
+++ b/tests/ui-fulldeps/fluent-messages/test.stderr
`@@` -1,3 +1,8 `@@`
+warning[E0602]: unknown lint: `linker_messages`
+   |
+   = note: requested on the command line with `-A linker_messages`
+   = note: `#[warn(unknown_lints)]` on by default
```

See https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/.E2.9C.94.20unknown.20lint.3A.20.60linker_messages.60.20when.20blessing.20tests.20on.20.2E.2E.2E for more context.
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Feb 14, 2025
Fix `x test --stage 1 ui-fulldeps` on macOS (until the next beta bump)

"stage 1" for fulldeps means "compile with stage 0, link against stage 1". But this code wanted to switch on the compiler that's building, not the compiler that's being tested. Fix the check.

Previously, it would fail with a warning about linker-messages:
```
--- stderr -------------------------------
warning[E0602]: unknown lint: `linker_messages`
   |
   = note: requested on the command line with `-A linker_messages`
   = note: `#[warn(unknown_lints)]` on by default
```

cc https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/unknown.20lint.3A.20.60linker_messages.60.20when.20blessing.20tests.20on.20.2E.2E.2E, rust-lang#136960
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Feb 14, 2025
Fix `x test --stage 1 ui-fulldeps` on macOS (until the next beta bump)

"stage 1" for fulldeps means "compile with stage 0, link against stage 1". But this code wanted to switch on the compiler that's building, not the compiler that's being tested. Fix the check.

Previously, it would fail with a warning about linker-messages:
```
--- stderr -------------------------------
warning[E0602]: unknown lint: `linker_messages`
   |
   = note: requested on the command line with `-A linker_messages`
   = note: `#[warn(unknown_lints)]` on by default
```

cc https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/unknown.20lint.3A.20.60linker_messages.60.20when.20blessing.20tests.20on.20.2E.2E.2E, rust-lang#136960
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 14, 2025
…kingjubilee

Rollup of 11 pull requests

Successful merges:

 - rust-lang#136863 (rework rigid alias handling )
 - rust-lang#136869 (Fix diagnostic when using = instead of : in let binding)
 - rust-lang#136895 (debuginfo: Set bitwidth appropriately in enum variant tags)
 - rust-lang#136928 (eagerly prove WF when resolving fully qualified paths)
 - rust-lang#136941 (Move `llvm.ccache` to `build.ccache`)
 - rust-lang#136950 (rustdoc: use better, consistent SVG icons for scraped examples)
 - rust-lang#136957 (coverage: Eliminate more counters by giving them to unreachable nodes)
 - rust-lang#136960 (Compiletest should not inherit all host RUSTFLAGS)
 - rust-lang#136962 (unify LLVM version finding logic)
 - rust-lang#136970 (ci: move `x86_64-gnu-debug` job to the free runner)
 - rust-lang#136973 (Fix `x test --stage 1 ui-fulldeps` on macOS (until the next beta bump))

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit bde2091 into rust-lang:master Feb 14, 2025
6 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 14, 2025
Rollup merge of rust-lang#136973 - jyn514:fulldeps-stage1, r=jieyouxu

Fix `x test --stage 1 ui-fulldeps` on macOS (until the next beta bump)

"stage 1" for fulldeps means "compile with stage 0, link against stage 1". But this code wanted to switch on the compiler that's building, not the compiler that's being tested. Fix the check.

Previously, it would fail with a warning about linker-messages:
```
--- stderr -------------------------------
warning[E0602]: unknown lint: `linker_messages`
   |
   = note: requested on the command line with `-A linker_messages`
   = note: `#[warn(unknown_lints)]` on by default
```

cc https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/unknown.20lint.3A.20.60linker_messages.60.20when.20blessing.20tests.20on.20.2E.2E.2E, rust-lang#136960
@rustbot rustbot added this to the 1.86.0 milestone Feb 14, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 14, 2025
Rollup merge of rust-lang#136960 - jyn514:compiletest-args, r=jieyouxu

Compiletest should not inherit all host RUSTFLAGS

I told ``@rhelmot`` to do this in rust-lang#134913. But it's not correct; compiletest shouldn't inherit RUSTFLAGS at all.

Pass a single new --host-rustcflags to compiletest instead, without overwriting any existing arguments.

Fixes the following failure, which only happens when building llvm from source and then running `x test --stage 1 ui-fulldeps`:
```
diff --git a/tests/ui-fulldeps/fluent-messages/test.stderr b/tests/ui-fulldeps/fluent-messages/test.stderr
index 0b3bb14ce51..978ac46c5a2 100644
--- a/tests/ui-fulldeps/fluent-messages/test.stderr
+++ b/tests/ui-fulldeps/fluent-messages/test.stderr
``@@`` -1,3 +1,8 ``@@``
+warning[E0602]: unknown lint: `linker_messages`
+   |
+   = note: requested on the command line with `-A linker_messages`
+   = note: `#[warn(unknown_lints)]` on by default
```

See https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/.E2.9C.94.20unknown.20lint.3A.20.60linker_messages.60.20when.20blessing.20tests.20on.20.2E.2E.2E for more context.
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-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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants