Skip to content

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Dec 26, 2024

when running `tests/ui/panics/default-backtrace-ice.rs locally it gave this error:

failures:

---- [ui] tests/ui/panics/default-backtrace-ice.rs stdout ----
Saved the actual stderr to "/home/jyn/src/rust3/build/x86_64-unknown-linux-gnu/test/ui/panics/default-backtrace-ice/default-backtrace-ice.stderr"
diff of stderr:

7
8	aborting due to `-Z treat-err-as-bug=1`
9	stack backtrace:
-	(end_short_backtrace)
-	(begin_short_backtrace)
-	(end_short_backtrace)
-	(begin_short_backtrace)
+	      [... omitted 22 frames ...]
+

(note that you must not use --bless; we previously did not have an error annotation to verify it was a full backtrace instead of a short backtrace.)

this is a regression from setting RUST_BACKTRACE=1 by default in #134743. we need to turn off the new behavior when running UI tests so that they reflect our dist compiler. normally that's done by checking sess.unstable_opts.ui_testing, but this happens extremely early in the compiler before we've expanded arg files. do an extremely simple hack that doesn't work in all cases - we don't need it to work in all cases, only when running UI tests.

cc #129658 (comment)

r? @jieyouxu

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 26, 2024
@jyn514
Copy link
Member Author

jyn514 commented Dec 26, 2024

an alternative fix would be to allow either BACKTRACE=1 or BACKTRACE=full in this test, by doing more normalization. i have mixed feelings about that; i do think it's valuable to test that we're setting full in dist builds.

when running `tests/ui/panics/default-backtrace-ice.rs locally it gave this error:
```
failures:

---- [ui] tests/ui/panics/default-backtrace-ice.rs stdout ----
Saved the actual stderr to "/home/jyn/src/rust3/build/x86_64-unknown-linux-gnu/test/ui/panics/default-backtrace-ice/default-backtrace-ice.stderr"
diff of stderr:

7
8	aborting due to `-Z treat-err-as-bug=1`
9	stack backtrace:
-	(end_short_backtrace)
-	(begin_short_backtrace)
-	(end_short_backtrace)
-	(begin_short_backtrace)
+	      [... omitted 22 frames ...]
+
```

this is a regression from setting RUST_BACKTRACE=1 by default. we need to turn off the new behavior when running UI tests so that they reflect our dist compiler. normally that's done by checking `sess.unstable_opts.ui_testing`, but this happens extremely early in the compiler before we've expanded arg files. do an extremely simple hack that doesn't work in all cases - we don't need it to work in all cases, only when running UI tests.
@jyn514 jyn514 force-pushed the rustc-dev-short-backtraces branch from ea662a8 to 801c1d8 Compare December 26, 2024 00:47
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.

Thanks. Seems a bit wonky, but I don't immediately have a better alternative besides adding yet another perma-unstable flag or env var.

@jieyouxu
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Dec 26, 2024

📌 Commit 801c1d8 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 Dec 26, 2024
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Dec 26, 2024
… r=jieyouxu

fix default-backtrace-ice test

when running `tests/ui/panics/default-backtrace-ice.rs locally it gave this error:
```
failures:

---- [ui] tests/ui/panics/default-backtrace-ice.rs stdout ----
Saved the actual stderr to "/home/jyn/src/rust3/build/x86_64-unknown-linux-gnu/test/ui/panics/default-backtrace-ice/default-backtrace-ice.stderr"
diff of stderr:

7
8	aborting due to `-Z treat-err-as-bug=1`
9	stack backtrace:
-	(end_short_backtrace)
-	(begin_short_backtrace)
-	(end_short_backtrace)
-	(begin_short_backtrace)
+	      [... omitted 22 frames ...]
+
```
(note that you must *not* use --bless; we previously did not have an error annotation to verify it was a full backtrace instead of a short backtrace.)

this is a regression from setting RUST_BACKTRACE=1 by default in rust-lang#134743. we need to turn off the new behavior when running UI tests so that they reflect our dist compiler. normally that's done by checking `sess.unstable_opts.ui_testing`, but this happens extremely early in the compiler before we've expanded arg files. do an extremely simple hack that doesn't work in all cases - we don't need it to work in all cases, only when running UI tests.

cc rust-lang#129658 (comment)

r? `@jieyouxu`
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 26, 2024
Rollup of 4 pull requests

Successful merges:

 - rust-lang#134664 (Account for removal of multiline span in suggestion)
 - rust-lang#134774 (fix default-backtrace-ice test)
 - rust-lang#134781 (Add more `begin_panic` normalizations to panic backtrace tests)
 - rust-lang#134784 (Miri subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Collaborator

bors commented Dec 26, 2024

⌛ Testing commit 801c1d8 with merge 4ed8cf4...

@bors
Copy link
Collaborator

bors commented Dec 26, 2024

☀️ Test successful - checks-actions
Approved by: jieyouxu
Pushing 4ed8cf4 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 26, 2024
@bors bors merged commit 4ed8cf4 into rust-lang:master Dec 26, 2024
7 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 26, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (4ed8cf4): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (secondary 2.6%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.6% [2.6%, 2.6%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Cycles

Results (primary -1.9%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.9% [-1.9%, -1.9%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -1.9% [-1.9%, -1.9%] 1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 765.315s -> 765.011s (-0.04%)
Artifact size: 330.50 MiB -> 330.55 MiB (0.01%)

poliorcetics pushed a commit to poliorcetics/rust that referenced this pull request Dec 26, 2024
…=jieyouxu

fix default-backtrace-ice test

when running `tests/ui/panics/default-backtrace-ice.rs locally it gave this error:
```
failures:

---- [ui] tests/ui/panics/default-backtrace-ice.rs stdout ----
Saved the actual stderr to "/home/jyn/src/rust3/build/x86_64-unknown-linux-gnu/test/ui/panics/default-backtrace-ice/default-backtrace-ice.stderr"
diff of stderr:

7
8	aborting due to `-Z treat-err-as-bug=1`
9	stack backtrace:
-	(end_short_backtrace)
-	(begin_short_backtrace)
-	(end_short_backtrace)
-	(begin_short_backtrace)
+	      [... omitted 22 frames ...]
+
```
(note that you must *not* use --bless; we previously did not have an error annotation to verify it was a full backtrace instead of a short backtrace.)

this is a regression from setting RUST_BACKTRACE=1 by default in rust-lang#134743. we need to turn off the new behavior when running UI tests so that they reflect our dist compiler. normally that's done by checking `sess.unstable_opts.ui_testing`, but this happens extremely early in the compiler before we've expanded arg files. do an extremely simple hack that doesn't work in all cases - we don't need it to work in all cases, only when running UI tests.

cc rust-lang#129658 (comment)

r? `@jieyouxu`
@jyn514 jyn514 deleted the rustc-dev-short-backtraces branch December 27, 2024 15:43
poliorcetics pushed a commit to poliorcetics/rust that referenced this pull request Dec 28, 2024
…=jieyouxu

fix default-backtrace-ice test

when running `tests/ui/panics/default-backtrace-ice.rs locally it gave this error:
```
failures:

---- [ui] tests/ui/panics/default-backtrace-ice.rs stdout ----
Saved the actual stderr to "/home/jyn/src/rust3/build/x86_64-unknown-linux-gnu/test/ui/panics/default-backtrace-ice/default-backtrace-ice.stderr"
diff of stderr:

7
8	aborting due to `-Z treat-err-as-bug=1`
9	stack backtrace:
-	(end_short_backtrace)
-	(begin_short_backtrace)
-	(end_short_backtrace)
-	(begin_short_backtrace)
+	      [... omitted 22 frames ...]
+
```
(note that you must *not* use --bless; we previously did not have an error annotation to verify it was a full backtrace instead of a short backtrace.)

this is a regression from setting RUST_BACKTRACE=1 by default in rust-lang#134743. we need to turn off the new behavior when running UI tests so that they reflect our dist compiler. normally that's done by checking `sess.unstable_opts.ui_testing`, but this happens extremely early in the compiler before we've expanded arg files. do an extremely simple hack that doesn't work in all cases - we don't need it to work in all cases, only when running UI tests.

cc rust-lang#129658 (comment)

r? `@jieyouxu`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants