Skip to content

tests: Fix duplicated-path-in-error fail with musl #142301

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+10 −0
Open

tests: Fix duplicated-path-in-error fail with musl #142301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/tools/compiletest/src/directives.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -973,6 +973,7 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
"only-mips64",
"only-msp430",
"only-msvc",
"only-musl",
"only-nightly",
"only-nvptx64",
"only-powerpc",
Expand Down
0 ...i/codegen/duplicated-path-in-error.stderr → ...degen/duplicated-path-in-error.gnu.stderr
View file
Open in desktop
File renamed without changes.
2 changes: 2 additions & 0 deletions tests/ui/codegen/duplicated-path-in-error.musl.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
error: couldn't load codegen backend /non-existing-one.so: Error loading shared library /non-existing-one.so: No such file or directory

7 changes: 7 additions & 0 deletions tests/ui/codegen/duplicated-path-in-error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
//@ revisions: musl gnu
//@ only-linux
//@ ignore-cross-compile because this relies on host libc behaviour
//@ compile-flags: -Zcodegen-backend=/non-existing-one.so
//@[gnu] only-gnu
//@[musl] only-musl

// This test ensures that the error of the "not found dylib" doesn't duplicate
// the path of the dylib.
Copy link
Member

Choose a reason for hiding this comment

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

then on the ERROR down here we will prefix it. the current one will be //[gnu] and the new one will be //[musl], containing the messages we are expecting, so like this:

Suggested change
// the path of the dylib.
// the path of the dylib.
// other stuff I can't comment on because GitHub reviews aren't great
//[gnu]~? ERROR couldn't load codegen backend /non-existing-one.so
//[musl]~? ERROR idk, you're the one making the PR

Copy link
Contributor Author

@Gelbpunkt Gelbpunkt Jun 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Nice, thanks for the hints! I think I got it working as intended now (edit: woops forgot one change, sec)

Copy link
Member

Choose a reason for hiding this comment

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

Still hasn't been resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still hasn't been resolved

How exactly? It is resolved AFAICT

Copy link
Member

Choose a reason for hiding this comment

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

Ah right, my bad, prefix match 🤦

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a case to be made to make the entire test only-linux && only-gnu since, while it is testing the error output with a nonexistent backend, the description is rather that it tests for the path not to be duplicated, which it is on musl...

Copy link
Member

@jieyouxu jieyouxu Jul 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The original test introduced in #121978 was an anti-regression test for glibc code path's duplicate path error message, so I'm fine with limiting it to gnu-only. But this is fine too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I do that instead then or leave it as it is now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, hm. That original issue duplicated the path in the part that isn't the dlopen error, so I guess it should be fine to keep it as is

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, the current version is fine.

//
// glibc and musl have different dlopen error messages, so the expected error
// message differs between the two.

fn main() {}

Expand Down
Loading