Skip to content

[libc++][test] extend -linux-gnu XFAIL to cover all of the -linux targets #129140

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

Merged
merged 1 commit into from
Feb 28, 2025

Conversation

pawosm-arm
Copy link
Contributor

The default triple of Amazon Linux on AArch64 is aarch64-amazon-linux, see issue highlighded by PR #109263, somewhat serious linker issues are encountered if any other triple is being used.

Unfortunately, this makes XFAIL lines like XFAIL: target=aarch64{{.*}}-linux-gnu ineffective, making it impossible to complete all of the check-cxx without failures.

…gets

The default triple of Amazon Linux on AArch64 is aarch64-amazon-linux,
see issue highlighded by PR llvm#109263, somewhat serious linker issues
are encountered if any other triple is being used.

Unfortunately, this makes XFAIL lines like
`XFAIL: target=aarch64{{.*}}-linux-gnu` ineffective, making it
impossible to complete all of the check-cxx without failures.
@pawosm-arm pawosm-arm requested a review from a team as a code owner February 27, 2025 23:13
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Feb 27, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 27, 2025

@llvm/pr-subscribers-libcxx

Author: Paul Osmialowski (pawosm-arm)

Changes

The default triple of Amazon Linux on AArch64 is aarch64-amazon-linux, see issue highlighded by PR #109263, somewhat serious linker issues are encountered if any other triple is being used.

Unfortunately, this makes XFAIL lines like XFAIL: target=aarch64{{.*}}-linux-gnu ineffective, making it impossible to complete all of the check-cxx without failures.


Full diff: https://github.com/llvm/llvm-project/pull/129140.diff

10 Files Affected:

  • (modified) libcxx/test/std/input.output/iostream.format/std.manip/setfill_wchar_max.pass.cpp (+1-1)
  • (modified) libcxx/test/std/re/re.alg/re.alg.match/awk.locale.pass.cpp (+1-1)
  • (modified) libcxx/test/std/re/re.alg/re.alg.match/basic.locale.pass.cpp (+1-1)
  • (modified) libcxx/test/std/re/re.alg/re.alg.match/ecma.locale.pass.cpp (+1-1)
  • (modified) libcxx/test/std/re/re.alg/re.alg.match/extended.locale.pass.cpp (+1-1)
  • (modified) libcxx/test/std/re/re.alg/re.alg.search/awk.locale.pass.cpp (+1-1)
  • (modified) libcxx/test/std/re/re.alg/re.alg.search/basic.locale.pass.cpp (+1-1)
  • (modified) libcxx/test/std/re/re.alg/re.alg.search/ecma.locale.pass.cpp (+1-1)
  • (modified) libcxx/test/std/re/re.alg/re.alg.search/extended.locale.pass.cpp (+1-1)
  • (modified) libcxx/test/std/re/re.traits/lookup_collatename.pass.cpp (+1-1)
diff --git a/libcxx/test/std/input.output/iostream.format/std.manip/setfill_wchar_max.pass.cpp b/libcxx/test/std/input.output/iostream.format/std.manip/setfill_wchar_max.pass.cpp
index 9d4126153cc23..82842a75827ac 100644
--- a/libcxx/test/std/input.output/iostream.format/std.manip/setfill_wchar_max.pass.cpp
+++ b/libcxx/test/std/input.output/iostream.format/std.manip/setfill_wchar_max.pass.cpp
@@ -16,7 +16,7 @@
 
 // XFAIL: target={{.*}}-windows{{.*}} && libcpp-abi-version=1
 // XFAIL: target=armv{{7|8}}{{l?}}{{.*}}-linux-gnueabihf && libcpp-abi-version=1
-// XFAIL: target=aarch64{{.*}}-linux-gnu && libcpp-abi-version=1
+// XFAIL: target=aarch64{{.*}}-linux{{.*}} && libcpp-abi-version=1
 
 #include <iomanip>
 #include <ostream>
diff --git a/libcxx/test/std/re/re.alg/re.alg.match/awk.locale.pass.cpp b/libcxx/test/std/re/re.alg/re.alg.match/awk.locale.pass.cpp
index 57b8c13aa3c14..879597b2f80fd 100644
--- a/libcxx/test/std/re/re.alg/re.alg.match/awk.locale.pass.cpp
+++ b/libcxx/test/std/re/re.alg/re.alg.match/awk.locale.pass.cpp
@@ -18,7 +18,7 @@
 
 // TODO: investigation needed
 // TODO(netbsd): incomplete support for locales
-// XFAIL: target={{.*}}-linux-gnu{{.*}}, netbsd, freebsd
+// XFAIL: target={{.*}}-linux{{.*}}, netbsd, freebsd
 // REQUIRES: locale.cs_CZ.ISO8859-2
 
 #include <regex>
diff --git a/libcxx/test/std/re/re.alg/re.alg.match/basic.locale.pass.cpp b/libcxx/test/std/re/re.alg/re.alg.match/basic.locale.pass.cpp
index 430d35fe739e5..59fb1c48e15d3 100644
--- a/libcxx/test/std/re/re.alg/re.alg.match/basic.locale.pass.cpp
+++ b/libcxx/test/std/re/re.alg/re.alg.match/basic.locale.pass.cpp
@@ -22,7 +22,7 @@
 //                  regex_constants::match_flag_type flags = regex_constants::match_default);
 
 // TODO: investigation needed
-// XFAIL: target={{.*}}-linux-gnu{{.*}}, freebsd
+// XFAIL: target={{.*}}-linux{{.*}}, freebsd
 
 #include <regex>
 #include <cassert>
diff --git a/libcxx/test/std/re/re.alg/re.alg.match/ecma.locale.pass.cpp b/libcxx/test/std/re/re.alg/re.alg.match/ecma.locale.pass.cpp
index b512fa9b5fcf8..0a966759eac3b 100644
--- a/libcxx/test/std/re/re.alg/re.alg.match/ecma.locale.pass.cpp
+++ b/libcxx/test/std/re/re.alg/re.alg.match/ecma.locale.pass.cpp
@@ -22,7 +22,7 @@
 //                  regex_constants::match_flag_type flags = regex_constants::match_default);
 
 // TODO: investigation needed
-// XFAIL: target={{.*}}-linux-gnu{{.*}}, freebsd
+// XFAIL: target={{.*}}-linux{{.*}}, freebsd
 
 #include <regex>
 #include <cassert>
diff --git a/libcxx/test/std/re/re.alg/re.alg.match/extended.locale.pass.cpp b/libcxx/test/std/re/re.alg/re.alg.match/extended.locale.pass.cpp
index 472dc19680263..87ff1e5b6ef12 100644
--- a/libcxx/test/std/re/re.alg/re.alg.match/extended.locale.pass.cpp
+++ b/libcxx/test/std/re/re.alg/re.alg.match/extended.locale.pass.cpp
@@ -22,7 +22,7 @@
 //                  regex_constants::match_flag_type flags = regex_constants::match_default);
 
 // TODO: investigation needed
-// XFAIL: target={{.*}}-linux-gnu{{.*}}, freebsd
+// XFAIL: target={{.*}}-linux{{.*}}, freebsd
 
 #include <regex>
 #include <cassert>
diff --git a/libcxx/test/std/re/re.alg/re.alg.search/awk.locale.pass.cpp b/libcxx/test/std/re/re.alg/re.alg.search/awk.locale.pass.cpp
index 9125df404b1de..c4b211e613bec 100644
--- a/libcxx/test/std/re/re.alg/re.alg.search/awk.locale.pass.cpp
+++ b/libcxx/test/std/re/re.alg/re.alg.search/awk.locale.pass.cpp
@@ -22,7 +22,7 @@
 //                  regex_constants::match_flag_type flags = regex_constants::match_default);
 
 // TODO: investigation needed
-// XFAIL: target={{.*}}-linux-gnu{{.*}}, freebsd
+// XFAIL: target={{.*}}-linux{{.*}}, freebsd
 
 #include <regex>
 #include <cassert>
diff --git a/libcxx/test/std/re/re.alg/re.alg.search/basic.locale.pass.cpp b/libcxx/test/std/re/re.alg/re.alg.search/basic.locale.pass.cpp
index f85b6a40ce129..56cf2e6a61ff3 100644
--- a/libcxx/test/std/re/re.alg/re.alg.search/basic.locale.pass.cpp
+++ b/libcxx/test/std/re/re.alg/re.alg.search/basic.locale.pass.cpp
@@ -22,7 +22,7 @@
 //                  regex_constants::match_flag_type flags = regex_constants::match_default);
 
 // TODO: investigation needed
-// XFAIL: target={{.*}}-linux-gnu{{.*}}, freebsd
+// XFAIL: target={{.*}}-linux{{.*}}, freebsd
 
 #include <regex>
 #include <cassert>
diff --git a/libcxx/test/std/re/re.alg/re.alg.search/ecma.locale.pass.cpp b/libcxx/test/std/re/re.alg/re.alg.search/ecma.locale.pass.cpp
index aa9441cb3e58f..4655a5c2e0ee6 100644
--- a/libcxx/test/std/re/re.alg/re.alg.search/ecma.locale.pass.cpp
+++ b/libcxx/test/std/re/re.alg/re.alg.search/ecma.locale.pass.cpp
@@ -22,7 +22,7 @@
 //                  regex_constants::match_flag_type flags = regex_constants::match_default);
 
 // TODO: investigation needed
-// XFAIL: target={{.*}}-linux-gnu{{.*}}, freebsd
+// XFAIL: target={{.*}}-linux{{.*}}, freebsd
 
 #include <regex>
 #include <cassert>
diff --git a/libcxx/test/std/re/re.alg/re.alg.search/extended.locale.pass.cpp b/libcxx/test/std/re/re.alg/re.alg.search/extended.locale.pass.cpp
index 9746e45f29da5..7bc8a537ca228 100644
--- a/libcxx/test/std/re/re.alg/re.alg.search/extended.locale.pass.cpp
+++ b/libcxx/test/std/re/re.alg/re.alg.search/extended.locale.pass.cpp
@@ -22,7 +22,7 @@
 //                  regex_constants::match_flag_type flags = regex_constants::match_default);
 
 // TODO: investigation needed
-// XFAIL: target={{.*}}-linux-gnu{{.*}}, freebsd
+// XFAIL: target={{.*}}-linux{{.*}}, freebsd
 
 #include <regex>
 #include <cassert>
diff --git a/libcxx/test/std/re/re.traits/lookup_collatename.pass.cpp b/libcxx/test/std/re/re.traits/lookup_collatename.pass.cpp
index 178979d5b9ce8..3cbbaef9c81b5 100644
--- a/libcxx/test/std/re/re.traits/lookup_collatename.pass.cpp
+++ b/libcxx/test/std/re/re.traits/lookup_collatename.pass.cpp
@@ -23,7 +23,7 @@
 //   lookup_collatename(ForwardIterator first, ForwardIterator last) const;
 
 // TODO: investigation needed
-// XFAIL: target={{.*}}-linux-gnu{{.*}}
+// XFAIL: target={{.*}}-linux{{.*}}
 
 #include <regex>
 #include <iterator>

Copy link
Collaborator

@DavidSpickett DavidSpickett left a comment

Choose a reason for hiding this comment

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

Given that none of these have any details suggesting they pass on non-gnu systems, this seems fine.

Some of them will have been -gnu purely because the buildbots at the time were all -gnu (probably still are).

@github-project-automation github-project-automation bot moved this from Needs Triage to Needs Merge in LLVM Release Status Feb 28, 2025
@pawosm-arm
Copy link
Contributor Author

The failures on Windows and FreeBSD are unrelated.

@pawosm-arm pawosm-arm merged commit c93dc58 into llvm:main Feb 28, 2025
76 of 85 checks passed
@github-project-automation github-project-automation bot moved this from Needs Merge to Done in LLVM Release Status Feb 28, 2025
@pawosm-arm
Copy link
Contributor Author

/cherry-pick c93dc58

@llvmbot
Copy link
Member

llvmbot commented Feb 28, 2025

/pull-request #129225

@philnik777
Copy link
Contributor

@pawosm-arm @DavidSpickett please wait for @llvm/reviewers-libcxx approval before landing libc++ patches. This really doesn't seem like the kind of patch that is time-sensitive.

@DavidSpickett
Copy link
Collaborator

Yes sure, I should have made @pawosm-arm aware of that.

@pawosm-arm
Copy link
Contributor Author

pawosm-arm commented Feb 28, 2025

I'm sorry for the confusion, this needs to be reverted (see #129271) and I'll try to prepare something afresh.

@pawosm-arm pawosm-arm removed this from the LLVM 20.X Release milestone Feb 28, 2025
pawosm-arm added a commit to pawosm-arm/llvm-project that referenced this pull request Feb 28, 2025
…inux targets (llvm#129140)"

The effect of this commit is too broad and may affect also those
variants of Linux systems on which the affected test cases are known
to pass.

An alternative version of this commit will be prepared afresh.

This reverts commit c93dc58.
@pawosm-arm pawosm-arm deleted the amazon-not-gnu branch February 28, 2025 18:38
cheezeburglar pushed a commit to cheezeburglar/llvm-project that referenced this pull request Feb 28, 2025
…gets (llvm#129140)

The default triple of Amazon Linux on AArch64 is aarch64-amazon-linux,
see issue highlighded by PR llvm#109263, somewhat serious linker issues are
encountered if any other triple is being used.

Unfortunately, this makes XFAIL lines like:
`XFAIL: target=aarch64{{.*}}-linux-gnu` ineffective,
making it impossible to complete all of the check-cxx without failures.
@ldionne
Copy link
Member

ldionne commented Feb 28, 2025

I'll echo what @philnik777 said here: please don't bypass the libc++ reviewers group. I'm not too comfortable with folks who don't have a history of contributing to libc++ merging commits themselves into the codebase (libcxx/ specifically, of course) without approval from the usual reviewers group. I know you had absolutely no wrong intentions, it was a small patch, and that's okay. What I'm expressing is discomfort with the fact that this is even technically feasible, since Github gives us means to easily avoid that.

CC @tstellar FWIW, this is a great example of something where different commit access policies could be helpful. In this case, there was no intention of wrongdoing, but unclear policies around who's expected to merge something led to a small and mostly harmless, but incorrect patch being merged (since it had to be reverted). I only stumbled upon that because I git log -- libcxx almost every day, and I think it's unfortunate that I've been feeling the need to do that in order to fulfill my duties as a code owner (which says specifically that I'm responsible for every change being reviewed appropriately).

Anyway, there's no harm done here but I do think this is something small and inconsequential that we can learn from in terms of setting project policies.

ldionne pushed a commit that referenced this pull request Feb 28, 2025
…inux targets (#129140)" (#129271)

The effect of this commit is too broad and may affect also those
variants of Linux systems on which the affected test cases are known to
pass.

An alternative version of this commit will be prepared afresh.

This reverts commit c93dc58.
jph-13 pushed a commit to jph-13/llvm-project that referenced this pull request Mar 21, 2025
…inux targets (llvm#129140)" (llvm#129271)

The effect of this commit is too broad and may affect also those
variants of Linux systems on which the affected test cases are known to
pass.

An alternative version of this commit will be prepared afresh.

This reverts commit c93dc58.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
Development

Successfully merging this pull request may close these issues.

5 participants