cmake: Fix python3 search for CMake 3.10 (Ubuntu 18.04) #14798
Conversation
jwnimmer-tri
added
priority: high
component: build system
|
+@jamiesnape for feature review, please. |
There was a problem hiding this comment.
+@SeanCurtis-TRI for platform.
Reviewed 1 of 1 files at r1.
Reviewable status: LGTM missing from assignee jamiesnape (waiting on @jamiesnape)
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jamiesnape (waiting on @jwnimmer-tri)
CMakeLists.txt, line 225 at r1 (raw file):
find_program(PYTHON_EXECUTABLE NAMES "${FIND_PROGRAM_PYTHON3_NAMES}" PATHS "${FIND_PROGRAM_PYTHON3_PATHS}" NO_DEFAULT_PATH
What I intended was NO_DEFAULT_PATH to be removed. Somehow we ended up with some duplicate code in the two branches (obviously my fault).
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jamiesnape (waiting on @jwnimmer-tri)
CMakeLists.txt, line 225 at r1 (raw file):
Previously, jamiesnape (Jamie Snape) wrote…
What I intended was
NO_DEFAULT_PATHto be removed. Somehow we ended up with some duplicate code in the two branches (obviously my fault).
(semi-duplicated, I guess, the case is important, but maybe there is a better way to handle it)
There was a problem hiding this comment.
So, it is unlikely Mac would be using an old version, but it would technically be broken too. My preference is just remove NO_DEFAULT_PATH below.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jamiesnape (waiting on @jwnimmer-tri)
jwnimmer-tri
force-pushed
the
cmake-3.10-python
branch
from
72f223b
to
76625e6
Compare
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jamiesnape (waiting on @jamiesnape and @SeanCurtis-TRI)
CMakeLists.txt, line 225 at r1 (raw file):
Previously, jamiesnape (Jamie Snape) wrote…
(semi-duplicated, I guess, the case is important, but maybe there is a better way to handle it)
Done.
There was a problem hiding this comment.
Reviewable status:
complete! all discussions resolved, LGTM from assignees SeanCurtis-TRI(platform),jamiesnape (waiting on @SeanCurtis-TRI)
This was broken in #14605.
Closes #14796.
This change is