apple / swift-argument-parser

Straightforward, type-safe argument parsing for Swift
Apache License 2.0
3.31k stars 311 forks source link

Platform.shellName #583

Closed PopTo closed 9 months ago

PopTo commented 1 year ago

Platform.shellName returns the shell that is currently in use

Checklist

natecook1000 commented 10 months ago

Can you add a note about how you've tested this? When I run /bin/ps, I get a list of all my running processes, some of which are shells and some not. Is this doing extra work to find the currently executing process?

rauhul commented 10 months ago

Im not a huge fan of running another process to find this information, it may not play well with sandboxing and other environmental constraints. Could we use posix or platform specific APIs where needed to find the parent shell?

PopTo commented 10 months ago

@natecook1000 You're right. Maybe echo $0 will be a better solution.

PopTo commented 10 months ago

Yes, @rauhul. Do you have any suggestions for this API? Because I don't know anything that would help.

rauhul commented 10 months ago

Yes, @rauhul. Do you have any suggestions for this API? Because I don't know anything that would help.

On unix-y things you can start with getppid() to get the parent's pid and use a platform specific API to get the process name.

For example libproc on Darwin platforms has a bunch of related API such as proc_pidpath. On linux you might use readlink("/proc/self/exe", ...) but im not super familiar with idiomatic linux system API so someone else might have a better idea.

PopTo commented 10 months ago

@rauhul maybe I found the solution. It's a simple C function:

#include <pwd.h>
#include <unistd.h>

char* shellPath(void) { return getpwuid(geteuid())->pw_shell; }

It's easy to get shell name from this path. It works on Darwin and Linux. probably also on Windows

What do you think?

natecook1000 commented 10 months ago

@PopTo As far as I can tell, that approach retrieves the same data as provided by the SHELL environment variable, which also gets populated from the /etc/passwd data. If you want to go down the getppid() route, we could start with with just Darwin and Linux to simplify testing. It's okay if this doesn't provide 100% coverage, since this API is only used to attempt to respond to a completion script request when no shell has been given by the user.

gwynne commented 10 months ago

The getppid() route is not really viable IMO; it requires potentially elevated privileges and is highly platform-specific. I think SHELL (maybe with a getpwuid()->pw_shell fallback) is as good as can be reasonably asked for - I've yet to see any utility that needs to differentiate between shells in vivo do so by any means other than requiring the user to specify.

natecook1000 commented 9 months ago

@PopTo Thanks again for your work on this, but I think the end solution is getting more heavyweight than justified by the benefit of the result (slightly better shell auto detection). Going to close this PR – let me know if you'd like to discuss further.

PopTo commented 9 months ago

@natecook1000 I agree with you.