Skip to content

Fix a flaky shebang handling issue #30

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

Closed
wants to merge 1 commit into from

Conversation

Kegeruneku
Copy link

@Kegeruneku Kegeruneku commented Oct 26, 2017

Running nginx-ldap-auth daemon as-is yields the following results:

12:12 kegeruneku@arkhangelsk ~/repositories/other/nginx-ldap-auth % ./nginx-ldap-auth-daemon.py                
./nginx-ldap-auth-daemon.py: 2: ./nginx-ldap-auth-daemon.py: cannot create : Directory nonexistent
./nginx-ldap-auth-daemon.py: 3: ./nginx-ldap-auth-daemon.py: cannot create : Directory nonexistent
Traceback (most recent call last):
  File "./nginx-ldap-auth-daemon.py", line 287, in <module>
    server = AuthHTTPServer(Listen, LDAPAuthHandler)
  File "/usr/lib/python2.7/SocketServer.py", line 417, in __init__
    self.server_bind()
  File "/usr/lib/python2.7/BaseHTTPServer.py", line 108, in server_bind
    SocketServer.TCPServer.server_bind(self)
  File "/usr/lib/python2.7/SocketServer.py", line 431, in server_bind
    self.socket.bind(self.server_address)
  File "/usr/lib/python2.7/socket.py", line 228, in meth
    return getattr(self._sock,name)(*args)
socket.error: [Errno 98] Address already in use

I think it is a better idea to use env to select the python version to use, to prevent issues like this.

Moreover, as this project is advertised to support python 2 only, https://www.python.org/dev/peps/pep-0394/ recommends specifying 'python2' as interpreter

@vl-homutov
Copy link
Contributor

Thanks for reporting.

The issue with running script as is was fixed, now it writes a log to stdout.

I prefer to leave shebang as is, it was made on purpose: there is a total mess in distributions,
'python' can point both to 2 or 3rd version, so we first try explicitly 'python2', and if then
simply 'python' in hope it is not v3.

Also we expect python2 or python to be in PATH, and test using 'which', rather than /usr/bin/env,
which is known to happen in different places (/bin vs /usr/bin) as well on some systems.
It is hard to say what is more robust 'which' or 'env' without exhaustive testing of systems, but
practically this works and I wouldn't change until some real evidence of current implementaion is broken.

@vl-homutov vl-homutov closed this Dec 22, 2017
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

Successfully merging this pull request may close these issues.

2 participants