From 1a13d84cb6b41c2eeeaf3d07b17bd4f30c46ac41 Mon Sep 17 00:00:00 2001 From: Elliot Morris Date: Tue, 7 Nov 2023 15:18:57 +0000 Subject: [PATCH] [Build] CMake install Python dist-info Part of #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 --- .github/workflows/test.yml | 33 ++++++++- CMakeLists.txt | 38 ++++++++++- RELEASE_NOTES.md | 9 +++ cmake/{ => packaging}/Config.cmake.in | 0 cmake/packaging/README.md | 23 +++++++ cmake/packaging/python.dist-info/INSTALLER | 1 + cmake/packaging/python.dist-info/METADATA.in | 3 + cmake/packaging/python.dist-info/REQUESTED | 0 .../packaging/python.dist-info/top_level.txt | 1 + tests/cpp/CMakeLists.txt | 49 ++++++++++++++ tests/cpp/test_cmake.py | 67 +++++++++++++++++++ 11 files changed, 222 insertions(+), 2 deletions(-) rename cmake/{ => packaging}/Config.cmake.in (100%) create mode 100644 cmake/packaging/README.md create mode 100644 cmake/packaging/python.dist-info/INSTALLER create mode 100644 cmake/packaging/python.dist-info/METADATA.in create mode 100644 cmake/packaging/python.dist-info/REQUESTED create mode 100644 cmake/packaging/python.dist-info/top_level.txt create mode 100644 tests/cpp/test_cmake.py diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 029601d..ac338e5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -98,4 +98,35 @@ jobs: - name: Test run: | python -m pip install -r tests/python/requirements.txt - PYTHONPATH=$(pwd)/build/dist/hybridpython python -m pytest -v \ No newline at end of file + PYTHONPATH=$(pwd)/build/dist/hybridpython python -m pytest -v + + disallowed_pip_install: + name: Disallowed pip install + runs-on: ubuntu-latest + container: + image: aswf/ci-vfxall:2022-clang14.3 + + steps: + - uses: actions/checkout@v3 + + - name: Install Traitgen + run: python -m pip install openassetio-traitgen==1.0.0a7 + + - name: Set Python Root Dir + run: echo "Python_ROOT_DIR=$(python -c 'import sys; print(sys.prefix)')" >> $GITHUB_ENV + + - name: Configure CMake build + run: > + cmake -S . -B build --install-prefix $Python_ROOT_DIR -DOPENASSETIO_MEDIACREATION_GENERATE_PYTHON=ON + + - name: Install package + run: cmake --install build + + - name: Attempt to install using pip + # The runner has pipefail set, so if either the pip install + # succeeds (inverted via `!`) or the grep fails, then the step + # will fail. + run: > + ! python -m pip install --upgrade --force-reinstall openassetio-mediacreation 2>&1 + | grep "The package was installed by cmake" + shell: bash diff --git a/CMakeLists.txt b/CMakeLists.txt index c67a87f..d1e11fb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -22,6 +22,12 @@ if (OPENASSETIO_MEDIACREATION_GENERATE_PYTHON) "" CACHE STRING "Override default Python module install directory, relative to CMAKE_INSTALL_PREFIX") + + set(OPENASSETIO_MEDIACREATION_ENABLE_PYTHON_INSTALL_DIST_INFO_desc + "Create a dist-info metadata directory alongside Python installation to provide" + " discoverability and prevent overwrite by package managers such as pip") + option(OPENASSETIO_MEDIACREATION_ENABLE_PYTHON_INSTALL_DIST_INFO + "${OPENASSETIO_MEDIACREATION_ENABLE_PYTHON_INSTALL_DIST_INFO_desc}" ON) endif () message(STATUS "Test enabled = ${OPENASSETIO_MEDIACREATION_ENABLE_TEST}") @@ -130,7 +136,7 @@ write_basic_package_version_file(${_version_config_file} COMPATIBILITY SameMajorVersion) configure_package_config_file( - cmake/Config.cmake.in + cmake/packaging/Config.cmake.in ${_project_config_file} INSTALL_DESTINATION ${_config_install_dir} ) @@ -152,6 +158,36 @@ if (OPENASSETIO_MEDIACREATION_GENERATE_PYTHON) DESTINATION "${OPENASSETIO_MEDIACREATION_PYTHON_SITEDIR}" FILES_MATCHING PATTERN "*.py" ) + + #------------------------------------------------------------------- + # Install dist-info into the Python environment, to prevent + # accidental overwrite, e.g. pip. + + if (OPENASSETIO_MEDIACREATION_ENABLE_PYTHON_INSTALL_DIST_INFO) + file(READ pyproject.toml _pyproject_toml) + string(REGEX MATCH [[version *= *"([^"]+)"]] _unused "${_pyproject_toml}") + set(OPENASSETIO_MEDIACREATION_PYTHON_PKG_VERSION ${CMAKE_MATCH_1}) + if (NOT OPENASSETIO_MEDIACREATION_PYTHON_PKG_VERSION) + message(FATAL_ERROR "Failed to parse version from pyproject.toml") + endif () + set(_dist_info_dir_name + openassetio_mediacreation-${OPENASSETIO_MEDIACREATION_PYTHON_PKG_VERSION}.dist-info) + file( + COPY + "${PROJECT_SOURCE_DIR}/cmake/packaging/python.dist-info/INSTALLER" + "${PROJECT_SOURCE_DIR}/cmake/packaging/python.dist-info/REQUESTED" + "${PROJECT_SOURCE_DIR}/cmake/packaging/python.dist-info/top_level.txt" + DESTINATION "${_dist_info_dir_name}" + ) + configure_file( + "${PROJECT_SOURCE_DIR}/cmake/packaging/python.dist-info/METADATA.in" + "${_dist_info_dir_name}/METADATA" + ) + install( + DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/${_dist_info_dir_name}" + DESTINATION "${OPENASSETIO_MEDIACREATION_PYTHON_SITEDIR}" + ) + endif () endif() #----------------------------------------------------------------------- diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 3e7a225..2abb587 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -10,6 +10,15 @@ v1.0.0-alpha.x compatibility with `openassetio` `v1.0.0a14`. [#60](https://github.com/OpenAssetIO/OpenAssetIO-MediaCreation/issues/60) +- Added some protection for accidental overwrites of a CMake installed + `openassetio-mediacreation` Python package, by installing a + `.dist-info` metadata directory alongside the package. `pip install` + will then fail/warn against accidental overwrites/overrides. Added a + CMake variable + `OPENASSETIO_MEDIACREATION_ENABLE_PYTHON_INSTALL_DIST_INFO` to disable + this feature. + [#58](https://github.com/OpenAssetIO/OpenAssetIO-MediaCreation/issues/58) + v1.0.0-alpha.7 -------------- diff --git a/cmake/Config.cmake.in b/cmake/packaging/Config.cmake.in similarity index 100% rename from cmake/Config.cmake.in rename to cmake/packaging/Config.cmake.in diff --git a/cmake/packaging/README.md b/cmake/packaging/README.md new file mode 100644 index 0000000..9ee0536 --- /dev/null +++ b/cmake/packaging/README.md @@ -0,0 +1,23 @@ +# Metadata files to be bundled with the installed package + +This directory contains files used for package discovery in the install +tree. + +This includes the CMake config files, as well as a Python "dist-info" +bundle. + +The CMake config files are used to allow CMake's `find_package` to +discover an installed OpenAssetIO-MediaCreation CMake package (see [CMake docs](https://cmake.org/cmake/help/latest/manual/cmake-packages.7.html) +for more info). + +The Python dist-info allows Python package managers (such as pip) to +detect the presence of an `openassetio-mediacreation` package, and is +configured such that a well-behaved package manager will error when +trying to overwrite it. Specifically, the dist-info deliberately +excludes a `RECORD` file (see [Python +docs](https://packaging.python.org/en/latest/specifications/recording-installed-packages/#intentionally-preventing-changes-to-installed-packages) +for more info). + +Some of the config files are templates (as evidenced by the `.in` +suffix), and will be rendered to their final form as part of a CMake +build/install. \ No newline at end of file diff --git a/cmake/packaging/python.dist-info/INSTALLER b/cmake/packaging/python.dist-info/INSTALLER new file mode 100644 index 0000000..b6868d4 --- /dev/null +++ b/cmake/packaging/python.dist-info/INSTALLER @@ -0,0 +1 @@ +cmake \ No newline at end of file diff --git a/cmake/packaging/python.dist-info/METADATA.in b/cmake/packaging/python.dist-info/METADATA.in new file mode 100644 index 0000000..21103d3 --- /dev/null +++ b/cmake/packaging/python.dist-info/METADATA.in @@ -0,0 +1,3 @@ +Metadata-Version: 2.1 +Name: openassetio-mediacreation +Version: @OPENASSETIO_MEDIACREATION_PYTHON_PACKAGE_VERSION@ \ No newline at end of file diff --git a/cmake/packaging/python.dist-info/REQUESTED b/cmake/packaging/python.dist-info/REQUESTED new file mode 100644 index 0000000..e69de29 diff --git a/cmake/packaging/python.dist-info/top_level.txt b/cmake/packaging/python.dist-info/top_level.txt new file mode 100644 index 0000000..00a11af --- /dev/null +++ b/cmake/packaging/python.dist-info/top_level.txt @@ -0,0 +1 @@ +openassetio_mediacreation \ No newline at end of file diff --git a/tests/cpp/CMakeLists.txt b/tests/cpp/CMakeLists.txt index 9176ded..b3fd7b1 100644 --- a/tests/cpp/CMakeLists.txt +++ b/tests/cpp/CMakeLists.txt @@ -20,3 +20,52 @@ OpenAssetIO::openassetio-core OpenAssetIO-MediaCreation::openassetio-mediacreation) target_compile_features(test.cpp PRIVATE cxx_std_17) + +#----------------------------------------------------------------------- +# CMake Python packaging tests. (Dist-info) + +if (OPENASSETIO_MEDIACREATION_ENABLE_PYTHON_INSTALL_DIST_INFO) + + # Build the command to extend the PYTHONPATH such that the + # site-packages directory in the install tree is included correctly. + set(_set_pythonpath_command + PYTHONPATH=${CMAKE_INSTALL_PREFIX}/${OPENASSETIO_MEDIACREATION_PYTHON_SITEDIR}) + + # Add pytest target to run the packaging tests. These are concerned + # with python metadata information so run in a python context. + add_custom_target( + openassetio-mediacreation.tests.packaging + COMMAND cmake -E echo -- "Running pytest check for CMake dist-info packaging" + COMMAND ${_set_pythonpath_command} && + pytest -v --capture=tee-sys + "${CMAKE_CURRENT_LIST_DIR}/test_cmake.py" + WORKING_DIRECTORY "${PROJECT_SOURCE_DIR}" + USES_TERMINAL + ) + + # Create a test fixture that performs the install step. + add_test( + NAME openassetio-mediacreation.internal.tests.install + COMMAND "${CMAKE_COMMAND}" --build "${PROJECT_BINARY_DIR}" --target install + ) + set_tests_properties(openassetio-mediacreation.internal.tests.install + PROPERTIES FIXTURES_SETUP test_install) + + # Add the packaging test, and set the install as a required fixture. + add_test( + NAME openassetio-mediacreation.tests.packaging + COMMAND ${CMAKE_COMMAND} --build "${PROJECT_BINARY_DIR}" + --target openassetio-mediacreation.tests.packaging + ) + set_tests_properties(openassetio-mediacreation.tests.packaging + PROPERTIES FIXTURES_REQUIRED test_install) + + # Set the project version as an environment variable accesible to + # the packaging tests. + set_tests_properties( + openassetio-mediacreation.tests.packaging + PROPERTIES + ENVIRONMENT OPENASSETIO_MEDIACREATION_CMAKE_PACKAGE_VERSION=${PROJECT_VERSION} + ) + +endif () diff --git a/tests/cpp/test_cmake.py b/tests/cpp/test_cmake.py new file mode 100644 index 0000000..e81c1ea --- /dev/null +++ b/tests/cpp/test_cmake.py @@ -0,0 +1,67 @@ +# +# Copyright 2013-2023 The Foundry Visionmongers Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +""" +Test CMake installed package. +Python script invoked from Ctest, in order to test structure +and contents of cmake installed package. Particularly with a focus +on the .dist_info metadata required to make this package play well +with pip. +""" +# pylint: disable=missing-function-docstring +import os + +import pytest + +try: + from importlib import metadata +except: + import importlib_metadata as metadata + + +@pytest.mark.skipif( + os.environ.get("OPENASSETIO_MEDIACREATION_CMAKE_PACKAGE_VERSION") is None, + reason="CMake package only", +) +def test_cmake_dist_info(): + dist = metadata.distribution("openassetio-mediacreation") + + # Check METADATA file exists with required keys. + assert {"Name", "Metadata-Version", "Version"}.issubset(dist.metadata.keys()) + assert dist.metadata["Name"] == "openassetio-mediacreation" + assert dist.metadata["Version"].startswith( + os.environ["OPENASSETIO_MEDIACREATION_CMAKE_PACKAGE_VERSION"] + ) + + # The lack of a RECORD means that `pip` is unable to accidentally + # uninstall the package + assert dist.read_text("RECORD") is None + + # The INSTALLER is used by `pip` to provide a hint when reporting + # that it is unable to install a package due to no RECORD. + installer = dist.read_text("INSTALLER") + assert installer is not None + assert installer.strip() == "cmake" + + # The above uses files in the dist-info directory, whereas pip uses + # the directory name itself. So check that they match. + # However, names use hyphen and paths use underscores, so switch. + underscore_name = dist.metadata["Name"].replace("-", "_") + dist_info_path = os.path.join( + dist.locate_file(""), f"{underscore_name}-{dist.metadata['Version']}.dist-info" + ) + assert os.path.isdir(dist_info_path) + dist_from_dir = metadata.Distribution.at(dist_info_path) + assert dict(dist.metadata) == dict(dist_from_dir.metadata)