[Lldb-commits] [PATCH] D65230: [CMake] Loosen Python version check and ignore patch version

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth added a comment.

Just closing the loop.  Regarding the "Python memory allocator called without 
holding the GIL" bugs:  this is not a "just me" problem.  It's for anyone who 
enables debug mode.  This appears to be a test infrastructure problem, but it 
might be harmless.  If I run in "release" mode, I get the same behavior as the 
Windows build bot.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D65230: [CMake] Loosen Python version check and ignore patch version

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth added a comment.

In D65230#1602370 , @labath wrote:

> Thanks.
>
> BTW, I tried this on windows, and it didn't blow up in my face, I think it 
> should be relatively safe. I couldn't reproduce the problems that @amccarth 
> was experiencing, so I don't know if it fixes that problem. It would be good 
> to know whether it does, as we could cherry-pick this to the release branch 
> in that case.


Sorry for the slow reply.  I've been in a conference most of the week.

Since my problem tends to be a "just me" problem that doesn't affect the bots 
or apparently anyone else, I don't think a cherrypick is necessary.

By having only one version of Python on my machine, I was able to work around 
that problem.  Unfortunately, I've been stuck on another problem since making 
that change (specifically, all dotest.py tests report "Python memory allocator 
called without holding the GIL" and crash).  This, too, seems to be a "just me" 
thing, and I'm continuing to work on that this morning.  Until I get that 
figured out, I don't think I want to perturb my environment further by 
re-installing a second version of Python.  I suspect the other patch to 
eliminate multiprocessing from dotest.py might solve the GIL problem as well, 
but I want to know more first.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D65230: [CMake] Loosen Python version check and ignore patch version

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367115: [CMake] Loosen Python version check and ignore patch 
version (authored by JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65230?vs=211811=211939#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65230

Files:
  lldb/trunk/cmake/modules/LLDBConfig.cmake


Index: lldb/trunk/cmake/modules/LLDBConfig.cmake
===
--- lldb/trunk/cmake/modules/LLDBConfig.cmake
+++ lldb/trunk/cmake/modules/LLDBConfig.cmake
@@ -140,7 +140,6 @@
  PYTHONLIBS_VERSION_STRING "${python_version_str}")
 message(STATUS "Found Python library version ${PYTHONLIBS_VERSION_STRING}")
 string(REGEX REPLACE "([0-9]+)[.]([0-9]+)[.][0-9]+" "python\\1\\2" 
PYTHONLIBS_BASE_NAME "${PYTHONLIBS_VERSION_STRING}")
-set(PYTHONLIBS_VERSION_STRING "${PYTHONLIBS_VERSION_STRING}" PARENT_SCOPE)
 unset(python_version_str)
   else()
 message(WARNING "Unable to find ${PYTHON_INCLUDE_DIR}/patchlevel.h, Python 
installation is corrupt.")
@@ -175,13 +174,31 @@
 return()
   endif()
 
-  set (PYTHON_EXECUTABLE ${PYTHON_EXE} PARENT_SCOPE)
-  set (PYTHON_LIBRARY ${PYTHON_LIB} PARENT_SCOPE)
-  set (PYTHON_DLL ${PYTHON_DLL} PARENT_SCOPE)
-  set (PYTHON_INCLUDE_DIR ${PYTHON_INCLUDE_DIR} PARENT_SCOPE)
+  # Find the version of the Python interpreter.
+  execute_process(COMMAND "${PYTHON_EXE}" -c
+  "import sys; sys.stdout.write(';'.join([str(x) for x in 
sys.version_info[:3]]))"
+  OUTPUT_VARIABLE PYTHON_VERSION_OUTPUT
+  RESULT_VARIABLE PYTHON_VERSION_RESULT
+  ERROR_QUIET)
+  if(NOT PYTHON_VERSION_RESULT)
+string(REPLACE ";" "." PYTHON_VERSION_STRING "${PYTHON_VERSION_OUTPUT}")
+list(GET PYTHON_VERSION_OUTPUT 0 PYTHON_VERSION_MAJOR)
+list(GET PYTHON_VERSION_OUTPUT 1 PYTHON_VERSION_MINOR)
+list(GET PYTHON_VERSION_OUTPUT 2 PYTHON_VERSION_PATCH)
+  endif()
+
+  # Set the same variables as FindPythonInterp and FindPythonLibs.
+  set(PYTHON_EXECUTABLE ${PYTHON_EXE} PARENT_SCOPE)
+  set(PYTHON_LIBRARY ${PYTHON_LIB} PARENT_SCOPE)
+  set(PYTHON_DLL ${PYTHON_DLL} PARENT_SCOPE)
+  set(PYTHONLIBS_VERSION_STRING "${PYTHONLIBS_VERSION_STRING}" PARENT_SCOPE)
+  set(PYTHON_VERSION_STRING ${PYTHON_VERSION_STRING} PARENT_SCOPE)
+  set(PYTHON_VERSION_MAJOR ${PYTHON_VERSION_MAJOR} PARENT_SCOPE)
+  set(PYTHON_VERSION_MINOR ${PYTHON_VERSION_MINOR} PARENT_SCOPE)
+  set(PYTHON_VERSION_PATCH ${PYTHON_VERSION_PATCH} PARENT_SCOPE)
 
-  message(STATUS "LLDB Found PythonExecutable: ${PYTHON_EXE}")
-  message(STATUS "LLDB Found PythonLibs: ${PYTHON_LIB}")
+  message(STATUS "LLDB Found PythonExecutable: ${PYTHON_EXECUTABLE}")
+  message(STATUS "LLDB Found PythonLibs: ${PYTHON_LIBRARY}")
   message(STATUS "LLDB Found PythonDLL: ${PYTHON_DLL}")
   message(STATUS "LLDB Found PythonIncludeDirs: ${PYTHON_INCLUDE_DIR}")
 endfunction(find_python_libs_windows)
@@ -199,9 +216,18 @@
 find_package(PythonLibs REQUIRED)
   endif()
 
-  if (NOT PYTHON_VERSION_STRING VERSION_EQUAL PYTHONLIBS_VERSION_STRING AND
-  NOT CMAKE_CROSSCOMPILING)
-message(FATAL_ERROR "Found incompatible Python interpreter 
(${PYTHON_VERSION_STRING}) and Python libraries (${PYTHONLIBS_VERSION_STRING})")
+  if (NOT CMAKE_CROSSCOMPILING)
+string(REPLACE "." ";" pythonlibs_version_list 
${PYTHONLIBS_VERSION_STRING})
+list(GET pythonlibs_version_list 0 pythonlibs_major)
+list(GET pythonlibs_version_list 1 pythonlibs_minor)
+
+# Ignore the patch version. Some versions of macOS report a different patch
+# version for the system provided interpreter and libraries.
+if (NOT PYTHON_VERSION_MAJOR VERSION_EQUAL pythonlibs_major OR
+NOT PYTHON_VERSION_MINOR VERSION_EQUAL pythonlibs_minor)
+  message(FATAL_ERROR "Found incompatible Python interpreter 
(${python_major}.${python_minor})"
+  " and Python libraries 
(${pythonlibs_major}.${pythonlibs_minor})")
+endif()
   endif()
 
   if (PYTHON_INCLUDE_DIR)


Index: lldb/trunk/cmake/modules/LLDBConfig.cmake
===
--- lldb/trunk/cmake/modules/LLDBConfig.cmake
+++ lldb/trunk/cmake/modules/LLDBConfig.cmake
@@ -140,7 +140,6 @@
  PYTHONLIBS_VERSION_STRING "${python_version_str}")
 message(STATUS "Found Python library version ${PYTHONLIBS_VERSION_STRING}")
 string(REGEX REPLACE "([0-9]+)[.]([0-9]+)[.][0-9]+" "python\\1\\2" PYTHONLIBS_BASE_NAME "${PYTHONLIBS_VERSION_STRING}")
-set(PYTHONLIBS_VERSION_STRING "${PYTHONLIBS_VERSION_STRING}" PARENT_SCOPE)
 unset(python_version_str)
   else()
 message(WARNING "Unable to find ${PYTHON_INCLUDE_DIR}/patchlevel.h, Python installation is corrupt.")
@@ -175,13 +174,31 @@

[Lldb-commits] [PATCH] D65230: [CMake] Loosen Python version check and ignore patch version

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks.

BTW, I tried this on windows, and it didn't blow up in my face, I think it 
should be relatively safe. I couldn't reproduce the problems that @amccarth was 
experiencing, so I don't know if it fixes that problem. It would be good to 
know whether it does, as we could cherry-pick this to the release branch in 
that case.


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

https://reviews.llvm.org/D65230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D65230: [CMake] Loosen Python version check and ignore patch version

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz accepted this revision.
sgraenitz added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D65230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D65230: [CMake] Loosen Python version check and ignore patch version

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

@amccarth I think this might explain the issues you were having last week. 
Without this patch, `find_python_libs_windows` is not setting the expected 
version variables. As a result, my check was complaining about a mismatch 
between the Python interpreter that LLVM found, and the libs that LLDB found, 
while LLDB was actually using a different Python interpreter, found by 
`find_python_libs_windows`.




Comment at: lldb/cmake/modules/LLDBConfig.cmake:203-204
+  if (NOT CMAKE_CROSSCOMPILING)
+# Manually split PYTHON_VERSION_STRING because find_python_libs_windows
+# doesn't set PYTHON_VERSION_MAJOR or PYTHON_VERSION_MINOR.
+string(REPLACE "." ";" python_version_list ${PYTHON_VERSION_STRING})

labath wrote:
> JDevlieghere wrote:
> > labath wrote:
> > > If this is the only reason, then maybe we could fix 
> > > find_python_libs_windows to set the relevant variables instead (?)
> > FindPythonLibs only sets `PYTHONLIBS_VERSION_STRING`, so we'd still have 
> > that to parse the major and minor version of that manually. It was a coin 
> > flip between keeping the parsing together and updating 
> > `find_python_libs_windows`. I'm happy to do either. 
> Yea, I see what you mean. I think I'd go with changing 
> `find_python_libs_windows` just to minimize the difference between the 
> windows and non-windows branches of the python-finding code (and possible 
> side-effects of those differences), but it's not a big deal either way...
Sure, sounds good!


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

https://reviews.llvm.org/D65230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D65230: [CMake] Loosen Python version check and ignore patch version

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 211811.
JDevlieghere marked an inline comment as done.
JDevlieghere added a comment.

Export the same variables in `find_python_libs_windows` as `FindPythonInterp` 
and `FindPythonLibs`.


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

https://reviews.llvm.org/D65230

Files:
  lldb/cmake/modules/LLDBConfig.cmake


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -140,7 +140,6 @@
  PYTHONLIBS_VERSION_STRING "${python_version_str}")
 message(STATUS "Found Python library version ${PYTHONLIBS_VERSION_STRING}")
 string(REGEX REPLACE "([0-9]+)[.]([0-9]+)[.][0-9]+" "python\\1\\2" 
PYTHONLIBS_BASE_NAME "${PYTHONLIBS_VERSION_STRING}")
-set(PYTHONLIBS_VERSION_STRING "${PYTHONLIBS_VERSION_STRING}" PARENT_SCOPE)
 unset(python_version_str)
   else()
 message(WARNING "Unable to find ${PYTHON_INCLUDE_DIR}/patchlevel.h, Python 
installation is corrupt.")
@@ -175,13 +174,31 @@
 return()
   endif()
 
-  set (PYTHON_EXECUTABLE ${PYTHON_EXE} PARENT_SCOPE)
-  set (PYTHON_LIBRARY ${PYTHON_LIB} PARENT_SCOPE)
-  set (PYTHON_DLL ${PYTHON_DLL} PARENT_SCOPE)
-  set (PYTHON_INCLUDE_DIR ${PYTHON_INCLUDE_DIR} PARENT_SCOPE)
+  # Find the version of the Python interpreter.
+  execute_process(COMMAND "${PYTHON_EXE}" -c
+  "import sys; sys.stdout.write(';'.join([str(x) for x in 
sys.version_info[:3]]))"
+  OUTPUT_VARIABLE PYTHON_VERSION_OUTPUT
+  RESULT_VARIABLE PYTHON_VERSION_RESULT
+  ERROR_QUIET)
+  if(NOT PYTHON_VERSION_RESULT)
+string(REPLACE ";" "." PYTHON_VERSION_STRING "${PYTHON_VERSION_OUTPUT}")
+list(GET PYTHON_VERSION_OUTPUT 0 PYTHON_VERSION_MAJOR)
+list(GET PYTHON_VERSION_OUTPUT 1 PYTHON_VERSION_MINOR)
+list(GET PYTHON_VERSION_OUTPUT 2 PYTHON_VERSION_PATCH)
+  endif()
 
-  message(STATUS "LLDB Found PythonExecutable: ${PYTHON_EXE}")
-  message(STATUS "LLDB Found PythonLibs: ${PYTHON_LIB}")
+  # Set the same variables as FindPythonInterp and FindPythonLibs.
+  set(PYTHON_EXECUTABLE ${PYTHON_EXE} PARENT_SCOPE)
+  set(PYTHON_LIBRARY ${PYTHON_LIB} PARENT_SCOPE)
+  set(PYTHON_DLL ${PYTHON_DLL} PARENT_SCOPE)
+  set(PYTHONLIBS_VERSION_STRING "${PYTHONLIBS_VERSION_STRING}" PARENT_SCOPE)
+  set(PYTHON_VERSION_STRING ${PYTHON_VERSION_STRING} PARENT_SCOPE)
+  set(PYTHON_VERSION_MAJOR ${PYTHON_VERSION_MAJOR} PARENT_SCOPE)
+  set(PYTHON_VERSION_MINOR ${PYTHON_VERSION_MINOR} PARENT_SCOPE)
+  set(PYTHON_VERSION_PATCH ${PYTHON_VERSION_PATCH} PARENT_SCOPE)
+
+  message(STATUS "LLDB Found PythonExecutable: ${PYTHON_EXECUTABLE}")
+  message(STATUS "LLDB Found PythonLibs: ${PYTHON_LIBRARY}")
   message(STATUS "LLDB Found PythonDLL: ${PYTHON_DLL}")
   message(STATUS "LLDB Found PythonIncludeDirs: ${PYTHON_INCLUDE_DIR}")
 endfunction(find_python_libs_windows)
@@ -199,9 +216,18 @@
 find_package(PythonLibs REQUIRED)
   endif()
 
-  if (NOT PYTHON_VERSION_STRING VERSION_EQUAL PYTHONLIBS_VERSION_STRING AND
-  NOT CMAKE_CROSSCOMPILING)
-message(FATAL_ERROR "Found incompatible Python interpreter 
(${PYTHON_VERSION_STRING}) and Python libraries (${PYTHONLIBS_VERSION_STRING})")
+  if (NOT CMAKE_CROSSCOMPILING)
+string(REPLACE "." ";" pythonlibs_version_list 
${PYTHONLIBS_VERSION_STRING})
+list(GET pythonlibs_version_list 0 pythonlibs_major)
+list(GET pythonlibs_version_list 1 pythonlibs_minor)
+
+# Ignore the patch version. Some versions of macOS report a different patch
+# version for the system provided interpreter and libraries.
+if (NOT PYTHON_VERSION_MAJOR VERSION_EQUAL pythonlibs_major OR
+NOT PYTHON_VERSION_MINOR VERSION_EQUAL pythonlibs_minor)
+  message(FATAL_ERROR "Found incompatible Python interpreter 
(${python_major}.${python_minor})"
+  " and Python libraries 
(${pythonlibs_major}.${pythonlibs_minor})")
+endif()
   endif()
 
   if (PYTHON_INCLUDE_DIR)


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -140,7 +140,6 @@
  PYTHONLIBS_VERSION_STRING "${python_version_str}")
 message(STATUS "Found Python library version ${PYTHONLIBS_VERSION_STRING}")
 string(REGEX REPLACE "([0-9]+)[.]([0-9]+)[.][0-9]+" "python\\1\\2" PYTHONLIBS_BASE_NAME "${PYTHONLIBS_VERSION_STRING}")
-set(PYTHONLIBS_VERSION_STRING "${PYTHONLIBS_VERSION_STRING}" PARENT_SCOPE)
 unset(python_version_str)
   else()
 message(WARNING "Unable to find ${PYTHON_INCLUDE_DIR}/patchlevel.h, Python installation is corrupt.")
@@ -175,13 +174,31 @@
 return()
   endif()
 
-  set (PYTHON_EXECUTABLE ${PYTHON_EXE} PARENT_SCOPE)
-  set (PYTHON_LIBRARY ${PYTHON_LIB} PARENT_SCOPE)
-  set (PYTHON_DLL ${PYTHON_DLL} PARENT_SCOPE)
-  set 

[Lldb-commits] [PATCH] D65230: [CMake] Loosen Python version check and ignore patch version

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/cmake/modules/LLDBConfig.cmake:203-204
+  if (NOT CMAKE_CROSSCOMPILING)
+# Manually split PYTHON_VERSION_STRING because find_python_libs_windows
+# doesn't set PYTHON_VERSION_MAJOR or PYTHON_VERSION_MINOR.
+string(REPLACE "." ";" python_version_list ${PYTHON_VERSION_STRING})

JDevlieghere wrote:
> labath wrote:
> > If this is the only reason, then maybe we could fix 
> > find_python_libs_windows to set the relevant variables instead (?)
> FindPythonLibs only sets `PYTHONLIBS_VERSION_STRING`, so we'd still have that 
> to parse the major and minor version of that manually. It was a coin flip 
> between keeping the parsing together and updating `find_python_libs_windows`. 
> I'm happy to do either. 
Yea, I see what you mean. I think I'd go with changing 
`find_python_libs_windows` just to minimize the difference between the windows 
and non-windows branches of the python-finding code (and possible side-effects 
of those differences), but it's not a big deal either way...


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

https://reviews.llvm.org/D65230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D65230: [CMake] Loosen Python version check and ignore patch version

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.



Comment at: lldb/cmake/modules/LLDBConfig.cmake:203-204
+  if (NOT CMAKE_CROSSCOMPILING)
+# Manually split PYTHON_VERSION_STRING because find_python_libs_windows
+# doesn't set PYTHON_VERSION_MAJOR or PYTHON_VERSION_MINOR.
+string(REPLACE "." ";" python_version_list ${PYTHON_VERSION_STRING})

labath wrote:
> If this is the only reason, then maybe we could fix find_python_libs_windows 
> to set the relevant variables instead (?)
FindPythonLibs only sets `PYTHONLIBS_VERSION_STRING`, so we'd still have that 
to parse the major and minor version of that manually. It was a coin flip 
between keeping the parsing together and updating `find_python_libs_windows`. 
I'm happy to do either. 


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

https://reviews.llvm.org/D65230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D65230: [CMake] Loosen Python version check and ignore patch version

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/cmake/modules/LLDBConfig.cmake:203-204
+  if (NOT CMAKE_CROSSCOMPILING)
+# Manually split PYTHON_VERSION_STRING because find_python_libs_windows
+# doesn't set PYTHON_VERSION_MAJOR or PYTHON_VERSION_MINOR.
+string(REPLACE "." ";" python_version_list ${PYTHON_VERSION_STRING})

If this is the only reason, then maybe we could fix find_python_libs_windows to 
set the relevant variables instead (?)


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

https://reviews.llvm.org/D65230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D65230: [CMake] Loosen Python version check and ignore patch version

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 211606.
JDevlieghere added a comment.

Use lowercase variables for locals.


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

https://reviews.llvm.org/D65230

Files:
  lldb/cmake/modules/LLDBConfig.cmake


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -199,9 +199,23 @@
 find_package(PythonLibs REQUIRED)
   endif()
 
-  if (NOT PYTHON_VERSION_STRING VERSION_EQUAL PYTHONLIBS_VERSION_STRING AND
-  NOT CMAKE_CROSSCOMPILING)
-message(FATAL_ERROR "Found incompatible Python interpreter 
(${PYTHON_VERSION_STRING}) and Python libraries (${PYTHONLIBS_VERSION_STRING})")
+  if (NOT CMAKE_CROSSCOMPILING)
+# Manually split PYTHON_VERSION_STRING because find_python_libs_windows
+# doesn't set PYTHON_VERSION_MAJOR or PYTHON_VERSION_MINOR.
+string(REPLACE "." ";" python_version_list ${PYTHON_VERSION_STRING})
+string(REPLACE "." ";" pythonlibs_version_list 
${PYTHONLIBS_VERSION_STRING})
+list(GET python_version_list 0 python_major)
+list(GET python_version_list 1 python_minor)
+list(GET pythonlibs_version_list 0 pythonlibs_major)
+list(GET pythonlibs_version_list 1 pythonlibs_minor)
+
+# Ignore the patch version. Some versions of macOS report a different patch
+# version for the system provided interpreter and libraries.
+if (NOT python_major VERSION_EQUAL pythonlibs_major OR
+NOT python_minor VERSION_EQUAL pythonlibs_minor)
+  message(FATAL_ERROR "Found incompatible Python interpreter 
(${python_major}.${python_minor})"
+  " and Python libraries 
(${pythonlibs_major}.${pythonlibs_minor})")
+endif()
   endif()
 
   if (PYTHON_INCLUDE_DIR)


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -199,9 +199,23 @@
 find_package(PythonLibs REQUIRED)
   endif()
 
-  if (NOT PYTHON_VERSION_STRING VERSION_EQUAL PYTHONLIBS_VERSION_STRING AND
-  NOT CMAKE_CROSSCOMPILING)
-message(FATAL_ERROR "Found incompatible Python interpreter (${PYTHON_VERSION_STRING}) and Python libraries (${PYTHONLIBS_VERSION_STRING})")
+  if (NOT CMAKE_CROSSCOMPILING)
+# Manually split PYTHON_VERSION_STRING because find_python_libs_windows
+# doesn't set PYTHON_VERSION_MAJOR or PYTHON_VERSION_MINOR.
+string(REPLACE "." ";" python_version_list ${PYTHON_VERSION_STRING})
+string(REPLACE "." ";" pythonlibs_version_list ${PYTHONLIBS_VERSION_STRING})
+list(GET python_version_list 0 python_major)
+list(GET python_version_list 1 python_minor)
+list(GET pythonlibs_version_list 0 pythonlibs_major)
+list(GET pythonlibs_version_list 1 pythonlibs_minor)
+
+# Ignore the patch version. Some versions of macOS report a different patch
+# version for the system provided interpreter and libraries.
+if (NOT python_major VERSION_EQUAL pythonlibs_major OR
+NOT python_minor VERSION_EQUAL pythonlibs_minor)
+  message(FATAL_ERROR "Found incompatible Python interpreter (${python_major}.${python_minor})"
+  " and Python libraries (${pythonlibs_major}.${pythonlibs_minor})")
+endif()
   endif()
 
   if (PYTHON_INCLUDE_DIR)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D65230: [CMake] Loosen Python version check and ignore patch version

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

Interesting way to get from version to list! :)

Cosmetics: It looks like `PYTHON_VERSION_STRING` and 
`PYTHONLIBS_VERSION_STRING` are global and all others are local variables? 
Using lowercase names for locals makes it easier to see. As names are 
case-sensitive, it would also avoid the collision with the actual 
`PYTHON_MAJOR/MINOR`.




Comment at: lldb/cmake/modules/LLDBConfig.cmake:205
+# doesn't set PYTHON_VERSION_MAJOR or PYTHON_VERSION_MINOR. Use the LLDB
+# prefix to not shadow the actual variables.
+string(REPLACE "." ";" PYTHON_VERSION_LIST ${PYTHON_VERSION_STRING})

I think CMake would not shadow, but overwrite them in directory scope (which is 
root here, so everywhere except the cache).


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D65230: [CMake] Loosen Python version check and ignore patch version

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: shafik, arphaman, sgraenitz, davide.
JDevlieghere added a project: LLDB.
Herald added subscribers: dexonsmith, mgorny.

Some versions of macOS report a different patch version for the system provided 
interpreter and libraries.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D65230

Files:
  lldb/cmake/modules/LLDBConfig.cmake


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -199,9 +199,24 @@
 find_package(PythonLibs REQUIRED)
   endif()
 
-  if (NOT PYTHON_VERSION_STRING VERSION_EQUAL PYTHONLIBS_VERSION_STRING AND
-  NOT CMAKE_CROSSCOMPILING)
-message(FATAL_ERROR "Found incompatible Python interpreter 
(${PYTHON_VERSION_STRING}) and Python libraries (${PYTHONLIBS_VERSION_STRING})")
+  if (NOT CMAKE_CROSSCOMPILING)
+# Manually split PYTHON_VERSION_STRING because find_python_libs_windows
+# doesn't set PYTHON_VERSION_MAJOR or PYTHON_VERSION_MINOR. Use the LLDB
+# prefix to not shadow the actual variables.
+string(REPLACE "." ";" PYTHON_VERSION_LIST ${PYTHON_VERSION_STRING})
+string(REPLACE "." ";" PYTHONLIBS_VERSION_LIST 
${PYTHONLIBS_VERSION_STRING})
+list(GET PYTHON_VERSION_LIST 0 LLDB_PYTHON_MAJOR)
+list(GET PYTHON_VERSION_LIST 1 LLDB_PYTHON_MINOR)
+list(GET PYTHONLIBS_VERSION_LIST 0 LLDB_PYTHONLIBS_MAJOR)
+list(GET PYTHONLIBS_VERSION_LIST 1 LLDB_PYTHONLIBS_MINOR)
+
+# Ignore the patch version. Some versions of macOS report a different patch
+# version for the system provided interpreter and libraries.
+if (NOT LLDB_PYTHON_MAJOR VERSION_EQUAL LLDB_PYTHONLIBS_MAJOR OR
+NOT LLDB_PYTHON_MINOR VERSION_EQUAL LLDB_PYTHONLIBS_MINOR)
+  message(FATAL_ERROR "Found incompatible Python interpreter 
(${LLDB_PYTHON_MAJOR}.${LLDB_PYTHON_MINOR})"
+  " and Python libraries 
(${LLDB_PYTHONLIBS_MAJOR}.${LLDB_PYTHONLIBS_MINOR})")
+endif()
   endif()
 
   if (PYTHON_INCLUDE_DIR)


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -199,9 +199,24 @@
 find_package(PythonLibs REQUIRED)
   endif()
 
-  if (NOT PYTHON_VERSION_STRING VERSION_EQUAL PYTHONLIBS_VERSION_STRING AND
-  NOT CMAKE_CROSSCOMPILING)
-message(FATAL_ERROR "Found incompatible Python interpreter (${PYTHON_VERSION_STRING}) and Python libraries (${PYTHONLIBS_VERSION_STRING})")
+  if (NOT CMAKE_CROSSCOMPILING)
+# Manually split PYTHON_VERSION_STRING because find_python_libs_windows
+# doesn't set PYTHON_VERSION_MAJOR or PYTHON_VERSION_MINOR. Use the LLDB
+# prefix to not shadow the actual variables.
+string(REPLACE "." ";" PYTHON_VERSION_LIST ${PYTHON_VERSION_STRING})
+string(REPLACE "." ";" PYTHONLIBS_VERSION_LIST ${PYTHONLIBS_VERSION_STRING})
+list(GET PYTHON_VERSION_LIST 0 LLDB_PYTHON_MAJOR)
+list(GET PYTHON_VERSION_LIST 1 LLDB_PYTHON_MINOR)
+list(GET PYTHONLIBS_VERSION_LIST 0 LLDB_PYTHONLIBS_MAJOR)
+list(GET PYTHONLIBS_VERSION_LIST 1 LLDB_PYTHONLIBS_MINOR)
+
+# Ignore the patch version. Some versions of macOS report a different patch
+# version for the system provided interpreter and libraries.
+if (NOT LLDB_PYTHON_MAJOR VERSION_EQUAL LLDB_PYTHONLIBS_MAJOR OR
+NOT LLDB_PYTHON_MINOR VERSION_EQUAL LLDB_PYTHONLIBS_MINOR)
+  message(FATAL_ERROR "Found incompatible Python interpreter (${LLDB_PYTHON_MAJOR}.${LLDB_PYTHON_MINOR})"
+  " and Python libraries (${LLDB_PYTHONLIBS_MAJOR}.${LLDB_PYTHONLIBS_MINOR})")
+endif()
   endif()
 
   if (PYTHON_INCLUDE_DIR)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits