From d61f4fa49f84d867cc1926fb8d0c8c97574bd5af Mon Sep 17 00:00:00 2001 From: Nils Leif Fischer Date: Tue, 1 Feb 2022 14:43:42 +0100 Subject: [PATCH 1/4] Fix installation of SpECTRE Python package with pip It worked in editable mode (pip install -e), but not everything was copied correctly without editable mode. Setuptools [recommends](https://setuptools.pypa.io/en/latest/userguide/quickstart.html) explicitly specifying a backend in a pyproject.toml file and configuring metadata in setup.cfg, so that's what we do. --- cmake/SpectreSetupPythonPackage.cmake | 9 ++++++--- src/PythonBindings/pyproject.toml | 6 ++++++ src/PythonBindings/setup.cfg | 21 +++++++++++++++++++++ src/PythonBindings/setup.py | 23 ----------------------- 4 files changed, 33 insertions(+), 26 deletions(-) create mode 100644 src/PythonBindings/pyproject.toml create mode 100644 src/PythonBindings/setup.cfg delete mode 100644 src/PythonBindings/setup.py diff --git a/cmake/SpectreSetupPythonPackage.cmake b/cmake/SpectreSetupPythonPackage.cmake index 302c9dc4f5ab..af7305367778 100644 --- a/cmake/SpectreSetupPythonPackage.cmake +++ b/cmake/SpectreSetupPythonPackage.cmake @@ -23,10 +23,13 @@ if(NOT EXISTS "${SPECTRE_PYTHON_PREFIX}/__init__.py") "__all__ = []") endif() -# Write a file for installing the Python modules +# Write configuration files for installing the Python modules configure_file( - "${CMAKE_SOURCE_DIR}/src/PythonBindings/setup.py" - "${SPECTRE_PYTHON_PREFIX_PARENT}/setup.py") + "${CMAKE_SOURCE_DIR}/src/PythonBindings/pyproject.toml" + "${SPECTRE_PYTHON_PREFIX_PARENT}/pyproject.toml") +configure_file( + "${CMAKE_SOURCE_DIR}/src/PythonBindings/setup.cfg" + "${SPECTRE_PYTHON_PREFIX_PARENT}/setup.cfg") set(_JEMALLOC_MESSAGE "") if(BUILD_PYTHON_BINDINGS AND "${JEMALLOC_LIB_TYPE}" STREQUAL SHARED) diff --git a/src/PythonBindings/pyproject.toml b/src/PythonBindings/pyproject.toml new file mode 100644 index 000000000000..c809f6c5c68e --- /dev/null +++ b/src/PythonBindings/pyproject.toml @@ -0,0 +1,6 @@ +# Distributed under the MIT License. +# See LICENSE.txt for details. + +[build-system] +requires = ["setuptools"] +build-backend = "setuptools.build_meta" diff --git a/src/PythonBindings/setup.cfg b/src/PythonBindings/setup.cfg new file mode 100644 index 000000000000..4243e536eb08 --- /dev/null +++ b/src/PythonBindings/setup.cfg @@ -0,0 +1,21 @@ +# Distributed under the MIT License. +# See LICENSE.txt for details. + +[metadata] +name = spectre +version = @SPECTRE_VERSION@ +description = Python bindings for SpECTRE +author = SXS collaboration +url = @SPECTRE_HOMEPAGE@ +license = MIT + +[options] +packages = find: +install_requires = + h5py >= 3.0.0 + numpy + scipy + +[options.package_data] +# Install Python bindings libs alongside the Python code +* = *.so diff --git a/src/PythonBindings/setup.py b/src/PythonBindings/setup.py deleted file mode 100644 index d965dce70796..000000000000 --- a/src/PythonBindings/setup.py +++ /dev/null @@ -1,23 +0,0 @@ -#!/usr/bin/env python - -# Distributed under the MIT License. -# See LICENSE.txt for details. - -from distutils.core import setup - -setup( - name='spectre', - version='@SPECTRE_VERSION@', - description="Python bindings for SpECTRE", - author="SXS collaboration", - url="@SPECTRE_HOMEPAGE@", - license="MIT", - packages=['spectre'], - install_requires=['h5py', 'numpy', 'scipy'], - classifiers=[ - "Programming Language :: Python :: 2.7", - "Programming Language :: Python :: 3", - "License :: OSI Approved :: MIT License", - "Operating System :: OS Independent", - ], -) From d4121d074894f61ca4c47ae0fa69f2379c2a1269 Mon Sep 17 00:00:00 2001 From: Nils Leif Fischer Date: Tue, 1 Feb 2022 14:50:01 +0100 Subject: [PATCH 2/4] Install SpECTRE Python package using pip This sets up entry points automatically. It also chooses the installation subdirectory automatically. --- cmake/SpectreSetupPythonPackage.cmake | 20 ++++++++++++-------- docs/DevGuide/BuildSystem.md | 4 ---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/cmake/SpectreSetupPythonPackage.cmake b/cmake/SpectreSetupPythonPackage.cmake index af7305367778..4748d5bcb8a7 100644 --- a/cmake/SpectreSetupPythonPackage.cmake +++ b/cmake/SpectreSetupPythonPackage.cmake @@ -1,11 +1,6 @@ # Distributed under the MIT License. # See LICENSE.txt for details. -set(SPECTRE_PYTHON_INSTALL_LIBDIR - "lib/python${Python_VERSION_MAJOR}.${Python_VERSION_MINOR}/site-packages" - CACHE STRING "Location where the Python package is installed. Defaults to \ -CMAKE_INSTALL_PREFIX/lib/pythonX.Y/site-packages/.") - spectre_define_test_timeout_factor_option(PYTHON "Python") set(SPECTRE_PYTHON_PREFIX "${CMAKE_BINARY_DIR}/bin/python/spectre/") @@ -50,10 +45,17 @@ configure_file( "${CMAKE_BINARY_DIR}/tmp/LoadPython.sh" "${CMAKE_BINARY_DIR}/bin/LoadPython.sh") -# Install the SpECTRE Python package to the user-specified location. +# Install the SpECTRE Python package to the CMAKE_INSTALL_PREFIX, using pip. +# This will install the package into the expected subdirectory, typically +# `lib/pythonX.Y/site-packages/`. It also creates symlinks to entry points +# specified in `setup.py`. install( - DIRECTORY ${SPECTRE_PYTHON_PREFIX} - DESTINATION ${SPECTRE_PYTHON_INSTALL_LIBDIR} + CODE "execute_process(\ + COMMAND ${Python_EXECUTABLE} -m pip install \ + --no-deps --no-input --no-cache-dir --no-index --ignore-installed \ + --disable-pip-version-check --no-build-isolation \ + --prefix ${CMAKE_INSTALL_PREFIX} ${SPECTRE_PYTHON_PREFIX_PARENT} \ + )" ) add_custom_target(all-pybindings) @@ -260,6 +262,8 @@ function(SPECTRE_PYTHON_ADD_MODULE MODULE_NAME) # Write an executable in `${CMAKE_BINARY_DIR}/bin` that runs the Python # script in the correct Python environment if(${PYTHON_FILE} IN_LIST ARG_PYTHON_EXECUTABLES) + set(PYTHON_SCRIPT_LOCATION + "${PYTHON_MODULE_LOCATION}.${PYTHON_FILE_JUST_NAME_WE}") configure_file( "${CMAKE_SOURCE_DIR}/cmake/SpectrePythonExecutable.sh" "${CMAKE_BINARY_DIR}/tmp/${PYTHON_FILE_JUST_NAME_WE}") diff --git a/docs/DevGuide/BuildSystem.md b/docs/DevGuide/BuildSystem.md index b4d118fda91f..3edd814b5a36 100644 --- a/docs/DevGuide/BuildSystem.md +++ b/docs/DevGuide/BuildSystem.md @@ -198,10 +198,6 @@ cmake -D FLAG1=OPT1 ... -D FLAGN=OPTN - Location where the `install` target copies executables, libraries, etc. Make sure to set this variable before you `install`, or a default location such as `/usr/local` is used. -- SPECTRE_PYTHON_INSTALL_LIBDIR - - Location where the `install` target copies the SpECTRE Python package. - Defaults to `CMAKE_INSTALL_PREFIX/lib/pythonX.Y/site-packages/`, which is a - location where Python packages are often expected. - DEBUG_SYMBOLS - Whether or not to use debug symbols (default is `ON`) - Disabling debug symbols will reduce compile time and total size of the build From b2dba416b55852e657d3b65977e416f3d28ddc51 Mon Sep 17 00:00:00 2001 From: Nils Leif Fischer Date: Tue, 1 Feb 2022 14:47:40 +0100 Subject: [PATCH 3/4] Add a main entry point to the Python module The entry point currently only supports: $ spectre --version I intend to add some basic CLI functionality that calls into our Python module, e.g. to clean output, check an input file, etc. --- cmake/SpectrePythonExecutable.sh | 2 +- cmake/SpectreSetupPythonPackage.cmake | 15 +++++++++++++++ src/PythonBindings/setup.cfg | 4 ++++ support/Python/__main__.py | 27 +++++++++++++++++++++++++++ tests/CMakeLists.txt | 1 + tests/support/CMakeLists.txt | 4 ++++ tests/support/Python/CMakeLists.txt | 11 +++++++++++ 7 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 support/Python/__main__.py create mode 100644 tests/support/CMakeLists.txt create mode 100644 tests/support/Python/CMakeLists.txt diff --git a/cmake/SpectrePythonExecutable.sh b/cmake/SpectrePythonExecutable.sh index 9f126c8129e0..37dd26548b40 100644 --- a/cmake/SpectrePythonExecutable.sh +++ b/cmake/SpectrePythonExecutable.sh @@ -4,4 +4,4 @@ # See LICENSE.txt for details. PYTHONPATH="@CMAKE_BINARY_DIR@/bin/python:$PYTHONPATH" @Python_EXECUTABLE@ \ - -m @PYTHON_MODULE_LOCATION@.@PYTHON_FILE_JUST_NAME_WE@ "$@" + -m @PYTHON_SCRIPT_LOCATION@ "$@" diff --git a/cmake/SpectreSetupPythonPackage.cmake b/cmake/SpectreSetupPythonPackage.cmake index 4748d5bcb8a7..676f3eb74433 100644 --- a/cmake/SpectreSetupPythonPackage.cmake +++ b/cmake/SpectreSetupPythonPackage.cmake @@ -18,6 +18,21 @@ if(NOT EXISTS "${SPECTRE_PYTHON_PREFIX}/__init__.py") "__all__ = []") endif() +# Create the root __main__.py entry point +configure_file( + "${CMAKE_SOURCE_DIR}/support/Python/__main__.py" + "${SPECTRE_PYTHON_PREFIX}/__main__.py" +) +# Also link the main entry point to bin/ +set(PYTHON_SCRIPT_LOCATION "spectre") +configure_file( + "${CMAKE_SOURCE_DIR}/cmake/SpectrePythonExecutable.sh" + "${CMAKE_BINARY_DIR}/tmp/spectre") +file(COPY "${CMAKE_BINARY_DIR}/tmp/spectre" + DESTINATION "${CMAKE_BINARY_DIR}/bin" + FILE_PERMISSIONS OWNER_READ OWNER_WRITE OWNER_EXECUTE GROUP_READ + GROUP_EXECUTE WORLD_READ WORLD_EXECUTE) + # Write configuration files for installing the Python modules configure_file( "${CMAKE_SOURCE_DIR}/src/PythonBindings/pyproject.toml" diff --git a/src/PythonBindings/setup.cfg b/src/PythonBindings/setup.cfg index 4243e536eb08..988a1762a7f5 100644 --- a/src/PythonBindings/setup.cfg +++ b/src/PythonBindings/setup.cfg @@ -19,3 +19,7 @@ install_requires = [options.package_data] # Install Python bindings libs alongside the Python code * = *.so + +[options.entry_points] +console_scripts = + spectre = spectre.__main__:main diff --git a/support/Python/__main__.py b/support/Python/__main__.py new file mode 100644 index 000000000000..00f0ac1129d8 --- /dev/null +++ b/support/Python/__main__.py @@ -0,0 +1,27 @@ +# Distributed under the MIT License. +# See LICENSE.txt for details. + +SPECTRE_VERSION = "@SPECTRE_VERSION@" + + +def main(): + import argparse + + class HelpFormatter(argparse.ArgumentDefaultsHelpFormatter, + argparse.RawDescriptionHelpFormatter): + pass + + parser = argparse.ArgumentParser( + prog='spectre', + description="SpECTRE version: {}".format(SPECTRE_VERSION), + formatter_class=HelpFormatter) + + # Version endpoint + parser.add_argument('--version', action='version', version=SPECTRE_VERSION) + + parser.parse_args() + raise NotImplementedError("No subprograms are implemented yet.") + + +if __name__ == '__main__': + main() diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 12ebf25763fa..508228b7640d 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,4 +1,5 @@ # Distributed under the MIT License. # See LICENSE.txt for details. +add_subdirectory(support) add_subdirectory(Unit) diff --git a/tests/support/CMakeLists.txt b/tests/support/CMakeLists.txt new file mode 100644 index 000000000000..89480d106dd5 --- /dev/null +++ b/tests/support/CMakeLists.txt @@ -0,0 +1,4 @@ +# Distributed under the MIT License. +# See LICENSE.txt for details. + +add_subdirectory(Python) diff --git a/tests/support/Python/CMakeLists.txt b/tests/support/Python/CMakeLists.txt new file mode 100644 index 000000000000..b7850035035c --- /dev/null +++ b/tests/support/Python/CMakeLists.txt @@ -0,0 +1,11 @@ +# Distributed under the MIT License. +# See LICENSE.txt for details. + +if (BUILD_PYTHON_BINDINGS) + add_test(NAME "support.Python.main" + COMMAND ${CMAKE_BINARY_DIR}/bin/spectre --version) + set_tests_properties( + "support.Python.main" PROPERTIES + PASS_REGULAR_EXPRESSION "${SPECTRE_VERSION}" + LABELS "python") +endif() From c9419907a51195aa9b496017da92f37900de2a2a Mon Sep 17 00:00:00 2001 From: Nils Vu Date: Tue, 1 Mar 2022 16:03:50 +0100 Subject: [PATCH 4/4] Test `make install` on CI --- .github/workflows/Tests.yaml | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/.github/workflows/Tests.yaml b/.github/workflows/Tests.yaml index 256df299dc05..880d72ba749b 100644 --- a/.github/workflows/Tests.yaml +++ b/.github/workflows/Tests.yaml @@ -456,6 +456,7 @@ ${{ env.CACHE_KEY_SUFFIX }}" -D SPECTRE_UNIT_TEST_TIMEOUT_FACTOR=${TEST_TIMEOUT_FACTOR:-'1'} -D SPECTRE_INPUT_FILE_TEST_TIMEOUT_FACTOR=${TEST_TIMEOUT_FACTOR:-'1'} -D SPECTRE_PYTHON_TEST_TIMEOUT_FACTOR=${TEST_TIMEOUT_FACTOR:-'1'} + -D CMAKE_INSTALL_PREFIX=/work/spectre_install --warn-uninitialized $GITHUB_WORKSPACE 2>&1 | tee CMakeOutput.txt 2>&1 - name: Check for CMake warnings @@ -522,6 +523,17 @@ ${{ env.CACHE_KEY_SUFFIX }}" # We get occasional random timeouts, repeat tests to see if # it is a random timeout or systematic ctest -j2 -LE unit --output-on-failure --repeat after-timeout:3 + - name: Install + working-directory: /work/build + # Make sure the `install` target runs without error. We could add some + # basic smoke tests here to make sure the installation worked. + run: | + make install + - name: Print size of install directory + working-directory: /work/spectre_install + run: | + ls | xargs du -sh + du -sh . - name: Test formaline tar can be built if: matrix.build_type == 'Debug' working-directory: /work/build @@ -716,6 +728,7 @@ ${{ env.CACHE_KEY_SUFFIX }}" -D USE_PCH=ON \ -D USE_CCACHE=ON \ -D SPECTRE_TEST_TIMEOUT_FACTOR=5 \ + -D CMAKE_INSTALL_PREFIX=../install \ $GITHUB_WORKSPACE - name: Build tests working-directory: build @@ -739,6 +752,15 @@ ${{ env.CACHE_KEY_SUFFIX }}" working-directory: build run: | ctest -j3 --repeat after-timeout:3 --output-on-failure + - name: Install + working-directory: build + run: | + make install + - name: Print size of install directory + working-directory: install + run: | + ls | xargs du -sh + du -sh . # Retain caches only on the 'develop' branch (see unit_tests job) - name: Clear caches if: github.ref != 'refs/heads/develop'