You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "philwalk (via GitHub)" <gi...@apache.org> on 2024/11/22 18:50:44 UTC
[PR] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
philwalk opened a new pull request, #48937:
URL: https://github.com/apache/spark/pull/48937
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error type or message, please read the guideline first in
'common/utils/src/main/resources/error/README.md'.
-->
### 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:
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 4 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.
```
philwalk@quadd:/opt/spark
$ isPipe
not a pipe
#
$ echo "yo" | isPipe
is a pipe
#
```
### Was this patch authored or co-authored using generative AI tooling?
No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #48937:
URL: https://github.com/apache/spark/pull/48937#issuecomment-2496429524
Let's also file a JIRA, and add it into the PR description, see also https://spark.apache.org/contributing.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50416][CORE] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "philwalk (via GitHub)" <gi...@apache.org>.
philwalk commented on PR #48937:
URL: https://github.com/apache/spark/pull/48937#issuecomment-3156192622
Here's a version that preserves the original semantics:
```
if [[ ( ! $(ps -o stat= -p $$ 2>/dev/null) =~ "+" ) && ! ( -p /dev/stdin ) ]]; then
echo 'before: hit'
fi
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50416][CORE] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #48937:
URL: https://github.com/apache/spark/pull/48937#issuecomment-2733569642
@dongjoon-hyun @HyukjinKwon @pan3793 Do you need to take another look?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "philwalk (via GitHub)" <gi...@apache.org>.
philwalk commented on PR #48937:
URL: https://github.com/apache/spark/pull/48937#issuecomment-2496239797
I enabled Build and Test on my forked repo.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "philwalk (via GitHub)" <gi...@apache.org>.
philwalk commented on PR #48937:
URL: https://github.com/apache/spark/pull/48937#issuecomment-2498631855
> Let's also file a JIRA, and add it into the PR description, see also https://spark.apache.org/contributing.html
I requested a JIRA account, not yet created ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50416][CORE] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "philwalk (via GitHub)" <gi...@apache.org>.
philwalk commented on PR #48937:
URL: https://github.com/apache/spark/pull/48937#issuecomment-3160910002
The new PR is [51879](https://github.com/apache/spark/pull/51879)
and the JIRA ticket is [SPARK-53149](https://issues.apache.org/jira/browse/SPARK-53149)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50416][CORE] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang closed pull request #48937: [SPARK-50416][CORE] A more portable terminal / pipe test needed for bin/load-spark-env.sh
URL: https://github.com/apache/spark/pull/48937
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50416][CORE] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #48937:
URL: https://github.com/apache/spark/pull/48937#issuecomment-3157198352
I'm not an expert in this area, the GPT suggested version is
```shell
if [[ ! $(ps -o stat= -p $$) == *"+"* ]] && [[ ! -p /dev/stdin ]]; then
...
fi
```
@philwalk, please send a new PR to fix it, and as the current change is already released in 3.5.6 and 4.0.0, you need create a new JIRA ticket ahead
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #48937:
URL: https://github.com/apache/spark/pull/48937#issuecomment-2496912756
The touched code seems to be borrowed from Apache Hive, would you like to also report the issue to the Hive community?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "philwalk (via GitHub)" <gi...@apache.org>.
philwalk commented on PR #48937:
URL: https://github.com/apache/spark/pull/48937#issuecomment-2498687452
[jira bug report](https://issues.apache.org/jira/browse/SPARK-50416)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50416][CORE] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "philwalk (via GitHub)" <gi...@apache.org>.
philwalk commented on PR #48937:
URL: https://github.com/apache/spark/pull/48937#issuecomment-3155804924
@pan3793
> (ps -o stat= -p $$)
You raise a valid point, the two tests aren't equivalent. However, `ps -o stat` isn't portable.
AFAICT, there is no portable way to detect whether running in the background. We could preserve current semantics where `ps -o stat=` is supported, and avoid printing error messages otherwise. But my impression is that what is needed is to know whether we have a terminal to write to or not.
```
philwalk@d5 MSYS /opt/ue
# ./test.sh
ps: unknown option -- o
Try `ps --help' for more information.
before: hit
SPARK-50416: hit
philwalk@d5 MSYS /opt/ue
# ./test.sh &
[4] 119467
philwalk@d5 MSYS /opt/ue
# ps: unknown option -- o
Try `ps --help' for more information.
before: hit
SPARK-50416: hit
```
[4]+ Done ./test.sh
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50416][CORE] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #48937:
URL: https://github.com/apache/spark/pull/48937#discussion_r1998655871
##########
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
Review Comment:
ref https://github.com/koalaman/shellcheck/wiki/SC2166 and https://github.com/koalaman/shellcheck/wiki/SC2006
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "philwalk (via GitHub)" <gi...@apache.org>.
philwalk commented on PR #48937:
URL: https://github.com/apache/spark/pull/48937#issuecomment-2498688840
> The touched code seems to be borrowed from Apache Hive, would you like to also report the issue to the Hive community?
Not at this time ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50416][CORE] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #48937:
URL: https://github.com/apache/spark/pull/48937#issuecomment-3153105696
> 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.
@philwalk The original intention here is to check whether the process is running in the background, you can check SPARK-8731 for more details.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50416][CORE] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "philwalk (via GitHub)" <gi...@apache.org>.
philwalk commented on PR #48937:
URL: https://github.com/apache/spark/pull/48937#issuecomment-2521125904
Is this waiting on me to do something?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50416][CORE] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #48937:
URL: https://github.com/apache/spark/pull/48937#discussion_r1998654398
##########
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
Review Comment:
```suggestion
if [ -e /usr/bin/tty ] && [ "$(tty)" != "not a tty" ] && [ ! -p /dev/stdin ]; then
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50416][CORE] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #48937:
URL: https://github.com/apache/spark/pull/48937#issuecomment-2738953740
Merged into master/branch-4.0/branch-3.5. Thanks @philwalk @dongjoon-hyun @HyukjinKwon @pan3793
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50416][CORE] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "philwalk (via GitHub)" <gi...@apache.org>.
philwalk commented on PR #48937:
URL: https://github.com/apache/spark/pull/48937#issuecomment-3156118926
Thanks for the clarification. I can submit a version that preserves the
semantics for capable systems and avoids the crash for others.
On Mon, Aug 4, 2025, 21:00 Cheng Pan ***@***.***> wrote:
> *pan3793* left a comment (apache/spark#48937)
> <https://github.com/apache/spark/pull/48937#issuecomment-3153105696>
>
> 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.
>
> @philwalk <https://github.com/philwalk> The original intention here is to
> check whether the process is running in the background, you can check
> SPARK-8731 <https://issues.apache.org/jira/browse/SPARK-8731> for more
> details.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/spark/pull/48937#issuecomment-3153105696>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAYGLFSM7WS7JCUVV2YOKF33MAM6RAVCNFSM6AAAAABSJ6TLLGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCNJTGEYDKNRZGY>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "philwalk (via GitHub)" <gi...@apache.org>.
philwalk commented on PR #48937:
URL: https://github.com/apache/spark/pull/48937#issuecomment-2494575464
Not sure what "workflow run detection" is, or how to correct it.
The output of `git remote -vv` is:
```
# git remote -vv
origin https://github.com/philwalk/spark.git (fetch)
origin https://github.com/philwalk/spark.git (push)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50416][CORE] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #48937:
URL: https://github.com/apache/spark/pull/48937#issuecomment-3153091465
@philwalk this causes behavior change.
Write a `test.sh` with content
```
#!/usr/bin/env bash
if [[ ( ! $(ps -o stat= -p $$) =~ "+" ) && ! ( -p /dev/stdin ) ]]; then
echo 'before: hit'
fi
if [ -e /usr/bin/tty -a "`tty`" != "not a tty" -a ! -p /dev/stdin ]; then
echo 'SPARK-50416: hit'
fi
```
```
$ ./test.sh
SPARK-50416: hit
$ ./test.sh &
[1] 1828302
before: hit
SPARK-50416: hit
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50416][CORE] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #48937:
URL: https://github.com/apache/spark/pull/48937#issuecomment-2734317403
> I re-tested the modified line in `MINGW64_NT-10.0-22631 d5 3.5.4-0bc1222b.x86_64 2024-09-04 18:28 UTC x86_64 Msys` and the new version appears to be functionally identical, and the changes appear to be recommended style.
Thank you @philwalk , let's wait one more day. I will merge this one if there are no other new comments,
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50416][CORE] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "philwalk (via GitHub)" <gi...@apache.org>.
philwalk commented on PR #48937:
URL: https://github.com/apache/spark/pull/48937#issuecomment-2734301465
> @dongjoon-hyun @HyukjinKwon @pan3793 Do you need to take another look?
I re-tested the modified line in `MINGW64_NT-10.0-22631 d5 3.5.4-0bc1222b.x86_64 2024-09-04 18:28 UTC x86_64 Msys` and the new version appears to be functionally identical, although the changes are perhaps better style.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50416][CORE] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #48937:
URL: https://github.com/apache/spark/pull/48937#issuecomment-2727089790
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #48937:
URL: https://github.com/apache/spark/pull/48937#issuecomment-2498651252
Got it. I approved it a few minutes ago, @philwalk .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50416][CORE] A more portable terminal / pipe test needed for bin/load-spark-env.sh [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #48937: [SPARK-50416][CORE] A more portable terminal / pipe test needed for bin/load-spark-env.sh
URL: https://github.com/apache/spark/pull/48937
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org