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

[libc++] Simplify how the global stream tests are written #66842

Merged
merged 1 commit into from
Sep 20, 2023

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Sep 20, 2023

Instead of relying on Bash, use the builtin Lit commands whenever possible. The motivation is to stop running %t.exe behind Bash, which breaks on macOS 13.5 with SIP enabled because DYLD_LIBRARY_PATH isn't forwarded to the underlying process when running through a protected process.

For more details, see 1.

@ldionne ldionne requested a review from a team as a code owner September 20, 2023 01:03
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Sep 20, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 20, 2023

@llvm/pr-subscribers-libcxx

Changes

Instead of relying on Bash, use the builtin Lit commands whenever possible. The motivation is to stop running %t.exe behind Bash, which breaks on macOS 13.5 with SIP enabled because DYLD_LIBRARY_PATH isn't forwarded to the underlying process when running through a protected process.

For more details, see 1.


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

23 Files Affected:

  • (modified) libcxx/test/std/input.output/iostream.format/print.fun/print.sh.cpp (+2-6)
  • (modified) libcxx/test/std/input.output/iostream.format/print.fun/vprint_nonunicode.sh.cpp (+2-6)
  • (modified) libcxx/test/std/input.output/iostream.format/print.fun/vprint_unicode.sh.cpp (+2-6)
  • (removed) libcxx/test/std/input.output/iostream.objects/check-stderr.sh (-4)
  • (removed) libcxx/test/std/input.output/iostream.objects/check-stdout.sh (-4)
  • (modified) libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/cerr.sh.cpp (+3-3)
  • (modified) libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/cin.sh.cpp (+1-3)
  • (modified) libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/clog.sh.cpp (+3-3)
  • (modified) libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/cout.sh.cpp (+3-3)
  • (removed) libcxx/test/std/input.output/iostream.objects/send-stdin.sh (-3)
  • (removed) libcxx/test/std/input.output/iostream.objects/wide.stream.objects/check-stderr.sh (-5)
  • (removed) libcxx/test/std/input.output/iostream.objects/wide.stream.objects/check-stdout.sh (-5)
  • (removed) libcxx/test/std/input.output/iostream.objects/wide.stream.objects/send-stdin.sh (-4)
  • (modified) libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcerr-imbue.sh.cpp (+3-3)
  • (modified) libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcerr-wide-mode.sh.cpp (+3-3)
  • (modified) libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcerr.sh.cpp (+3-3)
  • (modified) libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcin-imbue.sh.cpp (+1-3)
  • (modified) libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcin-wide-mode.sh.cpp (+2-3)
  • (modified) libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcin.sh.cpp (+1-3)
  • (modified) libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wclog.sh.cpp (+3-3)
  • (modified) libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcout-imbue.sh.cpp (+3-3)
  • (modified) libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcout-wide-mode.sh.cpp (+3-3)
  • (modified) libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcout.sh.cpp (+3-3)
diff --git a/libcxx/test/std/input.output/iostream.format/print.fun/print.sh.cpp b/libcxx/test/std/input.output/iostream.format/print.fun/print.sh.cpp
index 8c06050d6b41833..d348a3b530be316 100644
--- a/libcxx/test/std/input.output/iostream.format/print.fun/print.sh.cpp
+++ b/libcxx/test/std/input.output/iostream.format/print.fun/print.sh.cpp
@@ -7,7 +7,6 @@
 
 // UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
 // UNSUPPORTED: no-filesystem
-// UNSUPPORTED: executor-has-no-bash
 // UNSUPPORTED: GCC-ALWAYS_INLINE-FIXME
 
 // XFAIL: availability-fp_to_chars-missing
@@ -29,12 +28,9 @@
 //
 // The testing is based on the testing for std::cout.
 
-// TODO PRINT Use lit builtin echo
-
-// FILE_DEPENDENCIES: echo.sh
 // RUN: %{build}
-// RUN: %{exec} bash echo.sh -n "1234 一二三四 true 0x0" > %t.expected
-// RUN: %{exec} "%t.exe" > %t.actual
+// RUN: echo -n "1234 一二三四 true 0x0" > %t.expected
+// RUN: %{exec} %t.exe > %t.actual
 // RUN: diff -u %t.actual %t.expected
 
 #include <print>
diff --git a/libcxx/test/std/input.output/iostream.format/print.fun/vprint_nonunicode.sh.cpp b/libcxx/test/std/input.output/iostream.format/print.fun/vprint_nonunicode.sh.cpp
index ca7687150b94c2e..63e1d2ad82b74e2 100644
--- a/libcxx/test/std/input.output/iostream.format/print.fun/vprint_nonunicode.sh.cpp
+++ b/libcxx/test/std/input.output/iostream.format/print.fun/vprint_nonunicode.sh.cpp
@@ -7,7 +7,6 @@
 
 // UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
 // UNSUPPORTED: no-filesystem
-// UNSUPPORTED: executor-has-no-bash
 // UNSUPPORTED: GCC-ALWAYS_INLINE-FIXME
 
 // XFAIL: availability-fp_to_chars-missing
@@ -27,12 +26,9 @@
 //
 // The testing is based on the testing for std::cout.
 
-// TODO PRINT Use lit builtin echo
-
-// FILE_DEPENDENCIES: echo.sh
 // RUN: %{build}
-// RUN: %{exec} bash echo.sh -n "1234 一二三四 true 0x0" > %t.expected
-// RUN: %{exec} "%t.exe" > %t.actual
+// RUN: echo -n "1234 一二三四 true 0x0" > %t.expected
+// RUN: %{exec} %t.exe > %t.actual
 // RUN: diff -u %t.actual %t.expected
 
 #include <print>
diff --git a/libcxx/test/std/input.output/iostream.format/print.fun/vprint_unicode.sh.cpp b/libcxx/test/std/input.output/iostream.format/print.fun/vprint_unicode.sh.cpp
index 3582293e9e7540a..a9bcc33d2e014ac 100644
--- a/libcxx/test/std/input.output/iostream.format/print.fun/vprint_unicode.sh.cpp
+++ b/libcxx/test/std/input.output/iostream.format/print.fun/vprint_unicode.sh.cpp
@@ -7,7 +7,6 @@
 
 // UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
 // UNSUPPORTED: no-filesystem
-// UNSUPPORTED: executor-has-no-bash
 // UNSUPPORTED: GCC-ALWAYS_INLINE-FIXME
 
 // XFAIL: availability-fp_to_chars-missing
@@ -27,12 +26,9 @@
 //
 // The testing is based on the testing for std::cout.
 
-// TODO PRINT Use lit builtin echo
-
-// FILE_DEPENDENCIES: echo.sh
 // RUN: %{build}
-// RUN: %{exec} bash echo.sh -n "1234 一二三四 true 0x0" > %t.expected
-// RUN: %{exec} "%t.exe" > %t.actual
+// RUN: echo -n "1234 一二三四 true 0x0" > %t.expected
+// RUN: %{exec} %t.exe > %t.actual
 // RUN: diff -u %t.actual %t.expected
 
 #include <print>
diff --git a/libcxx/test/std/input.output/iostream.objects/check-stderr.sh b/libcxx/test/std/input.output/iostream.objects/check-stderr.sh
deleted file mode 100644
index bc800aa9a6bbad7..000000000000000
--- a/libcxx/test/std/input.output/iostream.objects/check-stderr.sh
+++ /dev/null
@@ -1,4 +0,0 @@
-program=${1}
-expected_stderr=${2}
-${program} 2>stderr.log >stdout.log
-[ "${expected_stderr}" == "$(cat stderr.log)" ]
diff --git a/libcxx/test/std/input.output/iostream.objects/check-stdout.sh b/libcxx/test/std/input.output/iostream.objects/check-stdout.sh
deleted file mode 100644
index e86f8cc43a9292e..000000000000000
--- a/libcxx/test/std/input.output/iostream.objects/check-stdout.sh
+++ /dev/null
@@ -1,4 +0,0 @@
-program=${1}
-expected_stdout=${2}
-${program} 2>stderr.log >stdout.log
-[ "${expected_stdout}" == "$(cat stdout.log)" ]
diff --git a/libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/cerr.sh.cpp b/libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/cerr.sh.cpp
index 4df76091e13b5e3..b9e274af7334111 100644
--- a/libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/cerr.sh.cpp
+++ b/libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/cerr.sh.cpp
@@ -10,10 +10,10 @@
 
 // ostream cerr;
 
-// UNSUPPORTED: executor-has-no-bash
-// FILE_DEPENDENCIES: ../check-stderr.sh
 // RUN: %{build}
-// RUN: %{exec} bash check-stderr.sh "%t.exe" "1234"
+// RUN: %{exec} %t.exe 2> %t.actual
+// RUN: echo -n 1234 > %t.expected
+// RUN: diff %t.expected %t.actual
 
 #include <iostream>
 #include <cassert>
diff --git a/libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/cin.sh.cpp b/libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/cin.sh.cpp
index c394c1b00f3e809..533a7a07fa84764 100644
--- a/libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/cin.sh.cpp
+++ b/libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/cin.sh.cpp
@@ -10,10 +10,8 @@
 
 // istream cin;
 
-// UNSUPPORTED: executor-has-no-bash
-// FILE_DEPENDENCIES: ../send-stdin.sh
 // RUN: %{build}
-// RUN: %{exec} bash send-stdin.sh "%t.exe" "1234"
+// RUN: echo -n 1234 | %{exec} %t.exe
 
 #include <iostream>
 #include <cassert>
diff --git a/libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/clog.sh.cpp b/libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/clog.sh.cpp
index 0ac99ba992565e4..476addba050d1c8 100644
--- a/libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/clog.sh.cpp
+++ b/libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/clog.sh.cpp
@@ -10,10 +10,10 @@
 
 // ostream clog;
 
-// UNSUPPORTED: executor-has-no-bash
-// FILE_DEPENDENCIES: ../check-stderr.sh
 // RUN: %{build}
-// RUN: %{exec} bash check-stderr.sh "%t.exe" "1234"
+// RUN: %{exec} %t.exe 2> %t.actual
+// RUN: echo -n 1234 > %t.expected
+// RUN: diff %t.expected %t.actual
 
 #include <iostream>
 
diff --git a/libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/cout.sh.cpp b/libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/cout.sh.cpp
index 43388501d6f961d..b8d319385ca1f71 100644
--- a/libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/cout.sh.cpp
+++ b/libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/cout.sh.cpp
@@ -10,10 +10,10 @@
 
 // ostream cout;
 
-// UNSUPPORTED: executor-has-no-bash
-// FILE_DEPENDENCIES: ../check-stdout.sh
 // RUN: %{build}
-// RUN: %{exec} bash check-stdout.sh "%t.exe" "1234"
+// RUN: %{exec} %t.exe > %t.actual
+// RUN: echo -n 1234 > %t.expected
+// RUN: diff %t.expected %t.actual
 
 #include <iostream>
 
diff --git a/libcxx/test/std/input.output/iostream.objects/send-stdin.sh b/libcxx/test/std/input.output/iostream.objects/send-stdin.sh
deleted file mode 100644
index 2f93f2dfa422645..000000000000000
--- a/libcxx/test/std/input.output/iostream.objects/send-stdin.sh
+++ /dev/null
@@ -1,3 +0,0 @@
-program=${1}
-input=${2}
-echo -n ${input} | ${program}
diff --git a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/check-stderr.sh b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/check-stderr.sh
deleted file mode 100644
index 7edd63be08bae2e..000000000000000
--- a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/check-stderr.sh
+++ /dev/null
@@ -1,5 +0,0 @@
-# Check that the stderr of the executed program matches a reference file.
-program=${1}
-expected_file=${2}
-${program} 2>stderr.log >stdout.log
-cmp stderr.log "${expected_file}"
diff --git a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/check-stdout.sh b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/check-stdout.sh
deleted file mode 100644
index 996cae539e727ab..000000000000000
--- a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/check-stdout.sh
+++ /dev/null
@@ -1,5 +0,0 @@
-# Check that the stdout of the executed program matches a reference file.
-program=${1}
-expected_file=${2}
-${program} 2>stderr.log >stdout.log
-cmp stdout.log "${expected_file}"
diff --git a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/send-stdin.sh b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/send-stdin.sh
deleted file mode 100644
index 70a2a6fafe3297e..000000000000000
--- a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/send-stdin.sh
+++ /dev/null
@@ -1,4 +0,0 @@
-# Pass a reference file as stdin to a test executable.
-program=${1}
-input=${2}
-cat ${input} | ${program}
diff --git a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcerr-imbue.sh.cpp b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcerr-imbue.sh.cpp
index 2b9802b9b304248..ec4c8a009ba0c09 100644
--- a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcerr-imbue.sh.cpp
+++ b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcerr-imbue.sh.cpp
@@ -12,10 +12,10 @@
 
 // UNSUPPORTED: no-wide-characters
 
-// UNSUPPORTED: executor-has-no-bash
-// FILE_DEPENDENCIES: ../check-stderr.sh
 // RUN: %{build}
-// RUN: %{exec} bash check-stderr.sh "%t.exe" "zzzz"
+// RUN: %{exec} %t.exe 2> %t.actual
+// RUN: echo -n zzzz > %t.expected
+// RUN: diff %t.expected %t.actual
 
 #include <iostream>
 
diff --git a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcerr-wide-mode.sh.cpp b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcerr-wide-mode.sh.cpp
index 0abb55fd205920e..f23fbee92afd8e8 100644
--- a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcerr-wide-mode.sh.cpp
+++ b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcerr-wide-mode.sh.cpp
@@ -13,10 +13,10 @@
 // UNSUPPORTED: no-wide-characters
 // REQUIRES: target={{.+}}-windows-{{.+}}
 
-// UNSUPPORTED: executor-has-no-bash
-// FILE_DEPENDENCIES: check-stderr.sh, test.dat
+// FILE_DEPENDENCIES: test.dat
 // RUN: %{build}
-// RUN: %{exec} bash check-stderr.sh "%t.exe" "test.dat"
+// RUN: %{exec} %t.exe 2> %t.actual
+// RUN: diff test.dat %t.actual
 
 // Check that wcerr works, preserving the unicode characters, after switching
 // stderr to wide mode.
diff --git a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcerr.sh.cpp b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcerr.sh.cpp
index d6e77614f94a678..db1917c635c3336 100644
--- a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcerr.sh.cpp
+++ b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcerr.sh.cpp
@@ -12,10 +12,10 @@
 
 // XFAIL: no-wide-characters
 
-// UNSUPPORTED: executor-has-no-bash
-// FILE_DEPENDENCIES: ../check-stderr.sh
 // RUN: %{build}
-// RUN: %{exec} bash check-stderr.sh "%t.exe" "1234"
+// RUN: %{exec} %t.exe 2> %t.actual
+// RUN: echo -n 1234 > %t.expected
+// RUN: diff %t.expected %t.actual
 
 #include <iostream>
 #include <cassert>
diff --git a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcin-imbue.sh.cpp b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcin-imbue.sh.cpp
index 88b00840e64e216..74f0a88eaf8362c 100644
--- a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcin-imbue.sh.cpp
+++ b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcin-imbue.sh.cpp
@@ -12,10 +12,8 @@
 
 // UNSUPPORTED: no-wide-characters
 
-// UNSUPPORTED: executor-has-no-bash
-// FILE_DEPENDENCIES: ../send-stdin.sh
 // RUN: %{build}
-// RUN: %{exec} bash send-stdin.sh "%t.exe" "1234"
+// RUN: echo -n 1234 | %{exec} %t.exe
 
 #include <iostream>
 #include <cassert>
diff --git a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcin-wide-mode.sh.cpp b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcin-wide-mode.sh.cpp
index b7c45d686f835b7..e13d79605f118e0 100644
--- a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcin-wide-mode.sh.cpp
+++ b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcin-wide-mode.sh.cpp
@@ -13,10 +13,9 @@
 // UNSUPPORTED: no-wide-characters
 // REQUIRES: target={{.+}}-windows-{{.+}}
 
-// UNSUPPORTED: executor-has-no-bash
-// FILE_DEPENDENCIES: send-stdin.sh, test.dat
+// FILE_DEPENDENCIES: test.dat
 // RUN: %{build}
-// RUN: %{exec} bash send-stdin.sh "%t.exe" "test.dat"
+// RUN: cat test.dat | %{exec} %t.exe
 
 // Check that wcin works, preserving the unicode characters, after switching
 // stdin to wide mode.
diff --git a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcin.sh.cpp b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcin.sh.cpp
index 88147cfbe2e9eaa..474ea9c9f228236 100644
--- a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcin.sh.cpp
+++ b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcin.sh.cpp
@@ -12,10 +12,8 @@
 
 // XFAIL: no-wide-characters
 
-// UNSUPPORTED: executor-has-no-bash
-// FILE_DEPENDENCIES: ../send-stdin.sh
 // RUN: %{build}
-// RUN: %{exec} bash send-stdin.sh "%t.exe" "1234"
+// RUN: echo -n 1234 | %{exec} %t.exe
 
 #include <iostream>
 #include <cassert>
diff --git a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wclog.sh.cpp b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wclog.sh.cpp
index 03de942d447c476..89e178248ae5015 100644
--- a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wclog.sh.cpp
+++ b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wclog.sh.cpp
@@ -12,10 +12,10 @@
 
 // XFAIL: no-wide-characters
 
-// UNSUPPORTED: executor-has-no-bash
-// FILE_DEPENDENCIES: ../check-stderr.sh
 // RUN: %{build}
-// RUN: %{exec} bash check-stderr.sh "%t.exe" "1234"
+// RUN: %{exec} %t.exe 2> %t.actual
+// RUN: echo -n 1234 > %t.expected
+// RUN: diff %t.expected %t.actual
 
 #include <iostream>
 
diff --git a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcout-imbue.sh.cpp b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcout-imbue.sh.cpp
index 17de670e1bb3a0d..87fc167a20b31f6 100644
--- a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcout-imbue.sh.cpp
+++ b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcout-imbue.sh.cpp
@@ -12,10 +12,10 @@
 
 // UNSUPPORTED: no-wide-characters
 
-// UNSUPPORTED: executor-has-no-bash
-// FILE_DEPENDENCIES: ../check-stdout.sh
 // RUN: %{build}
-// RUN: %{exec} bash check-stdout.sh "%t.exe" "zzzz"
+// RUN: %{exec} %t.exe > %t.actual
+// RUN: echo -n zzzz > %t.expected
+// RUN: diff %t.expected %t.actual
 
 #include <iostream>
 
diff --git a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcout-wide-mode.sh.cpp b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcout-wide-mode.sh.cpp
index 16f2b329ad94873..9908ea0e64db1bb 100644
--- a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcout-wide-mode.sh.cpp
+++ b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcout-wide-mode.sh.cpp
@@ -13,10 +13,10 @@
 // UNSUPPORTED: no-wide-characters
 // REQUIRES: target={{.+}}-windows-{{.+}}
 
-// UNSUPPORTED: executor-has-no-bash
-// FILE_DEPENDENCIES: check-stdout.sh, test.dat
+// FILE_DEPENDENCIES: test.dat
 // RUN: %{build}
-// RUN: %{exec} bash check-stdout.sh "%t.exe" "test.dat"
+// RUN: %{exec} %t.exe > %t.actual
+// RUN: diff test.dat %t.actual
 
 // Check that wcout works, preserving the unicode characters, after switching
 // stdout to wide mode.
diff --git a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcout.sh.cpp b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcout.sh.cpp
index bfc6997224048af..1f7257abb47f1c2 100644
--- a/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcout.sh.cpp
+++ b/libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcout.sh.cpp
@@ -12,10 +12,10 @@
 
 // XFAIL: no-wide-characters
 
-// UNSUPPORTED: executor-has-no-bash
-// FILE_DEPENDENCIES: ../check-stdout.sh
 // RUN: %{build}
-// RUN: %{exec} bash check-stdout.sh "%t.exe" "1234"
+// RUN: %{exec} %t.exe > %t.actual
+// RUN: echo -n 1234 > %t.expected
+// RUN: diff %t.expected %t.actual
 
 #include <iostream>
 

Copy link
Contributor

@philnik777 philnik777 left a comment

Choose a reason for hiding this comment

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

Thanks for the cleanup! LGTM. (BTW this would probably be another great place to use FileCheck)

Instead of relying on Bash, use the builtin Lit commands whenever possible.
The motivation is to stop running %t.exe behind Bash, which breaks on
macOS 13.5 with SIP enabled because DYLD_LIBRARY_PATH isn't forwarded
to the underlying process when running through a protected process.

For more details, see [1].

[1]: https://developer.apple.com/library/archive/documentation/Security/Conceptual/System_Integrity_Protection_Guide/RuntimeProtections/RuntimeProtections.html
@ldionne ldionne force-pushed the review/simplify-stream-tests branch from 4aef3ac to 263b4a7 Compare September 20, 2023 13:31
@ldionne
Copy link
Member Author

ldionne commented Sep 20, 2023

I temporarily added some UNSUPPORTED annotations on AIX. It's a bit complicated, but this is needed to unblock a long chain of things:

  • Update macOS builders to 13.5
  • Update Xcode to 15
  • Drop support for older Clangs

@ldionne ldionne merged commit 257eb74 into llvm:main Sep 20, 2023
@ldionne ldionne deleted the review/simplify-stream-tests branch September 20, 2023 13:33
@vvereschaka
Copy link
Contributor

Hi @ldionne,
3 of all those updated tests are getting failed on the win-to-linux arm/aarch64 cross builders:

@ldionne
Copy link
Member Author

ldionne commented Sep 21, 2023

@vvereschaka Thanks for the heads up. I'm trying to think of a solution right now.

@ldionne
Copy link
Member Author

ldionne commented Sep 21, 2023

First, let's get your bot back to green: 354c99b

ldionne added a commit to ldionne/llvm-project that referenced this pull request Sep 21, 2023
This allows running tests like the ones for std::cin even on SSH executors.
This was originally reported as llvm#66842 (comment).
@ldionne
Copy link
Member Author

ldionne commented Sep 21, 2023

@vvereschaka It took a little while to work my way to it, but #67064 should solve the problem.

@vvereschaka
Copy link
Contributor

Thank you @ldionne ,
I will test #67064 locally on the cross toolchain builder.

@vvereschaka
Copy link
Contributor

@ldionne , done with testing. it looks fine. Thank you.

ldionne added a commit that referenced this pull request Sep 25, 2023
This allows running tests like the ones for std::cin even on SSH
executors. This was originally reported as
#66842 (comment).
blueboxd pushed a commit to blueboxd/libcxx that referenced this pull request Sep 29, 2023
This allows running tests like the ones for std::cin even on SSH
executors. This was originally reported as
llvm/llvm-project#66842 (comment).

NOKEYCHECK=True
GitOrigin-RevId: 21f8bc25adba762ab06e26a7dd50da78fcd17528
@mstorsjo
Copy link
Member

mstorsjo commented Oct 5, 2023

FYI, while the changed tests no longer require having an explicit bash available, the tests do require having an echo executable available (which on Windows often is provided at the same place as you get bash), so build configs that don't have bash available now try to execute them, as UNSUPPORTED: executor-has-no-bash was removed.

In most configs, we do have bash set up to be available when running tests on Windows, but I did run into one case in one setup where the tests previously were skipped but now execute and fail due to the missing echo executable.

(Or to make matters even more subtle - they did find some echo executable, but it didn't support the -n option, so with echo -n 1234, instead of echoing 1234, it echoed -n. The environment where this happened, on GitHub Actions runners, just updated to a different base image a couple hours ago, where that echo no longer is in place any more though, so I haven't been able to figure out what echo executable it actually ended up using.)

@ldionne
Copy link
Member Author

ldionne commented Oct 5, 2023

In most configs, we do have bash set up to be available when running tests on Windows, but I did run into one case in one setup where the tests previously were skipped but now execute and fail due to the missing echo executable.

Interesting. But aren't we using the Lit builtin echo command? I don't think we should ever be trying to call an echo executable on the platform?

@mstorsjo
Copy link
Member

mstorsjo commented Oct 5, 2023

In most configs, we do have bash set up to be available when running tests on Windows, but I did run into one case in one setup where the tests previously were skipped but now execute and fail due to the missing echo executable.

Interesting. But aren't we using the Lit builtin echo command? I don't think we should ever be trying to call an echo executable on the platform?

That was what I thought as well, but I had to dive through a bit of lit to see what it does. It turns out that lit only uses the builtin echo if it's not part of a pipeline - probably due to potential blocking, if I understand the comments correctly: https://github.com/llvm/llvm-project/blob/llvmorg-18-init/llvm/utils/lit/lit/TestRunner.py#L742-L748

@ldionne
Copy link
Member Author

ldionne commented Oct 5, 2023

In most configs, we do have bash set up to be available when running tests on Windows, but I did run into one case in one setup where the tests previously were skipped but now execute and fail due to the missing echo executable.

Interesting. But aren't we using the Lit builtin echo command? I don't think we should ever be trying to call an echo executable on the platform?

That was what I thought as well, but I had to dive through a bit of lit to see what it does. It turns out that lit only uses the builtin echo if it's not part of a pipeline - probably due to potential blocking, if I understand the comments correctly: https://github.com/llvm/llvm-project/blob/llvmorg-18-init/llvm/utils/lit/lit/TestRunner.py#L742-L748

Ahh, I see. It looks like it's also the target of a TODO. Concretely is this blocking any of your work?

@mstorsjo
Copy link
Member

mstorsjo commented Oct 5, 2023

Ahh, I see. It looks like it's also the target of a TODO. Concretely is this blocking any of your work?

Practically, no. In addition to echo, IIRC we have tests that rely on having the executable cp in some places as well anyway, and possibly a few other unixy commands.

The issue had me scratching my head for quite a while, but I realized I could fix the issue by manually adding C:\Program Files\Git\usr\bin to the head of my $PATH. Then when I tried to dig in more on the issue, GitHub rolled out new images to their runners, where that mystery echo executable no longer is in $PATH either, so I'm unable to figure out what really was up (and it's not possible to request a run with the old base image).

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
None yet
Development

Successfully merging this pull request may close these issues.

5 participants