From 78bca131fb4fa0981f6abf30d2ec790656ef8812 Mon Sep 17 00:00:00 2001 From: Matt Drozt Date: Fri, 4 Aug 2023 07:44:23 -0700 Subject: [PATCH] [py-smartredis] New Package (#39098) * Create a spack package for smartredis python client * make py-SR deps versions match docs * tie SR v0.4.0 to redis-plus-plus v1.3.5 * looser extension lib deps for concretization * Apply suggestions from code review Co-authored-by: Adam J. Stewart * Address reviewer feedback --------- Co-authored-by: Adam J. Stewart --- .../builtin/packages/py-smartredis/package.py | 52 +++++++++ .../py-smartredis/sr_0_4_0_no_deps.patch | 101 ++++++++++++++++++ .../py-smartredis/sr_0_4_1_no_deps.patch | 94 ++++++++++++++++ 3 files changed, 247 insertions(+) create mode 100644 var/spack/repos/builtin/packages/py-smartredis/package.py create mode 100644 var/spack/repos/builtin/packages/py-smartredis/sr_0_4_0_no_deps.patch create mode 100644 var/spack/repos/builtin/packages/py-smartredis/sr_0_4_1_no_deps.patch diff --git a/var/spack/repos/builtin/packages/py-smartredis/package.py b/var/spack/repos/builtin/packages/py-smartredis/package.py new file mode 100644 index 0000000000..12969a0454 --- /dev/null +++ b/var/spack/repos/builtin/packages/py-smartredis/package.py @@ -0,0 +1,52 @@ +# Copyright 2013-2023 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +from spack.package import * + + +class PySmartredis(PythonPackage): + """A Python Interface for the SmartRedis Library Client""" + + homepage = "https://www.craylabs.org/docs/smartredis.html" + pypi = "smartredis/smartredis-0.4.0.tar.gz" + git = "https://github.com/CrayLabs/SmartRedis" + + maintainers("MattToast") + + version("0.4.1", sha256="fff16ed1eb09648ac3c3f845373beb37f3ffe7414d8745ae36af9daf585f8c5b") + version("0.4.0", sha256="d12779aa8bb038e837c25eac41b178aab9e16b729d50ee360b5af8f813d9f1dd") + + depends_on("python@3.7:3.10", type=("build", "run")) + depends_on("py-setuptools@42:", type=("build",)) + + depends_on("cmake@3.13:", type=("build",)) + + # Documented dependencies + depends_on("hiredis@1.1:", type=("build", "link", "run"), when="@0.4.1") + depends_on("hiredis@1.0:", type=("build", "link", "run"), when="@0.4.0") + depends_on("redis-plus-plus@1.3.5: cxxstd=17", type=("build", "link")) + + # Unlisted dependency needed to build the python client. The pybind requirement + # can be found: + # - in the `build-scripts/build_deps.sh` for SmartRedis <= v0.4.0 + # - in the `Makefile` under the `pybind` target for SmartRedis >= v0.4.1 + depends_on("py-pybind11", type=("build",)) + + depends_on("py-numpy@1.18.2:", type=("build", "run")) + + # By default, the `setup.py` for SmartRedis <= v0.4.1 will fetch dependencies and + # use them to build the extension library; it does not allow users to supply + # their own previously obtained dependencies. These patches remove the 'autofetch' + # behavior and use the dependencies provided through spack. + patch("sr_0_4_1_no_deps.patch", when="@0.4.1") + patch("sr_0_4_0_no_deps.patch", when="@0.4.0") + + def setup_build_environment(self, env): + spec = self.spec + env.set("REDISPP_LIB_DIR", spec["redis-plus-plus"].libs.directories[0]) + env.set("REDISPP_INC_DIR", spec["redis-plus-plus"].headers.directories[0]) + env.set("HIREDIS_LIB_DIR", spec["hiredis"].libs.directories[0]) + env.set("HIREDIS_INC_DIR", spec["hiredis"].headers.directories[0]) + env.set("PYBIND11_TOOLS", spec["py-pybind11"].prefix.share.cmake.pybind11) diff --git a/var/spack/repos/builtin/packages/py-smartredis/sr_0_4_0_no_deps.patch b/var/spack/repos/builtin/packages/py-smartredis/sr_0_4_0_no_deps.patch new file mode 100644 index 0000000000..a22145eabe --- /dev/null +++ b/var/spack/repos/builtin/packages/py-smartredis/sr_0_4_0_no_deps.patch @@ -0,0 +1,101 @@ +diff --git a/CMakeLists.txt b/CMakeLists.txt +index 7dc8931..658d823 100644 +--- a/CMakeLists.txt ++++ b/CMakeLists.txt +@@ -59,8 +59,20 @@ if (COVERAGE) + endif() + endif() + +-find_library(REDISPP redis++ PATHS ${CMAKE_SOURCE_DIR}/install/lib NO_DEFAULT_PATH REQUIRED) +-find_library(HIREDIS hiredis PATHS ${CMAKE_SOURCE_DIR}/install/lib NO_DEFAULT_PATH REQUIRED) ++set(REDISPP_LIB_DIR ${CMAKE_SOURCE_DIR}/install/lib CACHE PATH "path to redis++") ++set(HIREDIS_LIB_DIR ${CMAKE_SOURCE_DIR}/install/lib CACHE PATH "path to hiredis") ++ ++set(REDISPP_INC_DIR ${CMAKE_SOURCE_DIR}/install/include CACHE PATH "path to redis++") ++set(HIREDIS_INC_DIR ${CMAKE_SOURCE_DIR}/install/include CACHE PATH "path to hiredis") ++ ++message("USING RPP PATH: ${REDISPP_LIB_DIR}") ++message("USING HIR PATH: ${HIREDIS_LIB_DIR}") ++message("USING RPP IPATH: ${REDISPP_INC_DIR}") ++message("USING HIR IPATH: ${HIREDIS_INC_DIR}") ++ ++ ++find_library(REDISPP libredis++.a PATHS ${REDISPP_LIB_DIR} NO_DEFAULT_PATH REQUIRED) ++find_library(HIREDIS hiredis PATHS ${HIREDIS_LIB_DIR} NO_DEFAULT_PATH REQUIRED) + find_package(Threads REQUIRED) + + set(EXT_CLIENT_LIBRARIES ${REDISPP} ${HIREDIS}) +@@ -106,7 +118,8 @@ set(CLIENT_SRC + + include_directories(SYSTEM + include +- install/include ++ ${REDISPP_INC_DIR} ++ ${HIREDIS_INC_DIR} + ) + + if (BUILD_FORTRAN) +@@ -148,8 +161,9 @@ install(TARGETS smartredis + + if(BUILD_PYTHON) + message("-- Python client build enabled") +- add_subdirectory(${CMAKE_SOURCE_DIR}/third-party/pybind +- ${CMAKE_SOURCE_DIR}/third-party/pybind/build) ++ ++ set(pybind11_DIR ${CMAKE_SOURCE_DIR}/third-party/pybind/tools) ++ find_package(pybind11) + + add_library(smartredis_static STATIC ${CLIENT_SRC}) + +diff --git a/setup.py b/setup.py +index dd19c6c..4248aef 100644 +--- a/setup.py ++++ b/setup.py +@@ -58,9 +58,20 @@ class CMakeBuild(build_ext): + build_directory = Path(self.build_temp).resolve() + cmake_args = [ + '-DCMAKE_LIBRARY_OUTPUT_DIRECTORY=' + str(build_directory), +- '-DPYTHON_EXECUTABLE=' + sys.executable ++ '-DPYTHON_EXECUTABLE=' + sys.executable, ++ '-Dpybind11_DIR=' + str(os.getenv('PYBIND11_TOOLS')), + ] + ++ for setting in [ ++ "REDISPP_LIB_DIR", ++ "REDISPP_INC_DIR", ++ "HIREDIS_LIB_DIR", ++ "HIREDIS_INC_DIR", ++ ]: ++ val = os.getenv(setting) ++ if val is not None: ++ cmake_args.append(f"-D{setting}={val}") ++ + cfg = 'Debug' if self.debug else 'Release' + build_args = ['--config', cfg] + build_args += ['--', f'-j{str(NPROC)}'] +@@ -78,15 +89,8 @@ class CMakeBuild(build_ext): + if not build_directory.is_dir(): + os.makedirs(self.build_temp) + +- print('-'*10, 'Building C dependencies', '-'*40) +- make_cmd = shutil.which("make") + setup_path = Path(os.path.abspath(os.path.dirname(__file__))).resolve() + +- # build dependencies +- subprocess.check_call([f"{make_cmd} deps"], +- cwd=setup_path, +- shell=True) +- + # run cmake prep step + print('-'*10, 'Running CMake prepare', '-'*40) + subprocess.check_call([self.cmake, setup_path] + cmake_args, +@@ -99,9 +103,6 @@ class CMakeBuild(build_ext): + subprocess.check_call(cmake_cmd, + cwd=build_directory) + +- shutil.copytree(setup_path.joinpath("install"), +- build_directory.joinpath("install")) +- + # Move from build temp to final position + for ext in self.extensions: + self.move_output(ext) diff --git a/var/spack/repos/builtin/packages/py-smartredis/sr_0_4_1_no_deps.patch b/var/spack/repos/builtin/packages/py-smartredis/sr_0_4_1_no_deps.patch new file mode 100644 index 0000000000..78fb4d2103 --- /dev/null +++ b/var/spack/repos/builtin/packages/py-smartredis/sr_0_4_1_no_deps.patch @@ -0,0 +1,94 @@ +diff --git a/CMakeLists.txt b/CMakeLists.txt +index ca88a4d..30118fc 100644 +--- a/CMakeLists.txt ++++ b/CMakeLists.txt +@@ -64,13 +64,27 @@ if (SR_PEDANTIC) + endif() + endif() + ++set(REDISPP_LIB_DIR ${CMAKE_SOURCE_DIR}/install/lib CACHE PATH "path to redis++") ++set(REDISPP_INC_DIR ${CMAKE_SOURCE_DIR}/install/include CACHE PATH ++ "path to redis++ headers") ++ ++set(HIREDIS_LIB_DIR ${CMAKE_SOURCE_DIR}/install/lib CACHE PATH "path to hiredis") ++set(HIREDIS_INC_DIR ${CMAKE_SOURCE_DIR}/install/include CACHE PATH ++ "path to hiredis headers") ++ ++message("USING REDIS++ PATH: ${REDISPP_LIB_DIR}") ++message("USING REDIS++ INCLUDE PATH: ${REDISPP_INC_DIR}") ++ ++message("USING HIREDIS PATH: ${HIREDIS_LIB_DIR}") ++message("USING HIREDIS INCLUDE PATH: ${HIREDIS_INC_DIR}") ++ + # Bring in third-party libaries needed for the SmartRedis library + find_library(REDISPP redis++ +- PATHS ${CMAKE_SOURCE_DIR}/install/lib NO_DEFAULT_PATH ++ PATHS ${REDISPP_LIB_DIR} NO_DEFAULT_PATH + REQUIRED STATIC + ) + find_library(HIREDIS hiredis +- PATHS ${CMAKE_SOURCE_DIR}/install/lib NO_DEFAULT_PATH ++ PATHS ${HIREDIS_LIB_DIR} NO_DEFAULT_PATH + REQUIRED STATIC + ) + find_package(Threads REQUIRED) +@@ -121,7 +135,8 @@ set(CLIENT_SRC + # Define include directories for header files + include_directories(SYSTEM + include +- install/include ++ ${REDISPP_INC_DIR} ++ ${HIREDIS_INC_DIR} + ) + + # Build the Fortran library +@@ -177,8 +192,8 @@ install(TARGETS smartredis + # Build the Python library for SmartRedis + if(SR_PYTHON) + message("-- Python client build enabled") +- add_subdirectory(${CMAKE_SOURCE_DIR}/third-party/pybind +- ${CMAKE_SOURCE_DIR}/third-party/pybind/build) ++ set(pybind11_DIR ${CMAKE_SOURCE_DIR}/third-party/pybind/tools) ++ find_package(pybind11) + + pybind11_add_module(smartredisPy + src/python/src/pyclient.cpp +diff --git a/setup.py b/setup.py +index 90493ee..dd075db 100644 +--- a/setup.py ++++ b/setup.py +@@ -73,14 +73,6 @@ class CMakeBuild(build_ext): + env.get('CXXFLAGS', ''), + self.distribution.get_version()) + +- # Build dependencies +- print('-'*10, 'Building third-party dependencies', '-'*40) +- subprocess.check_call( +- [self.make, "deps"], +- cwd=source_directory, +- shell=False +- ) +- + # Run CMake config step + print('-'*10, 'Configuring build', '-'*40) + config_args = [ +@@ -90,7 +82,19 @@ class CMakeBuild(build_ext): + '-DCMAKE_LIBRARY_OUTPUT_DIRECTORY=' + str(build_directory), + '-DPYTHON_EXECUTABLE=' + sys.executable, + '-DSR_PYTHON=ON', ++ '-Dpybind11_DIR=' + str(os.getenv('PYBIND11_TOOLS')), + ] ++ ++ for setting in [ ++ "REDISPP_LIB_DIR", ++ "REDISPP_INC_DIR", ++ "HIREDIS_LIB_DIR", ++ "HIREDIS_INC_DIR", ++ ]: ++ val = os.getenv(setting) ++ if val is not None: ++ config_args.append(f"-D{setting}={val}") ++ + subprocess.check_call( + [self.cmake] + config_args, + cwd=source_directory,