This is an archive of the discontinued LLVM Phabricator instance.

Fix setting Python3_ROOT_DIR on Windows
ClosedPublic

Authored by isuruf on Apr 9 2020, 5:45 PM.

Details

Reviewers
JDevlieghere
teemperor
Group Reviewers
Restricted Project
Commits
rG664fda72eaa3: Fix setting Python3_ROOT_DIR on Windows
Summary

Previously the value of Python3_ROOT_DIR was set to the string
"PYTHON_HOME" instead of the value of the variable named
PYTHON_HOME. This commit fixes that as CMake expects
a path as the value of Python3_ROOT_DIR

Diff Detail

Event Timeline

isuruf created this revision.Apr 9 2020, 5:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 9 2020, 5:45 PM
isuruf added a reviewer: Restricted Project.Apr 9 2020, 5:45 PM

Can you explain why this is necessary?

isuruf added a comment.Apr 9 2020, 7:31 PM

The intention of the code is to set the variable Python3_ROOT_DIR to the value of the variable PYTHON_HOME, but it was using just the string "PYTHON_HOME" instead.

teemperor accepted this revision.Apr 10 2020, 1:59 AM
teemperor added a subscriber: teemperor.

LGTM, PYTHON_HOME is apparently the Windows way we allow people to specify the python root when finding the package.

Please update the review description though with why this change was done that this isn't just documented in the comments here.

This revision is now accepted and ready to land.Apr 10 2020, 1:59 AM
isuruf edited the summary of this revision. (Show Details)Apr 10 2020, 4:37 PM
JDevlieghere accepted this revision.Apr 10 2020, 4:48 PM

@isuruf Do you have commit access or do you need this committed?

This revision was automatically updated to reflect the committed changes.