Skip to content

Commit 077b56c

Browse files
vmoensvmoens
committed
[CI,Setup] Fix builds
1 parent c11a95b commit 077b56c

File tree

4 files changed

+104
-21
lines changed

4 files changed

+104
-21
lines changed

‎pyproject.toml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
[build-system]
2-
requires = ["setuptools", "wheel", "pybind11", "setuptools_scm"]
2+
requires = ["setuptools>=61.0", "wheel", "pybind11>=2.13", "setuptools_scm"]
33
build-backend = "setuptools.build_meta"
44

55
[tool.usort]
@@ -38,7 +38,7 @@ dependencies = [
3838
"cloudpickle",
3939
"packaging",
4040
"importlib_metadata",
41-
# orjson fails to be installed in python 3.13t
41+
# orjson fails to be installed in python 3.13
4242
'orjson ; python_version < "3.13"',
4343
]
4444

@@ -55,7 +55,7 @@ tests = [
5555
]
5656
checkpointing = ["torchsnapshot-nightly"]
5757
h5 = ["h5py>=3.8"]
58-
dev = ["pybind11", "cmake", "ninja"]
58+
dev = ["pybind11>=2.13", "cmake>=3.18", "ninja"]
5959

6060
[tool.setuptools]
6161
include-package-data = false

‎setup.py‎

Lines changed: 52 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,10 @@
99
import shutil
1010
import subprocess
1111
import sys
12+
import warnings
1213
from pathlib import Path
1314

14-
from setuptools import Extension, find_packages, setup
15+
from setuptools import Command, Extension, find_packages, setup
1516
from setuptools.command.build_ext import build_ext
1617

1718
ROOT_DIR = Path(__file__).parent.resolve()
@@ -28,11 +29,38 @@ def get_python_executable():
2829
return python_executable
2930

3031

31-
class clean(distutils.command.clean.clean):
32-
def run(self):
33-
# Run default behavior first
34-
distutils.command.clean.clean.run(self)
32+
def check_cmake_version():
33+
"""Check if CMake version is sufficient."""
34+
try:
35+
result = subprocess.run(
36+
["cmake", "--version"], capture_output=True, text=True, check=True
37+
)
38+
version_line = result.stdout.split("\n")[0]
39+
version_str = version_line.split()[2]
40+
major, minor = map(int, version_str.split(".")[:2])
41+
if major < 3 or (major == 3 and minor < 18):
42+
warnings.warn(
43+
f"CMake version {version_str} may be too old. Recommended: 3.18+"
44+
)
45+
return True
46+
except (subprocess.CalledProcessError, FileNotFoundError, ValueError):
47+
warnings.warn("Could not determine CMake version")
48+
return False
49+
50+
51+
class clean(Command):
52+
"""Custom clean command to remove tensordict extensions."""
53+
54+
description = "remove tensordict extensions and build files"
55+
user_options = []
56+
57+
def initialize_options(self):
58+
pass
59+
60+
def finalize_options(self):
61+
pass
3562

63+
def run(self):
3664
# Remove tensordict extension
3765
for path in (ROOT_DIR / "tensordict").glob("**/*.so"):
3866
logging.info(f"removing '{path}'")
@@ -53,6 +81,9 @@ def __init__(self, name, sourcedir=""):
5381

5482
class CMakeBuild(build_ext):
5583
def run(self):
84+
# Check CMake version before building
85+
check_cmake_version()
86+
5687
for ext in self.extensions:
5788
self.build_extension(ext)
5889

@@ -64,6 +95,7 @@ def build_extension(self, ext):
6495
else:
6596
# For regular installs, place the extension in the build directory
6697
extdir = os.path.abspath(os.path.join(self.build_lib, "tensordict"))
98+
6799
cmake_args = [
68100
f"-DCMAKE_LIBRARY_OUTPUT_DIRECTORY={extdir}",
69101
f"-DCMAKE_RUNTIME_OUTPUT_DIRECTORY={extdir}",
@@ -78,12 +110,21 @@ def build_extension(self, ext):
78110
os.makedirs(self.build_temp)
79111
if sys.platform == "win32":
80112
build_args += ["--config", "Release"]
81-
subprocess.check_call(
82-
["cmake", ext.sourcedir] + cmake_args, cwd=self.build_temp
83-
)
84-
subprocess.check_call(
85-
["cmake", "--build", ".", "--verbose"] + build_args, cwd=self.build_temp
86-
)
113+
114+
try:
115+
subprocess.check_call(
116+
["cmake", ext.sourcedir] + cmake_args, cwd=self.build_temp
117+
)
118+
subprocess.check_call(
119+
["cmake", "--build", ".", "--verbose"] + build_args, cwd=self.build_temp
120+
)
121+
except subprocess.CalledProcessError as e:
122+
warnings.warn(
123+
f"Error building extension: {e}\n"
124+
"This might be due to missing dependencies or incompatible compiler. "
125+
"Please ensure you have CMake 3.18+ and a C++17 compatible compiler."
126+
)
127+
raise
87128

88129

89130
def get_extensions():

‎tensordict/csrc/CMakeLists.txt‎

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,23 @@
1-
cmake_minimum_required(VERSION 3.22)
1+
cmake_minimum_required(VERSION 3.18)
22
project(tensordict)
33

4+
# Try C++20 first, fall back to C++17 if not available
45
set(CMAKE_CXX_STANDARD 20)
56
set(CMAKE_CXX_STANDARD_REQUIRED ON)
67

8+
# Check if C++20 is supported, fall back to C++17 if not
9+
include(CheckCXXCompilerFlag)
10+
check_cxx_compiler_flag("-std=c++20" CXX20_SUPPORTED)
11+
if(NOT CXX20_SUPPORTED)
12+
check_cxx_compiler_flag("-std=c++17" CXX17_SUPPORTED)
13+
if(CXX17_SUPPORTED)
14+
set(CMAKE_CXX_STANDARD 17)
15+
message(STATUS "C++20 not supported, falling back to C++17")
16+
else()
17+
message(FATAL_ERROR "Neither C++20 nor C++17 are supported by the compiler")
18+
endif()
19+
endif()
20+
721
# Set the Python executable to the one from your virtual environment
822
if(APPLE) # Check if the target OS is OSX/macOS
923
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake/")
@@ -52,7 +66,32 @@ if(CMAKE_BUILD_TYPE STREQUAL "Debug")
5266
else()
5367
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -O3")
5468
endif()
55-
set(CMAKE_OSX_DEPLOYMENT_TARGET "15.0" CACHE STRING "Minimum OS X deployment version")
69+
70+
# More flexible macOS deployment target
71+
if(APPLE)
72+
# Try to detect the minimum supported macOS version
73+
execute_process(
74+
COMMAND sw_vers -productVersion
75+
OUTPUT_VARIABLE MACOS_VERSION
76+
OUTPUT_STRIP_TRAILING_WHITESPACE
77+
)
78+
if(MACOS_VERSION)
79+
# Extract major version
80+
string(REGEX MATCH "^([0-9]+)" MACOS_MAJOR_VERSION "${MACOS_VERSION}")
81+
set(MACOS_MAJOR_VERSION "${CMAKE_MATCH_1}")
82+
# Use a reasonable minimum, but not too restrictive
83+
if(MACOS_MAJOR_VERSION GREATER_EQUAL 14)
84+
set(CMAKE_OSX_DEPLOYMENT_TARGET "14.0" CACHE STRING "Minimum OS X deployment version")
85+
else()
86+
set(CMAKE_OSX_DEPLOYMENT_TARGET "13.0" CACHE STRING "Minimum OS X deployment version")
87+
endif()
88+
else()
89+
# Fallback to a reasonable default
90+
set(CMAKE_OSX_DEPLOYMENT_TARGET "13.0" CACHE STRING "Minimum OS X deployment version")
91+
endif()
92+
message(STATUS "Setting macOS deployment target to ${CMAKE_OSX_DEPLOYMENT_TARGET}")
93+
endif()
94+
5695
set(CMAKE_VERBOSE_MAKEFILE ON)
5796

5897
if(WIN32)

‎tensordict/csrc/cmake/FindPythonPyEnv.cmake‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,9 @@ if (NOT DEFINED PYTHON_EXECUTABLE)
2929
)
3030
endif()
3131

32+
# Use sysconfig instead of distutils.sysconfig for Python 3.12+ compatibility
3233
execute_process(
33-
COMMAND ${PYTHON_EXECUTABLE} -c "from __future__ import print_function; from distutils.sysconfig import get_python_inc; print(get_python_inc())"
34+
COMMAND ${PYTHON_EXECUTABLE} -c "import sysconfig; print(sysconfig.get_path('include'))"
3435
OUTPUT_VARIABLE PYTHON_INCLUDE_DIR OUTPUT_STRIP_TRAILING_WHITESPACE ERROR_QUIET
3536
)
3637

@@ -39,22 +40,24 @@ if (NOT EXISTS ${PYTHON_INCLUDE_DIR})
3940
endif()
4041

4142
execute_process(
42-
COMMAND ${PYTHON_EXECUTABLE} -c "from __future__ import print_function; import os, numpy.distutils; print(os.pathsep.join(numpy.distutils.misc_util.get_numpy_include_dirs()))"
43+
COMMAND ${PYTHON_EXECUTABLE} -c "import os, numpy.distutils; print(os.pathsep.join(numpy.distutils.misc_util.get_numpy_include_dirs()))"
4344
OUTPUT_VARIABLE PYTHON_NUMPY_INCLUDE_DIR OUTPUT_STRIP_TRAILING_WHITESPACE ERROR_QUIET
4445
)
4546

47+
# Use sysconfig instead of distutils.sysconfig for Python 3.12+ compatibility
4648
execute_process(
47-
COMMAND ${PYTHON_EXECUTABLE} -c "from __future__ import print_function; import distutils.sysconfig as sysconfig; print('-L' + sysconfig.get_config_var('LIBDIR') + '/' + sysconfig.get_config_var('LDLIBRARY'))"
49+
COMMAND ${PYTHON_EXECUTABLE} -c "import sysconfig; print('-L' + sysconfig.get_config_var('LIBDIR') + '/' + sysconfig.get_config_var('LDLIBRARY'))"
4850
OUTPUT_VARIABLE PYTHON_LIBRARY OUTPUT_STRIP_TRAILING_WHITESPACE ERROR_QUIET
4951
)
5052

5153
execute_process(
52-
COMMAND ${PYTHON_EXECUTABLE} -c "from __future__ import print_function; import platform; print(platform.python_version())"
54+
COMMAND ${PYTHON_EXECUTABLE} -c "import platform; print(platform.python_version())"
5355
OUTPUT_VARIABLE PYTHONLIBS_VERSION_STRING OUTPUT_STRIP_TRAILING_WHITESPACE ERROR_QUIET
5456
)
5557

58+
# Use sysconfig instead of distutils.sysconfig for Python 3.12+ compatibility
5659
execute_process(
57-
COMMAND ${PYTHON_EXECUTABLE} -c "from __future__ import print_function; from distutils.sysconfig import get_python_lib; print(get_python_lib())"
60+
COMMAND ${PYTHON_EXECUTABLE} -c "import sysconfig; print(sysconfig.get_path('platlib'))"
5861
OUTPUT_VARIABLE PYTHON_SITE OUTPUT_STRIP_TRAILING_WHITESPACE ERROR_QUIET
5962
)
6063

0 commit comments

Comments
 (0)