This is an automated email from the ASF dual-hosted git repository.
yangjie01 pushed a commit to branch branch-4.0
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.0 by this push:
new dc20ec4a59d2 [SPARK-50416][CORE] A more portable terminal / pipe test
needed for bin/load-spark-env.sh
dc20ec4a59d2 is described below
commit dc20ec4a59d27f5aa1f88ee8b6e16ad8b3a36fc8
Author: philwalk <[email protected]>
AuthorDate: Thu Mar 20 10:34:14 2025 +0800
[SPARK-50416][CORE] A more portable terminal / pipe test needed for
bin/load-spark-env.sh
### What changes were proposed in this pull request?
The last action in
[bin/load-spark-env.sh](https://github.com/apache/spark/blob/d5da49d56d7dec5f8a96c5252384d865f7efd4d9/bin/load-spark-env.sh#L68)
performs a test to determine whether running in a terminal or not, and whether
`stdin` is reading from a pipe. A more portable test is needed.
### Why are the changes needed?
The current approach relies on `ps` with options that vary significantly
between different Unix-like systems. Specifically, it prints an error message
in both `cygwin` and `msys2` (and by extension, in all of the variations of
`git-for-windows`). It doesn't print an error message, but fails to detect a
terminal session in `Linux` and `Osx/Darwin homebrew` (always thinks STDIN is a
pipe).
Here's what the problem looks like in a `cygwin64` session (with `set -x`
just ahead of the section of interest):
If called directly:
```bash
$ bin/load-spark-env.sh
++ ps -o stat= -p 1947
ps: unknown option -- o
Try `ps --help' for more information.
+ [[ ! '' =~ \+ ]]
+ [[ -p /dev/stdin ]]
+ export 'SPARK_BEELINE_OPTS= -Djline.terminal=jline.UnsupportedTerminal'
+ SPARK_BEELINE_OPTS=' -Djline.terminal=jline.UnsupportedTerminal'
```
Interestingly, due to the 2-part test, it does the right thing w.r.t. the
Terminal test, the main problem being the error message.
If called downstream from a pipe:
```bash
$ echo "yo" | bin/load-spark-env.sh
++ ps -o stat= -p 1955
ps: unknown option -- o
Try `ps --help' for more information.
+ [[ ! '' =~ \+ ]]
+ [[ -p /dev/stdin ]]
```
Again, it correctly detects the pipe environment, but with an error message.
In WSL2 Ubuntu, the test doesn't correctly detect a non-pipe terminal
session:
```bash
# /opt/spark$ bin/load-spark-env.sh
++ ps -o stat= -p 1423
+ [[ ! S+ =~ \+ ]]
# echo "yo!" | bin/load-spark-env.sh
++ ps -o stat= -p 1416
+ [[ ! S+ =~ \+ ]]
```
In `#134-Ubuntu SMP Fri Sep 27 20:20:17 UTC 2024`, the same failure occurs
(it doesn't recognize terminal environments).
### Does this PR introduce _any_ user-facing change?
This is a proposed bug fix, and, other than fixing the bug, should be
invisible to users.
### How was this patch tested?
The patch was verified to behave as intended in terminal sessions, both
interactive and piped, in the following 5 environments.
```
- Linux quadd 5.15.0-124-generic #134-Ubuntu SMP Fri Sep 27 20:20:17 UTC
2024 x86_64 x86_64 x86_64 GNU/Linux
- Linux d5 5.15.153.1-microsoft-standard-WSL2 #1 SMP Fri Mar 29 23:14:13
UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
- MINGW64_NT-10.0-22631 d5 3.5.4-0bc1222b.x86_64 2024-09-04 18:28 UTC
x86_64 Msys
- CYGWIN_NT-10.0-22631 d5 3.5.3-1.x86_64 2024-04-03 17:25 UTC x86_64 Cygwin
- Darwin suemac.local 23.6.0 Darwin Kernel Version 23.6.0: Mon Jul 29
21:14:21 PDT 2024; root:xnu-10063.141.2~1/RELEASE_ARM64_T8103 arm64
```
The test was to manually run the following script, verifying the expected
response to both pipe and terminal sessions.
```bash
#!/bin/bash
if [ -e /usr/bin/tty -a "`tty`" != "not a tty" -a ! -p /dev/stdin ]; then
echo "not a pipe"
else
echo "is a pipe"
fi
```
The output of the manual test in all 5 tested environments.
```
philwalkquadd:/opt/spark
$ isPipe
not a pipe
#
$ echo "yo" | isPipe
is a pipe
#
```
### Was this patch authored or co-authored using generative AI tooling?
No
Closes #48937 from philwalk/portability-fix-for-load-spark-env.sh.
Authored-by: philwalk <[email protected]>
Signed-off-by: yangjie01 <[email protected]>
(cherry picked from commit 8d260084b8a50ff59a127c7292c0cdb6737981b0)
Signed-off-by: yangjie01 <[email protected]>
---
bin/load-spark-env.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bin/load-spark-env.sh b/bin/load-spark-env.sh
index 861ebbbcb7a3..4ab35ad28751 100644
--- a/bin/load-spark-env.sh
+++ b/bin/load-spark-env.sh
@@ -65,6 +65,6 @@ export SPARK_SCALA_VERSION=2.13
#fi
# Append jline option to enable the Beeline process to run in background.
-if [[ ( ! $(ps -o stat= -p $$) =~ "+" ) && ! ( -p /dev/stdin ) ]]; then
+if [ -e /usr/bin/tty -a "`tty`" != "not a tty" -a ! -p /dev/stdin ]; then
export SPARK_BEELINE_OPTS="$SPARK_BEELINE_OPTS
-Djline.terminal=jline.UnsupportedTerminal"
fi
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
