Autogenerate the shebang lines for tools/opt-viewer

[cbiesinger]

Summary

Since these files depend on the built python modules, they need to use
the right python binary to run them. So use configure_file
to set the right shebang line.

[chandlerc]

This seems .... somewhat unfortunate. Adding Chris Bieneman to see if this is really the right thing to do or there are any other alternatives.

[beanz]

Sadly I'm not sure that there is a better way than generating the shebang line, but I can imagine some problems with the way this patch does it. PYTHON_EXECUTABLE could be a full path, which means that this can't be used to generate binary distributions because you'd be relying on python being in the same place on disk, which defeats the purpose of env being on the shebang.

[JDevlieghere]

Chris is correct, PYTHON_EXECUTABLE will be an absolute path. For this to work you'll need to remove /usr/bin/env.

[cbiesinger]

Use python@PYTHON_VERSION_MAJOR@

[cbiesinger]

It actually worked with the env for me.

Anyway, beanz, does this address your concern? It uses #!/usr/bin/env python@PYTHON_VERSION_MAJOR@

[beanz]

In D65983#1633766, @cbiesinger wrote:

It actually worked with the env for me.

It will always work locally, it just may not work in a binary distribution (i.e. take your built binaries and move them somewhere else with a different path to python).

Anyway, beanz, does this address your concern? It uses #!/usr/bin/env python@PYTHON_VERSION_MAJOR@

Is there any situation where the python tool isn't named that way? What happens if PYTHON_EXECUTABLE points to a Python 3.2 install, but my system default is Python 3.5? Doesn't that make the python3 link point to python35?

What about using CMake's get_filename_component (https://cmake.org/cmake/help/v3.0/command/get_filename_component.html) function to find the name of the executable? That seems more robust.

[cbiesinger]

Use python's basename

[cbiesinger]

I'm not sure that such a python3 vs 3.5 difference matters, but anyway, here's a version that does arguably the right thing.

[compnerd]

At least for the from-source distribution, where the destination of the image is the build host, Python_EXECUTABLE is definitely preferable (e.g. #!@Python_EXECUTABLE@). Furthermore, there is no reason to assume that /usr/bin/env python3 will give you the same python 3 as what was given to cmake (you can control that with -DPython_EXECUTABLE=...). I think that requiring the user to specify the tool is possibly the best option - that is completely remove the shebang as that ensures that shifts the burden of selection to the user. Another option may be to use something like p2exe and create self-contained tools.

[compnerd]

Python eggs may be another option which might be an easier way forward.

[mgorny]

This looks like a correct approach to me.

[cbiesinger]

Ping? I think this version is a good compromise that will work well both for binary distributions and compiling from source...

[beanz]

Seems reasonable to me. @anemet any concerns?

[anemet]

No, concerns. Looks good to me too.

[cbiesinger]

Thank you! I am not a committer, could one of you land this?

[beanz]

I'm happy to commit this for you. Will do so shortly.

[beanz]

Landed in rL369486

[rnk]

How does this work at all? In llvm's lit.cfg.py I see this line:

opt_viewer_cmd = '%s %s/tools/opt-viewer/opt-viewer.py' % (sys.executable, config.llvm_src_root)

After this change, the referenced file does not exist.

I can probably update the path to refer into the object tree, but I'm wondering if there's a gap in testing somewhere.

If anyone is doing meaningful opt-viewer development, they might also want to consider separating most of the code out of the configured file so that they don't have to re-run cmake in order to test their changes.

[rnk]

I ended up reverting this in rL370095 since fixing the tests didn't seem easy.

llvm/CMakeLists.txt

 
 Please install Python or specify the PYTHON_EXECUTABLE CMake variable.")
 endif()
 
 if( ${PYTHON_VERSION_STRING} VERSION_LESS 2.7 )
   message(FATAL_ERROR "Python 2.7 or newer is required")
 endif()
 
+get_filename_component(PYTHON_BASENAME ${PYTHON_EXECUTABLE} NAME)
+
 ######
 # LLVMBuild Integration
 #
 # We use llvm-build to generate all the data required by the CMake based
 # build system in one swoop:
 #
 #  - We generate a file (a CMake fragment) in the object root which contains
 #    all the definitions that are required by CMake.

llvm/tools/opt-viewer/CMakeLists.txt

 set (files
+  "optpmap.py"
+  "style.css")
+
+set (generated_files
   "opt-diff.py"
   "opt-stats.py"
   "opt-viewer.py"
-  "optpmap.py"
-  "optrecord.py"
-  "style.css")
+  "optrecord.py")
+
+foreach (file ${generated_files})
+  configure_file(
+    ${CMAKE_CURRENT_SOURCE_DIR}/${file}.in
+    ${CMAKE_CURRENT_BINARY_DIR}/${file})
+endforeach (file)
 
 foreach (file ${files})
   install(PROGRAMS ${file}
     DESTINATION share/opt-viewer
     COMPONENT opt-viewer)
 endforeach (file)
+
+
+foreach (file ${generated_files})
+  install(PROGRAMS ${CMAKE_CURRENT_BINARY_DIR}/${file}
+    DESTINATION share/opt-viewer
+    COMPONENT opt-viewer)
+endforeach (file)

llvm/tools/opt-viewer/opt-diff.py.in

-#!/usr/bin/env python
+#!/usr/bin/env @PYTHON_BASENAME@
 
 from __future__ import print_function
 
 desc = '''Generate the difference of two YAML files into a new YAML file (works on
 pair of directories too).  A new attribute 'Added' is set to True or False
 depending whether the entry is added or removed from the first input to the
 next.
 

llvm/tools/opt-viewer/opt-stats.py.in

-#!/usr/bin/env python
+#!/usr/bin/env @PYTHON_BASENAME@
 
 from __future__ import print_function
 
 desc = '''Generate statistics about optimization records from the YAML files
 generated with -fsave-optimization-record and -fdiagnostics-show-hotness.
 
 The tools requires PyYAML and Pygments Python packages.'''
 

llvm/tools/opt-viewer/opt-viewer.py.in

-#!/usr/bin/env python
+#!/usr/bin/env @PYTHON_BASENAME@
 
 from __future__ import print_function
 
 import argparse
 import cgi
 import codecs
 import errno
 import functools

llvm/tools/opt-viewer/optrecord.py.in

-#!/usr/bin/env python
+#!/usr/bin/env @PYTHON_BASENAME@
 
 from __future__ import print_function
 
 import yaml
 # Try to use the C parser.
 try:
     from yaml import CLoader as Loader
 except ImportError: