Merge pull request OpenAssetIO#66 from elliotcmorris/work/1088-dist-info

elliotcmorris · web-flow · commit 536fd3841a0b · 2023-11-21T11:16:38.000Z
[Build]  CMake install Python dist-info
diff --git 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
+          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
@@ -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
@@ -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/packaging/Config.cmake.in b/cmake/packaging/Config.cmake.in
diff --git a/cmake/packaging/README.md 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.
diff --git a/cmake/packaging/python.dist-info/INSTALLER b/cmake/packaging/python.dist-info/INSTALLER
@@ -0,0 +1 @@
+cmake
diff --git a/cmake/packaging/python.dist-info/METADATA.in 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@
diff --git a/cmake/packaging/python.dist-info/REQUESTED b/cmake/packaging/python.dist-info/REQUESTED
diff --git a/cmake/packaging/python.dist-info/top_level.txt b/cmake/packaging/python.dist-info/top_level.txt
@@ -0,0 +1 @@
+openassetio_mediacreation
diff --git 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
@@ -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)

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+Metadata-Version: 2.1`
	`2`	`+Name: openassetio-mediacreation`
	`3`	`+Version: @OPENASSETIO_MEDIACREATION_PYTHON_PACKAGE_VERSION@`