You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by GitBox <gi...@apache.org> on 2021/02/02 12:45:11 UTC
[GitHub] [qpid-proton] jiridanek opened a new pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
jiridanek opened a new pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294
Use https://cmake.org/cmake/help/git-stage/module/FindPython.html
There is few issues with doing this
* branching on CMake version is bad practice
* PYTHON_EXECUTABLE can be specified from outside, this changes user API (depending on CMake version)
* Travis CI apparently uses CMake 3.12 (on all Ubuntu versions); can be upgraded
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher commented on a change in pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
astitcher commented on a change in pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294#discussion_r570318386
##########
File path: CMakeLists.txt
##########
@@ -32,7 +32,27 @@ include (CheckPythonModule)
find_package (OpenSSL)
find_package (Threads)
-find_package (PythonInterp REQUIRED)
+# FindPython was added in CMake 3.12, but there it always returned
+# newest Python on the entire PATH. We want to use the first one.
+if (CMAKE_VERSION VERSION_LESS "3.15.0")
+ find_package (PythonInterp REQUIRED)
+ # forward compatibility with FindPython
+ set(Python_EXECUTABLE "${PYTHON_EXECUTABLE}")
+ set(Python_LIBRARIES "${PYTHON_LIBRARIES}")
+ set(Python_INCLUDE_DIRS "${PYTHON_INCLUDE_PATH}")
+ set(Python_VERSION_STRING "${PYTHON_VERSION_STRING}")
+ # for completeness, these are not actually used now
+ set(Python_VERSION_MAJOR "${PYTHON_VERSION_MAJOR}")
+ set(Python_VERSION_MINOR "${PYTHON_VERSION_MINOR}")
+ set(Python_VERSION_PATCH "${PYTHON_VERSION_PATCH}")
+else ()
+ if (POLICY CMP0094) # https://cmake.org/cmake/help/latest/policy/CMP0094.html
+ cmake_policy(SET CMP0094 NEW) # FindPython should return the first matching Python on PATH
+ endif ()
+
+ find_package(Python REQUIRED COMPONENTS Interpreter
+ OPTIONAL_COMPONENTS Development)
+endif()
Review comment:
Yes your version will shadow the system one - but you can get it explicitly go look at tools/cmake/Modules/FindThreads.cmake for instance - we've done this before!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294#discussion_r574714200
##########
File path: tools/cmake/Modules/FindPython.cmake
##########
@@ -0,0 +1,66 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+# This is a wrapper hack purely so that we can use FindPython
+# with cmake 2.8.12 and its supplied older modules
+
+if (POLICY CMP0094) # https://cmake.org/cmake/help/latest/policy/CMP0094.html
+ cmake_policy(SET CMP0094 NEW) # FindPython should return the first matching Python on PATH
+endif ()
+
+if (DEFINED PYTHON_EXECUTABLE AND DEFINED Python_EXECUTABLE)
+ message(FATAL_ERROR "Both PYTHON_EXECUTABLE and Python_EXECUTABLE are defined. Define at most one of those.")
+endif ()
+
+# FindPython was added in CMake 3.12, but there it always returned
+# newest Python on the entire PATH. We want to use the first one.
+if (CMAKE_VERSION VERSION_LESS "3.15.0")
+ if (DEFINED Python_EXECUTABLE)
+ set(PYTHON_EXECUTABLE ${Python_EXECUTABLE})
+ endif ()
+
+ find_package (PythonInterp REQUIRED)
+ # forward compatibility with FindPython
+ set(Python_VERSION_STRING "${PYTHON_VERSION_STRING}")
+ set(Python_EXECUTABLE "${PYTHON_EXECUTABLE}")
+ # for completeness, these are not actually used now
+ set(Python_VERSION_MAJOR "${PYTHON_VERSION_MAJOR}")
+ set(Python_VERSION_MINOR "${PYTHON_VERSION_MINOR}")
+ set(Python_VERSION_PATCH "${PYTHON_VERSION_PATCH}")
+
+ find_package (PythonLibs ${PYTHON_VERSION_STRING} EXACT)
+ set(Python_Development_FOUND "${PYTHONLIBS_FOUND}")
+ set(Python_INCLUDE_DIRS "${PYTHON_INCLUDE_PATH}")
+ set(Python_LIBRARIES "${PYTHON_LIBRARIES}")
+else ()
+ if (DEFINED PYTHON_EXECUTABLE)
+ set(Python_EXECUTABLE ${PYTHON_EXECUTABLE})
+ endif ()
+
+ # needed on GitHub Actions CI: actions/setup-python does not touch registry/frameworks on Windows/macOS
+ # this mirrors PythonInterp behavior which did not consult registry/frameworks first
+ if (NOT DEFINED Python_FIND_REGISTRY)
+ set(Python_FIND_REGISTRY "LAST")
+ endif ()
+ if (NOT DEFINED Python_FIND_FRAMEWORK)
+ set(Python_FIND_FRAMEWORK "LAST")
+ endif ()
Review comment:
@astitcher This should take care about finding the right Python on GitHub Actions. New CMake defaults to looking for Virtual environment (and in CI does not find any), then goes to look into Windows Registers. The default behavior consults PATH last, if everything else failed. It's in the docs, I somehow forgot I read about it before https://cmake.org/cmake/help/v3.15/module/FindPython.html#hints.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher commented on a change in pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
astitcher commented on a change in pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294#discussion_r574826338
##########
File path: tools/cmake/Modules/FindPython.cmake
##########
@@ -0,0 +1,66 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+# This is a wrapper hack purely so that we can use FindPython
+# with cmake 2.8.12 and its supplied older modules
+
+if (POLICY CMP0094) # https://cmake.org/cmake/help/latest/policy/CMP0094.html
+ cmake_policy(SET CMP0094 NEW) # FindPython should return the first matching Python on PATH
+endif ()
+
+if (DEFINED PYTHON_EXECUTABLE AND DEFINED Python_EXECUTABLE)
+ message(FATAL_ERROR "Both PYTHON_EXECUTABLE and Python_EXECUTABLE are defined. Define at most one of those.")
+endif ()
+
+# FindPython was added in CMake 3.12, but there it always returned
+# newest Python on the entire PATH. We want to use the first one.
+if (CMAKE_VERSION VERSION_LESS "3.15.0")
+ if (DEFINED Python_EXECUTABLE)
+ set(PYTHON_EXECUTABLE ${Python_EXECUTABLE})
+ endif ()
+
+ find_package (PythonInterp REQUIRED)
+ # forward compatibility with FindPython
+ set(Python_VERSION_STRING "${PYTHON_VERSION_STRING}")
+ set(Python_EXECUTABLE "${PYTHON_EXECUTABLE}")
+ # for completeness, these are not actually used now
+ set(Python_VERSION_MAJOR "${PYTHON_VERSION_MAJOR}")
+ set(Python_VERSION_MINOR "${PYTHON_VERSION_MINOR}")
+ set(Python_VERSION_PATCH "${PYTHON_VERSION_PATCH}")
+
+ find_package (PythonLibs ${PYTHON_VERSION_STRING} EXACT)
+ set(Python_Development_FOUND "${PYTHONLIBS_FOUND}")
+ set(Python_INCLUDE_DIRS "${PYTHON_INCLUDE_PATH}")
+ set(Python_LIBRARIES "${PYTHON_LIBRARIES}")
+else ()
+ if (DEFINED PYTHON_EXECUTABLE)
+ set(Python_EXECUTABLE ${PYTHON_EXECUTABLE})
+ endif ()
+
+ # needed on GitHub Actions CI: actions/setup-python does not touch registry/frameworks on Windows/macOS
+ # this mirrors PythonInterp behavior which did not consult registry/frameworks first
Review comment:
II think this comment should just read:
`# This mirrors PythonInterp behavior on windows, which did not consult registry/frameworks first.`
IMO the specifics of github actions behaviour is not that important.
##########
File path: tools/cmake/Modules/FindPython.cmake
##########
@@ -0,0 +1,66 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+# This is a wrapper hack purely so that we can use FindPython
+# with cmake 2.8.12 and its supplied older modules
+
+if (POLICY CMP0094) # https://cmake.org/cmake/help/latest/policy/CMP0094.html
+ cmake_policy(SET CMP0094 NEW) # FindPython should return the first matching Python on PATH
+endif ()
+
+if (DEFINED PYTHON_EXECUTABLE AND DEFINED Python_EXECUTABLE)
+ message(FATAL_ERROR "Both PYTHON_EXECUTABLE and Python_EXECUTABLE are defined. Define at most one of those.")
+endif ()
+
+# FindPython was added in CMake 3.12, but there it always returned
+# newest Python on the entire PATH. We want to use the first one.
+if (CMAKE_VERSION VERSION_LESS "3.15.0")
+ if (DEFINED Python_EXECUTABLE)
+ set(PYTHON_EXECUTABLE ${Python_EXECUTABLE})
+ endif ()
+
+ find_package (PythonInterp REQUIRED)
+ # forward compatibility with FindPython
+ set(Python_VERSION_STRING "${PYTHON_VERSION_STRING}")
+ set(Python_EXECUTABLE "${PYTHON_EXECUTABLE}")
+ # for completeness, these are not actually used now
+ set(Python_VERSION_MAJOR "${PYTHON_VERSION_MAJOR}")
+ set(Python_VERSION_MINOR "${PYTHON_VERSION_MINOR}")
+ set(Python_VERSION_PATCH "${PYTHON_VERSION_PATCH}")
+
+ find_package (PythonLibs ${PYTHON_VERSION_STRING} EXACT)
+ set(Python_Development_FOUND "${PYTHONLIBS_FOUND}")
+ set(Python_INCLUDE_DIRS "${PYTHON_INCLUDE_PATH}")
+ set(Python_LIBRARIES "${PYTHON_LIBRARIES}")
+else ()
+ if (DEFINED PYTHON_EXECUTABLE)
+ set(Python_EXECUTABLE ${PYTHON_EXECUTABLE})
+ endif ()
+
+ # needed on GitHub Actions CI: actions/setup-python does not touch registry/frameworks on Windows/macOS
+ # this mirrors PythonInterp behavior which did not consult registry/frameworks first
+ if (NOT DEFINED Python_FIND_REGISTRY)
+ set(Python_FIND_REGISTRY "LAST")
+ endif ()
+ if (NOT DEFINED Python_FIND_FRAMEWORK)
+ set(Python_FIND_FRAMEWORK "LAST")
+ endif ()
Review comment:
Hmm, not sure this bit of code should go in the find module wrapper at all actually. As when we are able to only use cmake 3.15 everywhere (say in a decade or so!) we'll still need code like this for Windows/Mac.
Maybe we should have these lines:
```
set(Python_FIND_REGISTRY "LAST")
set(Python_FIND_FRAMEWORK "LAST")
```
before the `find_package(Python...)` line in `CMakeLists.txt`.
Possibly in both places?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher commented on a change in pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
astitcher commented on a change in pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294#discussion_r570318386
##########
File path: CMakeLists.txt
##########
@@ -32,7 +32,27 @@ include (CheckPythonModule)
find_package (OpenSSL)
find_package (Threads)
-find_package (PythonInterp REQUIRED)
+# FindPython was added in CMake 3.12, but there it always returned
+# newest Python on the entire PATH. We want to use the first one.
+if (CMAKE_VERSION VERSION_LESS "3.15.0")
+ find_package (PythonInterp REQUIRED)
+ # forward compatibility with FindPython
+ set(Python_EXECUTABLE "${PYTHON_EXECUTABLE}")
+ set(Python_LIBRARIES "${PYTHON_LIBRARIES}")
+ set(Python_INCLUDE_DIRS "${PYTHON_INCLUDE_PATH}")
+ set(Python_VERSION_STRING "${PYTHON_VERSION_STRING}")
+ # for completeness, these are not actually used now
+ set(Python_VERSION_MAJOR "${PYTHON_VERSION_MAJOR}")
+ set(Python_VERSION_MINOR "${PYTHON_VERSION_MINOR}")
+ set(Python_VERSION_PATCH "${PYTHON_VERSION_PATCH}")
+else ()
+ if (POLICY CMP0094) # https://cmake.org/cmake/help/latest/policy/CMP0094.html
+ cmake_policy(SET CMP0094 NEW) # FindPython should return the first matching Python on PATH
+ endif ()
+
+ find_package(Python REQUIRED COMPONENTS Interpreter
+ OPTIONAL_COMPONENTS Development)
+endif()
Review comment:
Yes your version will shadow the system one - but you can get it explicitly go look at tools/cmake/Modules/FindThreads.cmake for instance - we've done this before!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294#discussion_r570320151
##########
File path: CMakeLists.txt
##########
@@ -32,7 +32,27 @@ include (CheckPythonModule)
find_package (OpenSSL)
find_package (Threads)
-find_package (PythonInterp REQUIRED)
+# FindPython was added in CMake 3.12, but there it always returned
+# newest Python on the entire PATH. We want to use the first one.
+if (CMAKE_VERSION VERSION_LESS "3.15.0")
+ find_package (PythonInterp REQUIRED)
+ # forward compatibility with FindPython
+ set(Python_EXECUTABLE "${PYTHON_EXECUTABLE}")
+ set(Python_LIBRARIES "${PYTHON_LIBRARIES}")
+ set(Python_INCLUDE_DIRS "${PYTHON_INCLUDE_PATH}")
+ set(Python_VERSION_STRING "${PYTHON_VERSION_STRING}")
+ # for completeness, these are not actually used now
+ set(Python_VERSION_MAJOR "${PYTHON_VERSION_MAJOR}")
+ set(Python_VERSION_MINOR "${PYTHON_VERSION_MINOR}")
+ set(Python_VERSION_PATCH "${PYTHON_VERSION_PATCH}")
+else ()
+ if (POLICY CMP0094) # https://cmake.org/cmake/help/latest/policy/CMP0094.html
+ cmake_policy(SET CMP0094 NEW) # FindPython should return the first matching Python on PATH
+ endif ()
+
+ find_package(Python REQUIRED COMPONENTS Interpreter
+ OPTIONAL_COMPONENTS Development)
+endif()
Review comment:
Aha, makes sense; I agree that's the correct way. At least while CMake < 3.15 needs to be supported....
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher commented on pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
astitcher commented on pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294#issuecomment-772843307
I'd also say that you might as well use find_package(Python3 REQUIRED) as well (obviously you'd need to wait until we drop Python 2 - but not too long now).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294#issuecomment-777739384
`GitHub Actions has encountered an internal error when running your job. ` That's a first for me.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294#issuecomment-773153406
It could be `find_package(Python "3.6.0" REQUIRED)`, maybe. That way, the script will need not have to change when Python 4 comes out ;P
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher commented on pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
astitcher commented on pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294#issuecomment-773394598
> It could be `find_package(Python "3.6.0" REQUIRED)`, maybe. That way, the script will need not have to change when Python 4 comes out ;P
By all accounts that is *never* going to happen! (2->3 was so painful)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294#discussion_r573064096
##########
File path: tools/cmake/Modules/FindPython.cmake
##########
@@ -0,0 +1,49 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+# This is a wrapper hack purely so that we can use FindPython
+# with cmake 2.8.12 and its supplied older modules
+
+# FindPython was added in CMake 3.12, but there it always returned
+# newest Python on the entire PATH. We want to use the first one.
+if (CMAKE_VERSION VERSION_LESS "3.15.0")
+ find_package (PythonInterp REQUIRED)
+ # forward compatibility with FindPython
+ set(Python_EXECUTABLE "${PYTHON_EXECUTABLE}")
+ set(Python_LIBRARIES "${PYTHON_LIBRARIES}")
+ set(Python_INCLUDE_DIRS "${PYTHON_INCLUDE_PATH}")
+ set(Python_VERSION_STRING "${PYTHON_VERSION_STRING}")
+ # for completeness, these are not actually used now
+ set(Python_VERSION_MAJOR "${PYTHON_VERSION_MAJOR}")
+ set(Python_VERSION_MINOR "${PYTHON_VERSION_MINOR}")
+ set(Python_VERSION_PATCH "${PYTHON_VERSION_PATCH}")
+
+ find_package (PythonLibs ${PYTHON_VERSION_STRING} EXACT)
+ set(Python_Development_FOUND "${PYTHONLIBS_FOUND}")
+else ()
+ if (POLICY CMP0094) # https://cmake.org/cmake/help/latest/policy/CMP0094.html
+ cmake_policy(SET CMP0094 NEW) # FindPython should return the first matching Python on PATH
+ endif ()
+
+ if (DEFINED PYTHON_EXECUTABLE)
+ set(Python_EXECUTABLE ${PYTHON_EXECUTABLE})
+ endif ()
Review comment:
I'm doing this for forward compatibility. Some users might've gotten used to setting `PYTHON_EXECUTABLE` as a hint for CMake, so it finds the correct interpreter. If both variables are set, then the user is crazy person, I think. :P
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294#discussion_r570058120
##########
File path: CMakeLists.txt
##########
@@ -32,7 +32,27 @@ include (CheckPythonModule)
find_package (OpenSSL)
find_package (Threads)
-find_package (PythonInterp REQUIRED)
+# FindPython was added in CMake 3.12, but there it always returned
+# newest Python on the entire PATH. We want to use the first one.
+if (CMAKE_VERSION VERSION_LESS "3.15.0")
+ find_package (PythonInterp REQUIRED)
+ # forward compatibility with FindPython
+ set(Python_EXECUTABLE "${PYTHON_EXECUTABLE}")
+ set(Python_LIBRARIES "${PYTHON_LIBRARIES}")
+ set(Python_INCLUDE_DIRS "${PYTHON_INCLUDE_PATH}")
+ set(Python_VERSION_STRING "${PYTHON_VERSION_STRING}")
+ # for completeness, these are not actually used now
+ set(Python_VERSION_MAJOR "${PYTHON_VERSION_MAJOR}")
+ set(Python_VERSION_MINOR "${PYTHON_VERSION_MINOR}")
+ set(Python_VERSION_PATCH "${PYTHON_VERSION_PATCH}")
+else ()
+ if (POLICY CMP0094) # https://cmake.org/cmake/help/latest/policy/CMP0094.html
+ cmake_policy(SET CMP0094 NEW) # FindPython should return the first matching Python on PATH
+ endif ()
+
+ find_package(Python REQUIRED COMPONENTS Interpreter
+ OPTIONAL_COMPONENTS Development)
+endif()
Review comment:
Is this going to work? Because my own FindPython.cmake will end up shadowing the CMake3's find_package(Python), I think. Not sure, I'll have to try that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294#issuecomment-777309859
Windows GHA build is broken by this. I'll have to figure out what's wrong there.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294#issuecomment-773153406
It could be `find_package(Python "3.6.0" REQUIRED)`, maybe. That way, the script will need not have to change when Python 4 comes out ;P
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher edited a comment on pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
astitcher edited a comment on pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294#issuecomment-777599005
> Windows GHA build is broken by this. I'll have to figure out what's wrong there.
For some reason the cmake configure is finding python 3.9.1 even though the build script explicitly asks for 3.6. I'm not sure that's exactly the cause, but it is something that should not be happening.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294#discussion_r570058120
##########
File path: CMakeLists.txt
##########
@@ -32,7 +32,27 @@ include (CheckPythonModule)
find_package (OpenSSL)
find_package (Threads)
-find_package (PythonInterp REQUIRED)
+# FindPython was added in CMake 3.12, but there it always returned
+# newest Python on the entire PATH. We want to use the first one.
+if (CMAKE_VERSION VERSION_LESS "3.15.0")
+ find_package (PythonInterp REQUIRED)
+ # forward compatibility with FindPython
+ set(Python_EXECUTABLE "${PYTHON_EXECUTABLE}")
+ set(Python_LIBRARIES "${PYTHON_LIBRARIES}")
+ set(Python_INCLUDE_DIRS "${PYTHON_INCLUDE_PATH}")
+ set(Python_VERSION_STRING "${PYTHON_VERSION_STRING}")
+ # for completeness, these are not actually used now
+ set(Python_VERSION_MAJOR "${PYTHON_VERSION_MAJOR}")
+ set(Python_VERSION_MINOR "${PYTHON_VERSION_MINOR}")
+ set(Python_VERSION_PATCH "${PYTHON_VERSION_PATCH}")
+else ()
+ if (POLICY CMP0094) # https://cmake.org/cmake/help/latest/policy/CMP0094.html
+ cmake_policy(SET CMP0094 NEW) # FindPython should return the first matching Python on PATH
+ endif ()
+
+ find_package(Python REQUIRED COMPONENTS Interpreter
+ OPTIONAL_COMPONENTS Development)
+endif()
Review comment:
Is this going to work? Because my own FindPython.cmake will end up shadowing the CMake3's find_package(Python), I think. Not sure, I'll have to try that.
##########
File path: CMakeLists.txt
##########
@@ -32,7 +32,27 @@ include (CheckPythonModule)
find_package (OpenSSL)
find_package (Threads)
-find_package (PythonInterp REQUIRED)
+# FindPython was added in CMake 3.12, but there it always returned
+# newest Python on the entire PATH. We want to use the first one.
+if (CMAKE_VERSION VERSION_LESS "3.15.0")
+ find_package (PythonInterp REQUIRED)
+ # forward compatibility with FindPython
+ set(Python_EXECUTABLE "${PYTHON_EXECUTABLE}")
+ set(Python_LIBRARIES "${PYTHON_LIBRARIES}")
+ set(Python_INCLUDE_DIRS "${PYTHON_INCLUDE_PATH}")
+ set(Python_VERSION_STRING "${PYTHON_VERSION_STRING}")
+ # for completeness, these are not actually used now
+ set(Python_VERSION_MAJOR "${PYTHON_VERSION_MAJOR}")
+ set(Python_VERSION_MINOR "${PYTHON_VERSION_MINOR}")
+ set(Python_VERSION_PATCH "${PYTHON_VERSION_PATCH}")
+else ()
+ if (POLICY CMP0094) # https://cmake.org/cmake/help/latest/policy/CMP0094.html
+ cmake_policy(SET CMP0094 NEW) # FindPython should return the first matching Python on PATH
+ endif ()
+
+ find_package(Python REQUIRED COMPONENTS Interpreter
+ OPTIONAL_COMPONENTS Development)
+endif()
Review comment:
Aha, makes sense; I agree that's the correct way. At least while CMake < 3.15 needs to be supported....
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher commented on pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
astitcher commented on pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294#issuecomment-773394598
> It could be `find_package(Python "3.6.0" REQUIRED)`, maybe. That way, the script will need not have to change when Python 4 comes out ;P
By all accounts that is *never* going to happen! (2->3 was so painful)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294#discussion_r575176165
##########
File path: tools/cmake/Modules/FindPython.cmake
##########
@@ -0,0 +1,66 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+# This is a wrapper hack purely so that we can use FindPython
+# with cmake 2.8.12 and its supplied older modules
+
+if (POLICY CMP0094) # https://cmake.org/cmake/help/latest/policy/CMP0094.html
+ cmake_policy(SET CMP0094 NEW) # FindPython should return the first matching Python on PATH
+endif ()
+
+if (DEFINED PYTHON_EXECUTABLE AND DEFINED Python_EXECUTABLE)
+ message(FATAL_ERROR "Both PYTHON_EXECUTABLE and Python_EXECUTABLE are defined. Define at most one of those.")
+endif ()
+
+# FindPython was added in CMake 3.12, but there it always returned
+# newest Python on the entire PATH. We want to use the first one.
+if (CMAKE_VERSION VERSION_LESS "3.15.0")
+ if (DEFINED Python_EXECUTABLE)
+ set(PYTHON_EXECUTABLE ${Python_EXECUTABLE})
+ endif ()
+
+ find_package (PythonInterp REQUIRED)
+ # forward compatibility with FindPython
+ set(Python_VERSION_STRING "${PYTHON_VERSION_STRING}")
+ set(Python_EXECUTABLE "${PYTHON_EXECUTABLE}")
+ # for completeness, these are not actually used now
+ set(Python_VERSION_MAJOR "${PYTHON_VERSION_MAJOR}")
+ set(Python_VERSION_MINOR "${PYTHON_VERSION_MINOR}")
+ set(Python_VERSION_PATCH "${PYTHON_VERSION_PATCH}")
+
+ find_package (PythonLibs ${PYTHON_VERSION_STRING} EXACT)
+ set(Python_Development_FOUND "${PYTHONLIBS_FOUND}")
+ set(Python_INCLUDE_DIRS "${PYTHON_INCLUDE_PATH}")
+ set(Python_LIBRARIES "${PYTHON_LIBRARIES}")
+else ()
+ if (DEFINED PYTHON_EXECUTABLE)
+ set(Python_EXECUTABLE ${PYTHON_EXECUTABLE})
+ endif ()
+
+ # needed on GitHub Actions CI: actions/setup-python does not touch registry/frameworks on Windows/macOS
+ # this mirrors PythonInterp behavior which did not consult registry/frameworks first
+ if (NOT DEFINED Python_FIND_REGISTRY)
+ set(Python_FIND_REGISTRY "LAST")
+ endif ()
+ if (NOT DEFINED Python_FIND_FRAMEWORK)
+ set(Python_FIND_FRAMEWORK "LAST")
+ endif ()
Review comment:
> say in a decade or so!
Middle of this year, by my estimate! It is contingent on RHEL 8.4 being released and on getting rid of some of the older Ubuntu in CI.
I'm putting the `set`s in the top level CMake only. And want to keep the ifs around it, so that users can override it, if they really want to prioritize systemwide setting for python. I guess that CMake made that the default behavior for a good reason. It is only my fault that I cannot figure what reason that might be,
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher commented on a change in pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
astitcher commented on a change in pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294#discussion_r573058292
##########
File path: tools/cmake/Modules/FindPython.cmake
##########
@@ -0,0 +1,49 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+# This is a wrapper hack purely so that we can use FindPython
+# with cmake 2.8.12 and its supplied older modules
+
+# FindPython was added in CMake 3.12, but there it always returned
+# newest Python on the entire PATH. We want to use the first one.
+if (CMAKE_VERSION VERSION_LESS "3.15.0")
+ find_package (PythonInterp REQUIRED)
+ # forward compatibility with FindPython
+ set(Python_EXECUTABLE "${PYTHON_EXECUTABLE}")
+ set(Python_LIBRARIES "${PYTHON_LIBRARIES}")
+ set(Python_INCLUDE_DIRS "${PYTHON_INCLUDE_PATH}")
+ set(Python_VERSION_STRING "${PYTHON_VERSION_STRING}")
+ # for completeness, these are not actually used now
+ set(Python_VERSION_MAJOR "${PYTHON_VERSION_MAJOR}")
+ set(Python_VERSION_MINOR "${PYTHON_VERSION_MINOR}")
+ set(Python_VERSION_PATCH "${PYTHON_VERSION_PATCH}")
+
+ find_package (PythonLibs ${PYTHON_VERSION_STRING} EXACT)
+ set(Python_Development_FOUND "${PYTHONLIBS_FOUND}")
+else ()
+ if (POLICY CMP0094) # https://cmake.org/cmake/help/latest/policy/CMP0094.html
+ cmake_policy(SET CMP0094 NEW) # FindPython should return the first matching Python on PATH
+ endif ()
+
+ if (DEFINED PYTHON_EXECUTABLE)
+ set(Python_EXECUTABLE ${PYTHON_EXECUTABLE})
+ endif ()
Review comment:
What if both PYTHON_EXECUTABLE and Python_EXECUTABLE are set? I know it seems unlikely, but if it happens I think what this does might be surprising - I'd expect Python_EXECUTABLE not to be overridden.
##########
File path: tools/cmake/Modules/FindPython.cmake
##########
@@ -0,0 +1,49 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+# This is a wrapper hack purely so that we can use FindPython
+# with cmake 2.8.12 and its supplied older modules
+
+# FindPython was added in CMake 3.12, but there it always returned
+# newest Python on the entire PATH. We want to use the first one.
+if (CMAKE_VERSION VERSION_LESS "3.15.0")
+ find_package (PythonInterp REQUIRED)
+ # forward compatibility with FindPython
+ set(Python_EXECUTABLE "${PYTHON_EXECUTABLE}")
+ set(Python_LIBRARIES "${PYTHON_LIBRARIES}")
+ set(Python_INCLUDE_DIRS "${PYTHON_INCLUDE_PATH}")
+ set(Python_VERSION_STRING "${PYTHON_VERSION_STRING}")
+ # for completeness, these are not actually used now
+ set(Python_VERSION_MAJOR "${PYTHON_VERSION_MAJOR}")
+ set(Python_VERSION_MINOR "${PYTHON_VERSION_MINOR}")
+ set(Python_VERSION_PATCH "${PYTHON_VERSION_PATCH}")
+
+ find_package (PythonLibs ${PYTHON_VERSION_STRING} EXACT)
+ set(Python_Development_FOUND "${PYTHONLIBS_FOUND}")
+else ()
+ if (POLICY CMP0094) # https://cmake.org/cmake/help/latest/policy/CMP0094.html
+ cmake_policy(SET CMP0094 NEW) # FindPython should return the first matching Python on PATH
+ endif ()
Review comment:
Given this checks if the policy exists I think you could hoist this out of the if block checking the cmake version - I think this would be a little tidier.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher commented on pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
astitcher commented on pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294#issuecomment-777599005
> Windows GHA build is broken by this. I'll have to figure out what's wrong there.
For some reason the cmake configure is finding python 3.9.1 even though the build script explicitly asks for 3.6. I'm not sure that's exactly the cause, but it is something that should not be happening.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher commented on a change in pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
astitcher commented on a change in pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294#discussion_r569466506
##########
File path: CMakeLists.txt
##########
@@ -32,7 +32,27 @@ include (CheckPythonModule)
find_package (OpenSSL)
find_package (Threads)
-find_package (PythonInterp REQUIRED)
+# FindPython was added in CMake 3.12, but there it always returned
+# newest Python on the entire PATH. We want to use the first one.
+if (CMAKE_VERSION VERSION_LESS "3.15.0")
+ find_package (PythonInterp REQUIRED)
+ # forward compatibility with FindPython
+ set(Python_EXECUTABLE "${PYTHON_EXECUTABLE}")
+ set(Python_LIBRARIES "${PYTHON_LIBRARIES}")
+ set(Python_INCLUDE_DIRS "${PYTHON_INCLUDE_PATH}")
+ set(Python_VERSION_STRING "${PYTHON_VERSION_STRING}")
+ # for completeness, these are not actually used now
+ set(Python_VERSION_MAJOR "${PYTHON_VERSION_MAJOR}")
+ set(Python_VERSION_MINOR "${PYTHON_VERSION_MINOR}")
+ set(Python_VERSION_PATCH "${PYTHON_VERSION_PATCH}")
+else ()
+ if (POLICY CMP0094) # https://cmake.org/cmake/help/latest/policy/CMP0094.html
+ cmake_policy(SET CMP0094 NEW) # FindPython should return the first matching Python on PATH
+ endif ()
+
+ find_package(Python REQUIRED COMPONENTS Interpreter
+ OPTIONAL_COMPONENTS Development)
+endif()
Review comment:
**This is totally the wrong approach!**
We already have a way to "branch" the processing so that we can use newer features. Create our own FindPython.cmake in tools/cmake/Modules which imitates the cmake 3 find_package(Python) processing so far as we use it.
Also make sure that any installed script will work with cmake 2.8.12 as our target environment is cmake 2.8.12 (sadly).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek merged pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
jiridanek merged pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #294: PROTON-2217 Prefer first working Python on PATH, prefer Py3 over Py2
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #294:
URL: https://github.com/apache/qpid-proton/pull/294#discussion_r573122587
##########
File path: tools/cmake/Modules/FindPython.cmake
##########
@@ -0,0 +1,53 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+# This is a wrapper hack purely so that we can use FindPython
+# with cmake 2.8.12 and its supplied older modules
+
+# FindPython was added in CMake 3.12, but there it always returned
+# newest Python on the entire PATH. We want to use the first one.
+if (CMAKE_VERSION VERSION_LESS "3.15.0")
+ if (DEFINED Python_EXECUTABLE)
+ set(PYTHON_EXECUTABLE ${Python_EXECUTABLE})
+ endif ()
+
+ find_package (PythonInterp REQUIRED)
+ # forward compatibility with FindPython
+ set(Python_EXECUTABLE "${PYTHON_EXECUTABLE}")
+ set(Python_LIBRARIES "${PYTHON_LIBRARIES}")
+ set(Python_INCLUDE_DIRS "${PYTHON_INCLUDE_PATH}")
Review comment:
hmm, PYTHON_INCLUDE_PATH won't be set until I find PythonLibs
##########
File path: tools/cmake/Modules/FindPython.cmake
##########
@@ -0,0 +1,49 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+# This is a wrapper hack purely so that we can use FindPython
+# with cmake 2.8.12 and its supplied older modules
+
+# FindPython was added in CMake 3.12, but there it always returned
+# newest Python on the entire PATH. We want to use the first one.
+if (CMAKE_VERSION VERSION_LESS "3.15.0")
+ find_package (PythonInterp REQUIRED)
+ # forward compatibility with FindPython
+ set(Python_EXECUTABLE "${PYTHON_EXECUTABLE}")
+ set(Python_LIBRARIES "${PYTHON_LIBRARIES}")
+ set(Python_INCLUDE_DIRS "${PYTHON_INCLUDE_PATH}")
+ set(Python_VERSION_STRING "${PYTHON_VERSION_STRING}")
+ # for completeness, these are not actually used now
+ set(Python_VERSION_MAJOR "${PYTHON_VERSION_MAJOR}")
+ set(Python_VERSION_MINOR "${PYTHON_VERSION_MINOR}")
+ set(Python_VERSION_PATCH "${PYTHON_VERSION_PATCH}")
+
+ find_package (PythonLibs ${PYTHON_VERSION_STRING} EXACT)
+ set(Python_Development_FOUND "${PYTHONLIBS_FOUND}")
+else ()
+ if (POLICY CMP0094) # https://cmake.org/cmake/help/latest/policy/CMP0094.html
+ cmake_policy(SET CMP0094 NEW) # FindPython should return the first matching Python on PATH
+ endif ()
+
+ if (DEFINED PYTHON_EXECUTABLE)
+ set(Python_EXECUTABLE ${PYTHON_EXECUTABLE})
+ endif ()
Review comment:
@astitcher if both are set, I'd fail the build. WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org