Skip to content

[libc++] Make sure we forward stdin through executors #67064

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
Sep 25, 2023

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Sep 21, 2023

This allows running tests like the ones for std::cin even on SSH executors. This was originally reported as #66842 (comment).

This allows running tests like the ones for std::cin even on SSH executors.
This was originally reported as llvm#66842 (comment).
@ldionne ldionne requested a review from a team as a code owner September 21, 2023 21:06
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Sep 21, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 21, 2023

@llvm/pr-subscribers-libcxx

Changes

This allows running tests like the ones for std::cin even on SSH executors. This was originally reported as #66842 (comment).


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

5 Files Affected:

  • (added) libcxx/test/libcxx/selftest/stdin-is-piped.sh.cpp (+22)
  • (modified) libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/cin.sh.cpp (-4)
  • (modified) libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcin-imbue.sh.cpp (-4)
  • (modified) libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcin.sh.cpp (-4)
  • (modified) libcxx/utils/ssh.py (+6-3)
diff --git a/libcxx/test/libcxx/selftest/stdin-is-piped.sh.cpp b/libcxx/test/libcxx/selftest/stdin-is-piped.sh.cpp
new file mode 100644
index 000000000000000..ffd10631c6a67d4
--- /dev/null
+++ b/libcxx/test/libcxx/selftest/stdin-is-piped.sh.cpp
@@ -0,0 +1,22 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// Make sure that the executor pipes standard input to the test-executable being run.
+
+// RUN: %{build}
+// RUN: echo "abc" | %{exec} %t.exe
+
+#include <cstdio>
+
+int main(int, char**) {
+  int input[] = {std::getchar(), std::getchar(), std::getchar()};
+
+  if (input[0] == 'a' && input[1] == 'b' && input[2] == 'c')
+    return 0;
+  return 1;
+}
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 8fe98f9064d2ee6..b39cd57ab212f1d 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
@@ -9,10 +9,6 @@
 // TODO: Investigate
 // UNSUPPORTED: LIBCXX-AIX-FIXME
 
-// TODO: Make it possible to run this test when cross-compiling and running via a SSH executor
-//       This is a workaround to silence issues reported in https://github.com/llvm/llvm-project/pull/66842#issuecomment-1728701639
-// XFAIL: buildhost=windows && target={{.+}}-linux-{{.+}}
-
 // <iostream>
 
 // istream cin;
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 27c9b0cf7adac58..6bdffc93f3b6693 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
@@ -9,10 +9,6 @@
 // TODO: Investigate
 // UNSUPPORTED: LIBCXX-AIX-FIXME
 
-// TODO: Make it possible to run this test when cross-compiling and running via a SSH executor
-//       This is a workaround to silence issues reported in https://github.com/llvm/llvm-project/pull/66842#issuecomment-1728701639
-// XFAIL: buildhost=windows && target={{.+}}-linux-{{.+}}
-
 // <iostream>
 
 // wistream wcin;
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 3780be1d5cbf226..c0f2c3258b540f6 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
@@ -9,10 +9,6 @@
 // TODO: Investigate
 // UNSUPPORTED: LIBCXX-AIX-FIXME
 
-// TODO: Make it possible to run this test when cross-compiling and running via a SSH executor
-//       This is a workaround to silence issues reported in https://github.com/llvm/llvm-project/pull/66842#issuecomment-1728701639
-// XFAIL: buildhost=windows && target={{.+}}-linux-{{.+}}
-
 // <iostream>
 
 // wistream wcin;
diff --git a/libcxx/utils/ssh.py b/libcxx/utils/ssh.py
index e1eaa5aae067e79..ec16efc3165ab1b 100755
--- a/libcxx/utils/ssh.py
+++ b/libcxx/utils/ssh.py
@@ -62,7 +62,8 @@ def runCommand(command, *args_, **kwargs):
         ssh("mktemp -d {}/libcxx.XXXXXXXXXX".format(args.tempdir)),
         universal_newlines=True,
         check=True,
-        capture_output=True
+        capture_output=True,
+        stdin=subprocess.DEVNULL
     ).stdout.strip()
 
     # HACK:
@@ -80,7 +81,7 @@ def runCommand(command, *args_, **kwargs):
         if args.codesign_identity:
             for exe in filter(isTestExe, commandLine):
                 codesign = ["codesign", "-f", "-s", args.codesign_identity, exe]
-                runCommand(codesign, env={}, check=True)
+                runCommand(codesign, env={}, check=True, stdin=subprocess.DEVNULL)
 
         # tar up the execution directory (which contains everything that's needed
         # to run the test), and copy the tarball over to the remote host.
@@ -93,7 +94,7 @@ def runCommand(command, *args_, **kwargs):
             # the temporary file while still open doesn't work on Windows.
             tmpTar.close()
             remoteTarball = pathOnRemote(tmpTar.name)
-            runCommand(scp(tmpTar.name, remoteTarball), check=True)
+            runCommand(scp(tmpTar.name, remoteTarball), check=True, stdin=subprocess.DEVNULL)
         finally:
             # Make sure we close the file in case an exception happens before
             # we've closed it above -- otherwise close() is idempotent.
@@ -130,6 +131,8 @@ def runCommand(command, *args_, **kwargs):
         remoteCommands.append(subprocess.list2cmdline(commandLine))
 
         # Finally, SSH to the remote host and execute all the commands.
+        # Make sure to forward stdin to the process so that the test suite
+        # can pipe stuff into the executor.
         rc = runCommand(ssh(" && ".join(remoteCommands))).returncode
         return rc
 

@vvereschaka
Copy link
Contributor

@ldionne ,

all of those 3 previously failed tests have passed locally on the win-to-linux aarch64 cross toolchain builder host:

  • llvm-libc++-static.cfg.in :: std/input.output/iostream.objects/narrow.stream.objects/cin.sh.cpp
  • llvm-libc++-static.cfg.in :: std/input.output/iostream.objects/wide.stream.objects/wcin-imbue.sh.cpp
  • llvm-libc++-static.cfg.in :: std/input.output/iostream.objects/wide.stream.objects/wcin.sh.cpp

should work on both of those cross builders. Thank you for the fix.

@ldionne ldionne merged commit 21f8bc2 into llvm:main Sep 25, 2023
@ldionne ldionne deleted the review/fix-stdin-issues-ssh branch September 25, 2023 13:50
Guzhu-AMD pushed a commit to GPUOpen-Drivers/llvm-project that referenced this pull request Sep 28, 2023
Local branch amd-gfx 1328346 Merged main:1f64dc8f59b9 into amd-gfx:83c7a35b9c6b
Remote branch main 21f8bc2 [libc++] Make sure we forward stdin through executors (llvm#67064)
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.

3 participants