-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Remove erroneous normalization step in tests/run-make/linker-warning
#146982
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
Conversation
.expected_file("short-error.txt") | ||
.actual_text("(linker error)", out.stderr()) | ||
.normalize(r#"/rustc[^/_-]*/"#, "/rustc/") | ||
.normalize(r#"^/rustc[^/_-]*/"#, "/rustc/") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me locally, the test as a whole passes just fine if I remove this normalization step altogether but I guess there are some environments where there's a rustcXXX
(tempdir?) in the (fake?) rootdir?
Or is the current change of prepending ^
as suggested in #146977 (comment) actually wrong as it makes this normalization step effectively unreachable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, that was what this was supposed to normalize. I was thinking about the /rustc/<commit>/
for source files, but those wouldn't have matched anyway. So yeah, my suggestion will probably break something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, so I've now completely removed that normalization step because short-error.txt
doesn't contain any occurrence of /rustc/
so I'm led to believe that it never fired anyway (except on my machine) since the subsequent normalizations didn't depend on the ↦/rustc/
normalization I think (so the lack of /rustc/
in the golden file should really mean "never fired").
Does that make sense to you or am I misunderstanding something? In which environments or circumstances would it be possible for rustc to pass a /rustc/<commit>/
path to the linker (but even if it did that the test would fail I believe)? Anything I could test via a try job?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the .normalize(r#""[^"]*\/symbols.o""#, "\"/symbols.o\"").normalize(r#""[^"]*\/raw-dylibs""#, "\"/raw-dylibs\"")
below made the tempdir normalization obsolete.
This comment was marked as outdated.
This comment was marked as outdated.
Make normalization step in `tests/run-make/linker-warning` more robust try-job: test-various
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Okay I hope that this one is a suitable try job (host @bors try jobs=x86_64-gnu |
This comment has been minimized.
This comment has been minimized.
Make normalization step in `tests/run-make/linker-warning` more robust try-job: x86_64-gnu
404d8ee
to
bc37dd4
Compare
tests/run-make/linker-warning
more robusttests/run-make/linker-warning
more robust
tests/run-make/linker-warning
more robusttests/run-make/linker-warning
@bors r+ |
…=bjorn3 Remove erroneous normalization step in `tests/run-make/linker-warning` Fixes rust-lang#146977. r? bjorn3 or reassign
Rollup of 14 pull requests Successful merges: - #145067 (RawVecInner: add missing `unsafe` to unsafe fns) - #145277 (Do not materialise X in [X; 0] when X is unsizing a const) - #145973 (Add `std` support for `armv7a-vex-v5`) - #146667 (Add an attribute to check the number of lanes in a SIMD vector after monomorphization) - #146735 (unstably constify float mul_add methods) - #146737 (f16_f128: enable some more tests in Miri) - #146766 (Add attributes for #[global_allocator] functions) - #146905 (llvm: update remarks support on LLVM 22) - #146982 (Remove erroneous normalization step in `tests/run-make/linker-warning`) - #147005 (Small string formatting cleanup) - #147007 (Explicitly note `&[SocketAddr]` impl of `ToSocketAddrs`) - #147008 (bootstrap.py: Respect build.jobs while building bootstrap tool) - #147013 (rustdoc: Fix documentation for `--doctest-build-arg`) - #147015 (Use `LLVMDisposeTargetMachine`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 14 pull requests Successful merges: - rust-lang/rust#145067 (RawVecInner: add missing `unsafe` to unsafe fns) - rust-lang/rust#145277 (Do not materialise X in [X; 0] when X is unsizing a const) - rust-lang/rust#145973 (Add `std` support for `armv7a-vex-v5`) - rust-lang/rust#146667 (Add an attribute to check the number of lanes in a SIMD vector after monomorphization) - rust-lang/rust#146735 (unstably constify float mul_add methods) - rust-lang/rust#146737 (f16_f128: enable some more tests in Miri) - rust-lang/rust#146766 (Add attributes for #[global_allocator] functions) - rust-lang/rust#146905 (llvm: update remarks support on LLVM 22) - rust-lang/rust#146982 (Remove erroneous normalization step in `tests/run-make/linker-warning`) - rust-lang/rust#147005 (Small string formatting cleanup) - rust-lang/rust#147007 (Explicitly note `&[SocketAddr]` impl of `ToSocketAddrs`) - rust-lang/rust#147008 (bootstrap.py: Respect build.jobs while building bootstrap tool) - rust-lang/rust#147013 (rustdoc: Fix documentation for `--doctest-build-arg`) - rust-lang/rust#147015 (Use `LLVMDisposeTargetMachine`) r? `@ghost` `@rustbot` modify labels: rollup
Fixes #146977.
r? bjorn3 or reassign