Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CMAKE] Find gapy instead of using PATH #26

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zero9178
Copy link
Contributor

@zero9178 zero9178 commented Jun 6, 2024

The cmake script previously relied on gapy needing to be in the PATH. This makes the cmake script less self-contained and adds a magic ceremonial step required to configure the cmake build.

This PR therefore uses cmake's find_program to find the gapy script instead. This has the following advantages:

  • We can specify hints where cmake should try and find gapy. This way we can add the path from the monorepo layout as a convenient default
  • If not found through hints, find_program will look in PATH preserving previous behavior
  • The gapy executable can also be overwritten on the command line using -DGAPY_SCRIPT_PATH=...
  • If not found, a nicer error message is omitted by cmake

Additionally, the gapy script is now executed using the python interpreter CMake was configured with, not the one that is currently active. This should better ensure that it is always using a venv with the requirements installed if desired.

The cmake script previously relied on `gapy` needing to be in the PATH. This makes the cmake script less self-contained and adds a magic ceremonial step required to configure the cmake build.

This PR therefore uses cmake's `find_program` to find the `gapy` script instead.
This has the following advantages:
* We can specify hints where cmake should try and find `gapy`. This way we can add the path from the monorepo layout as a convenient default
* If not found through hints, `find_program` will look in `PATH` preserving previous behavior
* The `gapy` executable can also be overwritten on the command line using `-DGAPY_SCRIPT_PATH=...`
* If not found, a nicer error message is omitted by cmake

Additionally, the `gapy` script is now executed using the python interpreter CMake was configured with, not the one that is currently active. This should better ensure that it is always using a venv with the requirements installed if desired.
@zero9178 zero9178 requested a review from haugoug as a code owner June 6, 2024 19:22
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.

1 participant