This is an archive of the discontinued LLVM Phabricator instance.

Support python in other than /usr/bin/python
ClosedPublic

Authored by emaste on May 21 2015, 8:41 AM.

Details

Summary

Use /usr/local/bin/python if it exists and /usr/bin/python does not. If that also does not exist, try python from $PATH.

This allows check_lint.sh to work on FreeBSD without the need to manually specify PYTHON_EXECUTABLE.

Diff Detail

Repository
rL LLVM

Event Timeline

emaste updated this revision to Diff 26248.May 21 2015, 8:41 AM
emaste retitled this revision from to Support python in other than /usr/bin/python.
emaste updated this object.
emaste edited the test plan for this revision. (Show Details)
emaste added a reviewer: samsonov.
emaste added a subscriber: Unknown Object (MLST).
samsonov added inline comments.May 21 2015, 10:30 AM
lib/sanitizer_common/scripts/check_lint.sh
15 ↗(On Diff #26248)

You never set PYTHON_EXECUTABLE to /usr/bin/python, is it intentional, or you believe it would always be in PATH in this case?

16 ↗(On Diff #26248)

I'd prefer to have this flattened:

if [ -z "${PYTHON_EXECUTABLE}" ] && [ -x /usr/bin/python ]; then
  PYTHON_EXECUTABLE = /usr/bin/python
fi
if [ -z "${PYTHON_EXECUTABLE}" ] && [ -x /usr/bin/local/python ]; then
  ....

then you may also factor this out to function.
Please make sure to not introduce bash-isms.

17 ↗(On Diff #26248)

Maybe, we should just print the value ${PYTHON_EXECUTABLE} if it's non-empty below?

MatzeB added a subscriber: MatzeB.May 21 2015, 11:06 AM

Why not use "/usr/bin/env python" to call the python that is in the users PATH?

I'm happy to just switch to /usr/bin/env python in the scripts if everyone's happy with that (leaving the explicit PYTHON_EXECUTABLE override as it was).

lib/sanitizer_common/scripts/check_lint.sh
15 ↗(On Diff #26248)

If it's in /usr/bin/python the PTYHON_EXECUTABLE can remain empty and #!/usr/bin/python at the beginning of the python scripts will take effect.

That is to say, it seemed like this intentionally used /usr/bin/python if it exists, rather than relying on python in the user's path, and I tried to keep that behaviour. I'm happy with either of

  • #!/usr/bin/env python
  • A find_python shell function that sets PYTHON_EXECUTABLE to the first match from /usr/bin/python, /usr/local/bin/python, $(which python)
samsonov edited edge metadata.May 26 2015, 1:28 PM

Let's try /usr/bin/env python then.

emaste updated this revision to Diff 26622.May 27 2015, 11:41 AM
emaste edited edge metadata.

Just use /usr/bin/env python instead

samsonov accepted this revision.May 27 2015, 2:26 PM
samsonov edited edge metadata.

LGTM

This revision is now accepted and ready to land.May 27 2015, 2:26 PM
This revision was automatically updated to reflect the committed changes.