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

ShellDetectionFailure for zsh #77

Closed
pmeier opened this issue Aug 13, 2023 · 10 comments
Closed

ShellDetectionFailure for zsh #77

pmeier opened this issue Aug 13, 2023 · 10 comments

Comments

@pmeier
Copy link

pmeier commented Aug 13, 2023

Not sure what is going on here:

❯ echo $SHELL
/usr/bin/zsh
❯ /usr/bin/zsh --version
zsh 5.9 (x86_64-pc-linux-gnu)
❯ python -c 'import shellingham; shellingham.detect_shell()'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/philip/.conda/envs/ora-dev/lib/python3.11/site-packages/shellingham/__init__.py", line 24, in detect_shell
    raise ShellDetectionFailure()
shellingham._core.ShellDetectionFailure
❯ python -c 'import shellingham; print(shellingham.__version__)'
1.5.1
@uranusjr
Copy link
Member

It would be best if you could do some digging since it’s close to impossible to replicate your exact environment. There is no way I can help with just this information.

@pmeier
Copy link
Author

pmeier commented Aug 14, 2023

It would be best if you could do some digging since it’s close to impossible to replicate your exact environment.

Will do and get back to you.

There is no way I can help with just this information.

I couldn't find any information about what I should provide. Happy to oblige.

@pmeier
Copy link
Author

pmeier commented Aug 14, 2023

It seems the detection fails, because it doesn't find any processes other than the current one:

❯ cat -p git/ora/foo.py
from shellingham import detect_shell

detect_shell()
❯ python git/ora/foo.py
detect_shell(): impl=<module 'shellingham.posix' from '/home/philip/.conda/envs/ora-dev/lib/python3.11/site-packages/shellingham/posix/__init__.py'>
get_shell(): pid='7531', mapping={'7531': Process(args=('python', 'git/ora/foo.py'), pid='7531', ppid='34819')}
get_shell(): proc_args=('python', 'git/ora/foo.py')
Traceback (most recent call last):
  File "/home/philip/git/ora/foo.py", line 3, in <module>
    detect_shell()
  File "/home/philip/.conda/envs/ora-dev/lib/python3.11/site-packages/shellingham/__init__.py", line 25, in detect_shell
    raise ShellDetectionFailure()
shellingham._core.ShellDetectionFailure

@uranusjr
Copy link
Member

Interesting. Can you describe your environment a bit? What operating system are you on, is there emulation involved (32-64 bit binaries, for example), a user that may have permission restrictions, or similar things? Try to dig into doing the process detection part manually as well, does the detection method (proc os ps?) really return only the current process, or is there a problem with parsing?

@pmeier
Copy link
Author

pmeier commented Aug 14, 2023

Can you describe your environment a bit? What operating system are you on, is there emulation involved (32-64 bit binaries, for example)

No emulation. I'm directly on a x86_64 Arch Linux box.

a user that may have permission restrictions

Default user with sudo permissions.

the detection method (proc os ps?)

It is looking at /proc. I'm hitting shellingham.posix.proc.get_process_mapping.

@pmeier
Copy link
Author

pmeier commented Aug 14, 2023

Something is off here. _get_stat returns ppid, tty

return parts[LINUX_STAT_PPID], parts[LINUX_STAT_TTY]

but get_process_mapping unpacks it as

tty, ppid = _get_stat(pid, stat_name)

@pmeier
Copy link
Author

pmeier commented Aug 14, 2023

Yep, that seems to be it. Will send a PR.

@pmeier
Copy link
Author

pmeier commented Aug 14, 2023

I see this is already fixed on master with 13aa99d and ultimately completely refactored with #75 that no longer uses the TTY at all. Can confirm that with 1.5.1 installed, my shell is correctly detected.

@uranusjr I see that 1.5.1 is yanked on PyPI. Any chance we can get a new release soon that includes the patches above?

@uranusjr
Copy link
Member

The version was yanked due to an incompatibility. I attempted to address it and released 1.5.2.

@pmeier
Copy link
Author

pmeier commented Aug 16, 2023

Can confirm that this particular issue is fixed in 1.5.2. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants