-
Notifications
You must be signed in to change notification settings - Fork 33
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
Fails when run on macOS outside a tty/pty #35
Comments
How can I set up a reproduction to investigate this? |
I've opened a PR (#36) with a Github actions workflow testing this. Which is failing on macOS but passing on Linux. Edit: https://github.com/ids1024/shellingham/actions/runs/280891996 |
Or, or I guess on Linux, import os
import fcntl
import termios
import subprocess
import shellingham
fd = os.open("/dev/tty", os.O_RDWR | os.O_NOCTTY)
fcntl.ioctl(fd, termios.TIOCNOTTY, '')
os.close(fd)
subprocess.call(["ps"])
print(shellingham.detect_shell())
|
Hmm. I wonder why that’d work on GitHub Actions though? Shellingham tests the shell is in the current process’s parent chain, so maybe it’s detecting the login shell in this case? If so, this arguably be attributed to a Linux quirk and unfixable in shellingham. |
Nope, there is legistimately a shell detected, persuambly the one used to run the commands you specify in |
It's a pain to debug with actually having a Mac to try things on locally, but it seemed when I just ran Using something like |
Fixes sarugaku#55 Tries to also address sarugaku#21, sarugaku#35
I noticed this on Github Actions. I haven't reproduced it locally since I don't have a Mac, and it doesn't seem to occur on Linux.
ps
normally lists processes on the same tty. It seems on Linux when run outside a tty, it just lists processes regardless of the connected terminal. But based on what I saw on Github actions, it seems this instead lists no processes.Probably can be fixed with an appropriate argument to
ps
, though some care may be required to avoid breaking anything else and to be compatible with allps
implementations.The text was updated successfully, but these errors were encountered: