[Lldb-commits] [PATCH] D64881: [Cmake] Use the modern way to find Python when possible

Wed Jul 17 14:46:12 PDT 2019

amccarth added a comment.

In D64881#1590252 <https://reviews.llvm.org/D64881#1590252>, @JDevlieghere wrote:

> In D64881#1590204 <https://reviews.llvm.org/D64881#1590204>, @amccarth wrote:
>
> > An aside ...
> >
> > I'm still trying to get back to a buildable state the earlier changes, like the one that tries to enforce version consistency between the libs and the interpreter.  I'm currently bisecting to figure out what I hope is the final blocker.
>
>
> Maybe we should just revert D64443 <https://reviews.llvm.org/D64443>? This is what broke my setup, and led me to introduce to the consistency check in the first place, which merely diagnoses a real problem.

It wasn't a real problem on Windows, where we've been using Python 3.x for LLDB for a long time (predating the patch you propose rolling back).

> 
> 
>> For me, the find_package(PythonInterp) call was always finding the older interpreter (2.7) even though everything pointed to the newer one (3.6).  That tripped the version compatibility check that was recently added.  The algorithm used by find_package isn't well documented (as far as I can tell), so I couldn't even tell you whether it was doing the wrong thing.  I can tell you that the version check seemed unnecessary, as lldb built and tested fine when I locally removed that check, despite the fact that the versions didn't match.
> 
> This is interesting, for me on macOS, it was the other way around. It would always find the 3.7 interpreter because of the LLVM change and never find the corresponding python libs for 3.7 in LLDB. I had to remove every trace of the Python 3 interpreter, for it to only find the 2.7 one. Anyway, I think we all agree (CMake developers included) that the old way of doing things is inherently broken. That's why I want to start adopting the new FindPython.
> 
>> The only way I was able to make things work again was to completely remove Python 2.7 from my machine.  Oddly, the uninstaller also took make with it, so then I couldn't run lldb tests.  With make re-installed, I now have a some crashes during tests to the Python allocator being called without the GIL being held.  I'm currently bisecting to figure out which change caused that.  This crash is especially painful, as it leaves an invisible Python process running and holding files open, which breaks subsequent builds.
>> 
>> To your question ...
>> 
>> Without understanding how either version of find_package finds Python, it's hard to say whether it solves the problems outlined in Zach's comment at the top of find_python_libs.  Zach's first two points don't apply anymore, as we're well past MSVC 2015.  But the third one, regarding a 32-bit CMake being able to find a 64-bit Python, seems like it would still be a problem.
> 
> Thanks, I guess we'd just need to try it then.

Testing the matrix of 32/64, 64/32, Python 2/3, debug/release, seems like a lot of work.  I'd be pretty surprised to learn that CMake did extra, Windows-specific work necessary to make find_package do the right thing in the face of registry and/or filesystem redirection.

Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64881/new/

https://reviews.llvm.org/D64881