Skip to content

Updated build.py to support relative CMake/CTest paths.#205

Merged
edgchen1 merged 1 commit intomasterfrom
edgchen1/build_py_resolve_cmake_path
Dec 18, 2018
Merged

Updated build.py to support relative CMake/CTest paths.#205
edgchen1 merged 1 commit intomasterfrom
edgchen1/build_py_resolve_cmake_path

Conversation

@edgchen1
Copy link
Contributor

No description provided.

@edgchen1 edgchen1 requested a review from a team as a code owner December 18, 2018 19:10

def resolve_executable_path(command_or_path):
"""Returns the absolute path of an executable."""
executable_path = shutil.which(command_or_path)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why adding this function? What if the input is already an absolute path?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's already an absolute path it should be fine. If not, it may not work since the current directory gets changed for some commands. I installed a later cmake version on Linux and gave a relative path from the repo and that didn't work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then why don't you give an absolute path to this script? People often have multiple cmake installed, calling which cmake at here may get unexpected result?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Giving an absolute path works (which is what I had to do). It seemed like a bug that a relative path does not also work.

For --cmake_path, the default is "cmake". So for the default, I assumed running a cmake command starting with the result of shutil.which("cmake") would be equivalent.

@edgchen1 edgchen1 merged commit 0aa1b54 into master Dec 18, 2018
@edgchen1 edgchen1 deleted the edgchen1/build_py_resolve_cmake_path branch December 18, 2018 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants