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

presence of uname not a definite criteria for CYGWIN #1979

Open
alexanderLinear opened this issue Nov 22, 2024 · 1 comment
Open

presence of uname not a definite criteria for CYGWIN #1979

alexanderLinear opened this issue Nov 22, 2024 · 1 comment

Comments

@alexanderLinear
Copy link

alexanderLinear commented Nov 22, 2024

process = subprocess.Popen([uname_cmd], stdout=subprocess.PIPE, universal_newlines=True)

see also this discussion:
oxwhirl/pymarl#113

The subject is, your code wants to look after a maybe or maybe not present program file of 'uname' in the same folder where 'git' is located.

In case there is no such file your code raises a benign exception. But in fact its rather expceted. Thus an is_present() check upfront would avoid such a sort of non-needed but irritating message for 99% of all use cases on regular linux. (fix 1)

If its not present you might find it somewhere else - probably just by going for the default search path... (fix 2)

If the program is finally there, then the program can be queried with option '-s' and the returned string could be compared to its leading chars matching with "CYGWIN", thus setting the result from the innermost block accordingly. (fix 3)

for the expectable results of a uname query see also: https://stackoverflow.com/a/3466183/3423146

backgrounder:
i have Linux with some self-updated versions of git plus a bunch of other tools - and thus using sort of variations of /usr/bin, /usr/local/bin and probably a few more locations. same can be for Mac, Cygwin or wherever. So the strategy to simply probing for one application sitting in the same folder as some other application has a bigger bunch of chances to fail - and so your codes might fail for it in various ways - giving a chance for a false result in either way: indicating cygwin where there is not, and denying cygwin where it really is.

@Byron
Copy link
Member

Byron commented Nov 22, 2024

Thanks a lot for reporting! I also think that finding uname in the PATH should be sufficient, even though I am not sure if that would be another uname that doesn't self-report accordingly?

Maybe the safest way here is to not fail if the program doesn't exist, which would be the smallest possible change to get this to work.
Maybe the exception could be caught?

A PR is definitely welcome.

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

No branches or pull requests

2 participants