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

[Build] CMake install Python dist-info #66

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

elliotcmorris
Copy link
Contributor

Closes #58.

Install a minimal dist-info alongside the Python sources. This means attempting to overwrite a CMake installation using pip install will error like:

This is as far as possible a direct duplication of OpenAssetIO/OpenAssetIO#1111

The cmake testing is more direct than in OpenAssetIO proper, due to the lack of the utility ecosystem, as well as the test file itself needing a minor change to account for openassetio-mediacreation being two words and thus having a hyphon.
Added some additional incidental comments around the cmake stuff, simply because I found the concepts confusing personally.

@elliotcmorris elliotcmorris self-assigned this Nov 16, 2023
@elliotcmorris elliotcmorris marked this pull request as ready for review November 16, 2023 16:32
@elliotcmorris elliotcmorris requested a review from a team as a code owner November 16, 2023 16:32
Copy link
Member

@feltech feltech left a comment

Choose a reason for hiding this comment

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

Beautiful. Some minor notes (will check again after lunch).

.github/workflows/test.yml Outdated Show resolved Hide resolved
.github/workflows/test.yml Show resolved Hide resolved
.github/workflows/test.yml Show resolved Hide resolved
.github/workflows/test.yml Outdated Show resolved Hide resolved
tests/cpp/CMakeLists.txt Outdated Show resolved Hide resolved
tests/cpp/CMakeLists.txt Outdated Show resolved Hide resolved
tests/cpp/CMakeLists.txt Outdated Show resolved Hide resolved
tests/cpp/test_cmake.py Outdated Show resolved Hide resolved
tests/cpp/test_cmake.py Outdated Show resolved Hide resolved
tests/cpp/test_cmake.py Outdated Show resolved Hide resolved
Copy link
Contributor

@foundrytom foundrytom left a comment

Choose a reason for hiding this comment

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

Aside from minor docs fix, LGTM 👍 Thanks @elliotcmorris

RELEASE_NOTES.md Outdated Show resolved Hide resolved
Part of OpenAssetIO#58. Install a minimal dist-info alongside the Python sources.
This means attempting to overwrite a CMake installation using
`pip install` will error like:

> ERROR: Cannot uninstall openassetio-mediacreation 1.0.0a7,
> RECORD file not found.
> Hint: The package was installed by cmake.

* `INSTALLER` is used by `pip` to provide the competing packager name
  ("cmake" in the example hint above).
* `METADATA` is mandatory according to the docs. It has the minimal
  contents required to be valid.
* `REQUESTED` is an empty flag file to signal that the package was
  installed individually and not as a dependency of another. Just seemed
  cheap to provide, not really necessary.
* `top_level.txt` contains the top-level namespace names provided by the
  package. Another one that seemed cheap to provide and is not really
  necessary.

Optionally skip installing `.dist-info` metadata directory as part of
installing Python files using a new
`OPENASSETIO_MEDIACREATION_ENABLE_PYTHON_INSTALL_DIST_INFO`
CMake option.

Signed-off-by: Elliot Morris <elliot.morris@foundry.com>
@elliotcmorris elliotcmorris merged commit 536fd38 into OpenAssetIO:main Nov 21, 2023
25 checks passed
@elliotcmorris elliotcmorris deleted the work/1088-dist-info branch November 21, 2023 11:16
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.

Create a dist-info directory with CMake Python builds
3 participants