Install six.py conditionally
ClosedPublic

Authored by krytarowski on Feb 1 2017, 10:45 AM.

Details

Summary

The current version of LLDB installs six.py into global python library directory. This approach produces conflicts downstream with distribution's py-six copy.

Introduce new configure option LLDB_USE_SYSTEM_SIX (disabled by default). Once specified as TRUE, six.py won't be installed to Python's directory.

Add new option in finishSwigWrapperClasses.py, namely --useSystemSix.

Sponsored by <The NetBSD Foundation>

Diff Detail

Repository
rL LLVM
krytarowski created this revision.Feb 1 2017, 10:45 AM

Please help to test this patch for regressions, at the moment I don't have all the test suits passing and I might have miss something.

At least scripts/Python/modify-python-lldb.py had to retain 'import six'.

krytarowski edited the summary of this revision. (Show Details)Feb 1 2017, 10:49 AM

It does not seem to work for me. I get this when I run ninja check-lldb in the build directory:

Traceback (most recent call last):
  File "/usr/local/google/home/labath/ll/llvm/tools/lldb/test/dotest.py", line 6, in <module>
    import lldbsuite.test
  File "/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/__init__.py", line 5, in <module>
    from . import dotest
  File "/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/dotest.py", line 36, in <module>
    from lldb import six
ImportError: No module named lldb

Any idea what could be the cause?

I have no issue with this approach in general (assuming it can be made to work), but I want to throw an alternative solution out there:

  • Add a LLDB_USE_SYSTEM_SIX cmake option. If this option is set, LLDB will not install or use the built-in copy of six.py, but will rely on the system one instead.

Thoughts?

It does not seem to work for me. I get this when I run ninja check-lldb in the build directory:

Traceback (most recent call last):
  File "/usr/local/google/home/labath/ll/llvm/tools/lldb/test/dotest.py", line 6, in <module>
    import lldbsuite.test
  File "/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/__init__.py", line 5, in <module>
    from . import dotest
  File "/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/dotest.py", line 36, in <module>
    from lldb import six
ImportError: No module named lldb

Any idea what could be the cause?

I've installed lldb before running tests, I'm not sure if it has the impact on it.

I have no issue with this approach in general (assuming it can be made to work), but I want to throw an alternative solution out there:

  • Add a LLDB_USE_SYSTEM_SIX cmake option. If this option is set, LLDB will not install or use the built-in copy of six.py, but will rely on the system one instead.

    Thoughts?

It's less invasive, I will go for it.

krytarowski updated this revision to Diff 86828.Feb 2 2017, 8:57 AM

LLDB_USE_SYSTEM_SIX approach

krytarowski retitled this revision from Install six.py copy into subdirectory lldb to Install six.py conditionally.Feb 2 2017, 9:00 AM
krytarowski edited the summary of this revision. (Show Details)

@labath patch done.

I'm specifying -DLLDB_USE_SYSTEM_SIX:BOOL=TRUE in pkgsrc.

It works for me.

Unfortunately, this prevents six.py being copied into the build directory, so a non-installed lldb will still not work (if you don't have the system six.py).

It looks like you will have to pass this flag into finishSwigPythonLLDB.py and check the condition there.

Unfortunately, this prevents six.py being copied into the build directory, so a non-installed lldb will still not work (if you don't have the system six.py).

It looks like you will have to pass this flag into finishSwigPythonLLDB.py and check the condition there.

I would do it but I understand how to pass a flag to this script. What and when is calling it? I guess that swig executable?

Is there an option to install this file to a build directory with CMake? This part is clearer.

I will try to reverse engineer it..

labath added a comment.Feb 2 2017, 1:32 PM

Look at top level CmakeLists.txt file
you will find an add_custom_target rule that runs it.

krytarowski edited the summary of this revision. (Show Details)

Pass option to --useSystemSix to finishSwigWrapperClasses.py

labath accepted this revision.Feb 3 2017, 12:46 PM

looks good, thank you.

This revision is now accepted and ready to land.Feb 3 2017, 12:46 PM
krytarowski closed this revision.Feb 3 2017, 4:31 PM