From 18dfb02ba289dcfbd7633123bc7afce53837bd0a Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Mon, 10 Apr 2023 20:31:50 -0700 Subject: [PATCH 01/61] Initial setup of pyproject.toml --- python-package/pyproject.toml | 4 + python-package/xgboost/packager/__init__.py | 0 python-package/xgboost/packager/distinfo.py | 50 +++++++++++ python-package/xgboost/packager/nativelib.py | 21 +++++ python-package/xgboost/packager/pep517.py | 87 ++++++++++++++++++++ python-package/xgboost/packager/wheel.py | 33 ++++++++ 6 files changed, 195 insertions(+) create mode 100644 python-package/pyproject.toml create mode 100644 python-package/xgboost/packager/__init__.py create mode 100644 python-package/xgboost/packager/distinfo.py create mode 100644 python-package/xgboost/packager/nativelib.py create mode 100644 python-package/xgboost/packager/pep517.py create mode 100644 python-package/xgboost/packager/wheel.py diff --git a/python-package/pyproject.toml b/python-package/pyproject.toml new file mode 100644 index 000000000000..3e58857048d5 --- /dev/null +++ b/python-package/pyproject.toml @@ -0,0 +1,4 @@ +[build-system] +requires = ["packaging"] +backend-path = ["xgboost"] +build-backend = "packager.pep517" diff --git a/python-package/xgboost/packager/__init__.py b/python-package/xgboost/packager/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/python-package/xgboost/packager/distinfo.py b/python-package/xgboost/packager/distinfo.py new file mode 100644 index 000000000000..8c983da3c885 --- /dev/null +++ b/python-package/xgboost/packager/distinfo.py @@ -0,0 +1,50 @@ +import base64 +import csv +import email.message +import hashlib + + +def create_dist_info_dir(container, name, version): + print("create_dist_info_dir()") + dist_info = container / f"{name}-{version}.dist-info" + dist_info.mkdir() + return dist_info + + +def write_metadata(dist_info, name, version): + print("write_metadata()") + m = email.message.EmailMessage() # RFC 822. + m["Metadata-Version"] = "2.1" + m["Name"] = name + m["Version"] = version + dist_info.joinpath("METADATA").write_bytes(bytes(m)) + + +def _record_row_from_path(path, relative): + file_data = path.read_bytes() + file_size = len(file_data) + file_hash = ( + base64.urlsafe_b64encode(hashlib.sha256(file_data).digest()) + .decode("latin1") + .rstrip("=") + ) + return [relative.as_posix(), f"sha256={file_hash}", str(file_size)] + + +def iter_files(roots): + for root in roots: + for path in root.glob("**/*"): + if not path.is_file(): + continue + if path.suffix == ".pyc" or path.parent.name == "__pycache__": + continue + yield path, path.relative_to(root.parent) + + +def write_record(dist_info, package): + print("write_record()") + with dist_info.joinpath("RECORD").open("w") as f: + w = csv.writer(f, lineterminator="\n") + for path, relative in iter_files((package, dist_info)): + w.writerow(_record_row_from_path(path, relative)) + w.writerow([f"{dist_info.name}/RECORD", "", ""]) diff --git a/python-package/xgboost/packager/nativelib.py b/python-package/xgboost/packager/nativelib.py new file mode 100644 index 000000000000..e8411d6355a9 --- /dev/null +++ b/python-package/xgboost/packager/nativelib.py @@ -0,0 +1,21 @@ +from platform import system + + +def _lib_name() -> str: + """Return platform dependent shared object name.""" + if system() in ["Linux", "OS400"] or system().upper().endswith("BSD"): + name = "libxgboost.so" + elif system() == "Darwin": + name = "libxgboost.dylib" + elif system() == "Windows": + name = "xgboost.dll" + else: + raise NotImplementedError(f"System {system()} not supported") + return name + + +def locate_libxgboost(toplevel_dir): + libxgboost = toplevel_dir.parent / "lib" / _lib_name() + if libxgboost.exists(): + return libxgboost + raise NotImplementedError(f"Please build native lib first using CMake") diff --git a/python-package/xgboost/packager/pep517.py b/python-package/xgboost/packager/pep517.py new file mode 100644 index 000000000000..30cd82d4c50d --- /dev/null +++ b/python-package/xgboost/packager/pep517.py @@ -0,0 +1,87 @@ +""" +Custom build backend for XGBoost Python package. +Builds source distribution and binary wheels +Follows PEP 517 +""" +import pathlib +import sysconfig +import tarfile +import tempfile + +import packaging.version + +from .distinfo import iter_files +from .nativelib import locate_libxgboost +from .wheel import create_dist_info, create_wheel + + +def get_tag(): + tag_platform = sysconfig.get_platform().replace("-", "_").replace(".", "_") + return f"py3-none-{tag_platform}" + + +def get_version(toplevel_dir): + version = ( + open(toplevel_dir / "xgboost" / "VERSION", "r", encoding="utf-8").read().strip() + ) + return str(packaging.version.Version(version)) + + +TOPLEVEL_DIR = pathlib.Path(__file__).parent.parent.parent.absolute().resolve() +NAME = "xgboost" +VERSION = get_version(TOPLEVEL_DIR) +TAG = get_tag() +PACKAGE = pathlib.Path(TOPLEVEL_DIR / "xgboost") + + +def build_wheel( + wheel_directory, + config_settings=None, + metadata_directory=None, +): + print("build_wheel()") + with tempfile.TemporaryDirectory() as td: + if config_settings is not None: + raise NotImplementedError( + f"XGBoost's custom build backend doesn't support config_settings option" + ) + if metadata_directory is None: + td_path = pathlib.Path(td) + dist_info = create_dist_info( + NAME, + VERSION, + TAG, + PACKAGE, + td_path, + ) + else: + raise NotImplementedError( + f"XGBoost's custom build backend doesn't support metadata_directory option" + ) + + wheel_path = create_wheel( + NAME, + VERSION, + TAG, + PACKAGE, + dist_info=dist_info, + libxgboost=locate_libxgboost(TOPLEVEL_DIR), + output_dir=pathlib.Path(wheel_directory).absolute().resolve(), + ) + + return wheel_path.name + + +def build_sdist(sdist_directory, config_settings=None): + if config_settings is not None: + raise NotImplementedError( + f"XGBoost's custom build backend doesn't support config_settings option" + ) + packager_dir = pathlib.Path(__file__).absolute().resolve().parent + sdist_path = pathlib.Path(sdist_directory, f"{NAME}-{VERSION}.tar.gz") + with tarfile.open(sdist_path, "w:gz", format=tarfile.PAX_FORMAT) as tf: + for path, relative in iter_files((PACKAGE, packager_dir)): + tf.add(path, relative.as_posix()) + pyproject_path = TOPLEVEL_DIR / "pyproject.toml" + tf.add(pyproject_path, pyproject_path.relative_to(packager_dir.parent)) + return sdist_path.name diff --git a/python-package/xgboost/packager/wheel.py b/python-package/xgboost/packager/wheel.py new file mode 100644 index 000000000000..c94379898ceb --- /dev/null +++ b/python-package/xgboost/packager/wheel.py @@ -0,0 +1,33 @@ +import email.message +import zipfile + +from .distinfo import create_dist_info_dir, iter_files, write_metadata, write_record + + +def write_wheel_metadata(dist_info, tag): + print("write_wheel_metadata()") + m = email.message.EmailMessage() + m["Wheel-Version"] = "1.0" + m["Generator"] = "packager/wheel.py" + m["Root-Is-Purelib"] = "true" + m["Tag"] = tag + dist_info.joinpath("WHEEL").write_bytes(bytes(m)) + + +def create_dist_info(name, version, tag, package, output_dir): + print("create_dist_info()") + dist_info = create_dist_info_dir(output_dir, name, version) + write_metadata(dist_info, name, version) + write_wheel_metadata(dist_info, tag) + write_record(dist_info, package) + return dist_info + + +def create_wheel(name, version, tag, package, *, dist_info, libxgboost, output_dir): + print("create_wheel()") + wheel_path = output_dir / f"{name}-{version}-{tag}.whl" + with zipfile.ZipFile(wheel_path, "w") as zf: + for path, relative in iter_files((package, dist_info)): + zf.write(path, relative.as_posix()) + zf.write(libxgboost, f"xgboost/lib/{libxgboost.name}") + return wheel_path From 2469e986f38846293d04f1993d93812a553edffa Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Mon, 10 Apr 2023 20:39:11 -0700 Subject: [PATCH 02/61] Fix path in sdist --- python-package/xgboost/packager/pep517.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/python-package/xgboost/packager/pep517.py b/python-package/xgboost/packager/pep517.py index 30cd82d4c50d..eceb008850c4 100644 --- a/python-package/xgboost/packager/pep517.py +++ b/python-package/xgboost/packager/pep517.py @@ -73,15 +73,15 @@ def build_wheel( def build_sdist(sdist_directory, config_settings=None): - if config_settings is not None: + if config_settings: raise NotImplementedError( - f"XGBoost's custom build backend doesn't support config_settings option" + f"XGBoost's custom build backend doesn't support config_settings option." + f"{config_settings=}" ) - packager_dir = pathlib.Path(__file__).absolute().resolve().parent sdist_path = pathlib.Path(sdist_directory, f"{NAME}-{VERSION}.tar.gz") with tarfile.open(sdist_path, "w:gz", format=tarfile.PAX_FORMAT) as tf: - for path, relative in iter_files((PACKAGE, packager_dir)): + for path, relative in iter_files((PACKAGE,)): tf.add(path, relative.as_posix()) pyproject_path = TOPLEVEL_DIR / "pyproject.toml" - tf.add(pyproject_path, pyproject_path.relative_to(packager_dir.parent)) + tf.add(pyproject_path, pyproject_path.relative_to(TOPLEVEL_DIR)) return sdist_path.name From 7037c80d8077bcca515552dd999d3be99ae1f2f2 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Mon, 10 Apr 2023 23:04:08 -0700 Subject: [PATCH 03/61] Full support for sdist --- CMakeLists.txt | 10 +++- python-package/xgboost/packager/nativelib.py | 51 +++++++++++++++++++- python-package/xgboost/packager/pep517.py | 25 +++++++--- python-package/xgboost/packager/sdist.py | 19 ++++++++ 4 files changed, 94 insertions(+), 11 deletions(-) create mode 100644 python-package/xgboost/packager/sdist.py diff --git a/CMakeLists.txt b/CMakeLists.txt index e5a61c60b082..7953a10dd990 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -47,6 +47,7 @@ option(USE_NVTX "Build with cuda profiling annotations. Developers only." OFF) set(NVTX_HEADER_DIR "" CACHE PATH "Path to the stand-alone nvtx header") option(RABIT_MOCK "Build rabit with mock" OFF) option(HIDE_CXX_SYMBOLS "Build shared library and hide all C++ symbols" OFF) +option(KEEP_BUILD_ARTIFACTS_IN_BINARY_DIR "Output build artifacts in CMake binary dir" OFF) ## CUDA option(USE_CUDA "Build with GPU acceleration" OFF) option(USE_NCCL "Build with NCCL to enable distributed GPU support." OFF) @@ -268,8 +269,13 @@ if (JVM_BINDINGS) xgboost_target_defs(xgboost4j) endif (JVM_BINDINGS) -set_output_directory(runxgboost ${xgboost_SOURCE_DIR}) -set_output_directory(xgboost ${xgboost_SOURCE_DIR}/lib) +if (KEEP_BUILD_ARTIFACTS_IN_BINARY_DIR) + set_output_directory(runxgboost ${xgboost_BINARY_DIR}) + set_output_directory(xgboost ${xgboost_BINARY_DIR}/lib) +else () + set_output_directory(runxgboost ${xgboost_SOURCE_DIR}) + set_output_directory(xgboost ${xgboost_SOURCE_DIR}/lib) +endif () # Ensure these two targets do not build simultaneously, as they produce outputs with conflicting names add_dependencies(xgboost runxgboost) diff --git a/python-package/xgboost/packager/nativelib.py b/python-package/xgboost/packager/nativelib.py index e8411d6355a9..f631aa911f1e 100644 --- a/python-package/xgboost/packager/nativelib.py +++ b/python-package/xgboost/packager/nativelib.py @@ -1,3 +1,6 @@ +import os +import shutil +import subprocess from platform import system @@ -14,8 +17,52 @@ def _lib_name() -> str: return name -def locate_libxgboost(toplevel_dir): +def build_libxgboost(cpp_src_dir, *, tempdir): + if not cpp_src_dir.is_dir(): + raise RuntimeError(f"Expected {cpp_src_dir} to be a directory") + print(f"Building {_lib_name()} from the C++ source files in {cpp_src_dir}...") + build_dir = tempdir / "build" + build_dir.mkdir(exist_ok=True) + + if shutil.which("ninja"): + build_tool = "ninja" + else: + build_tool = "make" + + if system() == "Windows": + raise NotImplementedError( + f"Installing from sdist is not supported on Windows. You have two alternatives:\n" + "1. Install XGBoost from the official wheel (recommended): pip install xgboost\n" + "2. Build XGBoost from the source by running CMake at the project root folder. See " + "documentation for details." + ) + + generator = "-GNinja" if build_tool == "ninja" else "-GUnix Makefiles" + cmake_cmd = ["cmake", cpp_src_dir, generator] + cmake_cmd.append("-DKEEP_BUILD_ARTIFACTS_IN_BINARY_DIR=ON") + + # TODO(hcho3): handle CMake args + print(f"{cmake_cmd=}") + subprocess.check_call(cmake_cmd, cwd=build_dir) + + nproc = os.cpu_count() + assert build_tool is not None + subprocess.check_call([build_tool, f"-j{nproc}"], cwd=build_dir) + + return build_dir / "lib" / _lib_name() + + +def locate_or_build_libxgboost(toplevel_dir, *, tempdir): libxgboost = toplevel_dir.parent / "lib" / _lib_name() if libxgboost.exists(): + print(f"Found {libxgboost.name}") return libxgboost - raise NotImplementedError(f"Please build native lib first using CMake") + if toplevel_dir.joinpath("cpp_src").exists(): + # Source distribution; all C++ source files to be found in cpp_src/ + cpp_src_dir = toplevel_dir.joinpath("cpp_src") + else: + raise RuntimeError( + "Before building a binary wheel, please build XGBoost shared library using CMake. " + f"The setup script will expect to see {_lib_name()} at {libxgboost}" + ) + return build_libxgboost(cpp_src_dir, tempdir=tempdir) diff --git a/python-package/xgboost/packager/pep517.py b/python-package/xgboost/packager/pep517.py index eceb008850c4..7dee26d2d056 100644 --- a/python-package/xgboost/packager/pep517.py +++ b/python-package/xgboost/packager/pep517.py @@ -11,7 +11,8 @@ import packaging.version from .distinfo import iter_files -from .nativelib import locate_libxgboost +from .nativelib import locate_or_build_libxgboost +from .sdist import copy_cpp_src_tree from .wheel import create_dist_info, create_wheel @@ -65,7 +66,7 @@ def build_wheel( TAG, PACKAGE, dist_info=dist_info, - libxgboost=locate_libxgboost(TOPLEVEL_DIR), + libxgboost=locate_or_build_libxgboost(TOPLEVEL_DIR, tempdir=td_path), output_dir=pathlib.Path(wheel_directory).absolute().resolve(), ) @@ -78,10 +79,20 @@ def build_sdist(sdist_directory, config_settings=None): f"XGBoost's custom build backend doesn't support config_settings option." f"{config_settings=}" ) + + cpp_src_dir = TOPLEVEL_DIR.parent + if not cpp_src_dir.joinpath("CMakeLists.txt").exists(): + raise RuntimeError(f"Did not find CMakeLists.txt from {cpp_src_dir}") + sdist_path = pathlib.Path(sdist_directory, f"{NAME}-{VERSION}.tar.gz") - with tarfile.open(sdist_path, "w:gz", format=tarfile.PAX_FORMAT) as tf: - for path, relative in iter_files((PACKAGE,)): - tf.add(path, relative.as_posix()) - pyproject_path = TOPLEVEL_DIR / "pyproject.toml" - tf.add(pyproject_path, pyproject_path.relative_to(TOPLEVEL_DIR)) + with tempfile.TemporaryDirectory() as td: + temp_cpp_src_dir = pathlib.Path(td) / "cpp_src" + copy_cpp_src_tree(cpp_src_dir, temp_cpp_src_dir) + with tarfile.open(sdist_path, "w:gz", format=tarfile.PAX_FORMAT) as tf: + for path, relative in iter_files((PACKAGE,)): + tf.add(path, relative.as_posix()) + for path, relative in iter_files((temp_cpp_src_dir,)): + tf.add(path, relative.as_posix()) + pyproject_path = TOPLEVEL_DIR / "pyproject.toml" + tf.add(pyproject_path, pyproject_path.relative_to(TOPLEVEL_DIR)) return sdist_path.name diff --git a/python-package/xgboost/packager/sdist.py b/python-package/xgboost/packager/sdist.py new file mode 100644 index 000000000000..31e3b185d5f3 --- /dev/null +++ b/python-package/xgboost/packager/sdist.py @@ -0,0 +1,19 @@ +import shutil + + +def copy_cpp_src_tree(cpp_src_dir, target_dir): + """Copy C++ source tree into build directory""" + + for subdir in [ + "src", + "include", + "dmlc-core", + "gputreeshap", + "rabit", + "cmake", + "plugin", + ]: + shutil.copytree(cpp_src_dir.joinpath(subdir), target_dir.joinpath(subdir)) + + for filename in ["CMakeLists.txt", "LICENSE"]: + shutil.copy(cpp_src_dir.joinpath(filename), target_dir) From 662ab40ae5d5f54afad2587327d06d94f5f21f4e Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Mon, 10 Apr 2023 23:19:36 -0700 Subject: [PATCH 04/61] Remove setup.py --- python-package/pyproject.toml | 34 +++ python-package/setup.py | 399 ---------------------------------- 2 files changed, 34 insertions(+), 399 deletions(-) delete mode 100644 python-package/setup.py diff --git a/python-package/pyproject.toml b/python-package/pyproject.toml index 3e58857048d5..970d05a9ac11 100644 --- a/python-package/pyproject.toml +++ b/python-package/pyproject.toml @@ -2,3 +2,37 @@ requires = ["packaging"] backend-path = ["xgboost"] build-backend = "packager.pep517" + +[project] +name = "xgboost" +version = "2.0.0-dev0" +authors = [ + {name = "Hyunsu Cho", email = "chohyu01@cs.washington.edu"}, + {name = "Jiaming Yuan", email = "jm.yuan@outlook.com"} +] +description = "XGBoost Python Package" +readme = {file = "README.rst", content-type = "text/x-rst"} +requires-python = ">=3.8" +license = {text = "Apache-2.0"} +classifiers = [ + "License :: OSI Approved :: Apache Software License", + "Development Status :: 5 - Production/Stable", + "Operating System :: OS Independent", + "Programming Language :: Python", + "Programming Language :: Python :: 3", + "Programming Language :: Python :: 3.8", + "Programming Language :: Python :: 3.9", + "Programming Language :: Python :: 3.10" +] +dependencies = [ + "numpy", + "scipy" +] + +[project.optional-dependencies] +pandas = ["pandas"], +scikit-learn = ["scikit-learn"], +dask = ["dask", "pandas", "distributed"], +datatable = ["datatable"], +plotting = ["graphviz", "matplotlib"], +pyspark = ["pyspark", "scikit-learn", "cloudpickle"] diff --git a/python-package/setup.py b/python-package/setup.py deleted file mode 100644 index fe1cbf2e9c19..000000000000 --- a/python-package/setup.py +++ /dev/null @@ -1,399 +0,0 @@ -"""Setup xgboost package.""" -import logging -import os -import shutil -import subprocess -import sys -from platform import system -from typing import List, Optional - -from setuptools import Extension, find_packages, setup -from setuptools.command import build_ext, install, install_lib, sdist - -# You can't use `pip install .` as pip copies setup.py to a temporary -# directory, parent directory is no longer reachable (isolated build) . -CURRENT_DIR = os.path.abspath(os.path.dirname(__file__)) -sys.path.insert(0, CURRENT_DIR) - -# Options only effect `python setup.py install`, building `bdist_wheel` -# requires using CMake directly. -USER_OPTIONS = { - # libxgboost options. - "use-openmp": (None, "Build with OpenMP support.", 1), - "use-cuda": (None, "Build with GPU acceleration.", 0), - "use-nccl": (None, "Build with NCCL to enable distributed GPU support.", 0), - "build-with-shared-nccl": (None, "Build with shared NCCL library.", 0), - "hide-cxx-symbols": (None, "Hide all C++ symbols during build.", 1), - "use-hdfs": (None, "Build with HDFS support", 0), - "use-azure": (None, "Build with AZURE support.", 0), - "use-s3": (None, "Build with S3 support", 0), - "plugin-dense-parser": (None, "Build dense parser plugin.", 0), - # Python specific - "use-system-libxgboost": (None, "Use libxgboost.so in system path.", 0), -} - -NEED_CLEAN_TREE = set() -NEED_CLEAN_FILE = set() -BUILD_TEMP_DIR = None - - -def lib_name() -> str: - """Return platform dependent shared object name.""" - if system() == "Linux" or system().upper().endswith("BSD"): - name = "libxgboost.so" - elif system() == "Darwin": - name = "libxgboost.dylib" - elif system() == "Windows": - name = "xgboost.dll" - elif system() == "OS400": - name = "libxgboost.so" - return name - - -def copy_tree(src_dir: str, target_dir: str) -> None: - """Copy source tree into build directory.""" - - def clean_copy_tree(src: str, dst: str) -> None: - shutil.copytree(src, dst) - NEED_CLEAN_TREE.add(os.path.abspath(dst)) - - def clean_copy_file(src: str, dst: str) -> None: - shutil.copy(src, dst) - NEED_CLEAN_FILE.add(os.path.abspath(dst)) - - src = os.path.join(src_dir, "src") - inc = os.path.join(src_dir, "include") - dmlc_core = os.path.join(src_dir, "dmlc-core") - gputreeshap = os.path.join(src_dir, "gputreeshap") - rabit = os.path.join(src_dir, "rabit") - cmake = os.path.join(src_dir, "cmake") - plugin = os.path.join(src_dir, "plugin") - - clean_copy_tree(src, os.path.join(target_dir, "src")) - clean_copy_tree(inc, os.path.join(target_dir, "include")) - clean_copy_tree(dmlc_core, os.path.join(target_dir, "dmlc-core")) - clean_copy_tree(gputreeshap, os.path.join(target_dir, "gputreeshap")) - clean_copy_tree(rabit, os.path.join(target_dir, "rabit")) - clean_copy_tree(cmake, os.path.join(target_dir, "cmake")) - clean_copy_tree(plugin, os.path.join(target_dir, "plugin")) - - cmake_list = os.path.join(src_dir, "CMakeLists.txt") - clean_copy_file(cmake_list, os.path.join(target_dir, "CMakeLists.txt")) - lic = os.path.join(src_dir, "LICENSE") - clean_copy_file(lic, os.path.join(target_dir, "LICENSE")) - - -def clean_up() -> None: - """Removed copied files.""" - for path in NEED_CLEAN_TREE: - shutil.rmtree(path) - for path in NEED_CLEAN_FILE: - os.remove(path) - - -class CMakeExtension(Extension): # pylint: disable=too-few-public-methods - """Wrapper for extension""" - - def __init__(self, name: str) -> None: - super().__init__(name=name, sources=[]) - - -class BuildExt(build_ext.build_ext): # pylint: disable=too-many-ancestors - """Custom build_ext command using CMake.""" - - logger = logging.getLogger("XGBoost build_ext") - - # pylint: disable=too-many-arguments - def build( - self, - src_dir: str, - build_dir: str, - generator: str, - build_tool: Optional[str] = None, - use_omp: int = 1, - ) -> None: - """Build the core library with CMake.""" - cmake_cmd = ["cmake", src_dir, generator] - - for k, v in USER_OPTIONS.items(): - arg = k.replace("-", "_").upper() - value = str(v[2]) - if arg == "USE_SYSTEM_LIBXGBOOST": - continue - if arg == "USE_OPENMP" and use_omp == 0: - cmake_cmd.append("-D" + arg + "=0") - continue - cmake_cmd.append("-D" + arg + "=" + value) - - # Flag for cross-compiling for Apple Silicon - # We use environment variable because it's the only way to pass down custom flags - # through the cibuildwheel package, which otherwise calls `python setup.py bdist_wheel` - # command. - if "CIBW_TARGET_OSX_ARM64" in os.environ: - cmake_cmd.append("-DCMAKE_OSX_ARCHITECTURES=arm64") - - self.logger.info("Run CMake command: %s", str(cmake_cmd)) - subprocess.check_call(cmake_cmd, cwd=build_dir) - - if system() != "Windows": - nproc = os.cpu_count() - assert build_tool is not None - subprocess.check_call([build_tool, "-j" + str(nproc)], cwd=build_dir) - else: - subprocess.check_call( - ["cmake", "--build", ".", "--config", "Release"], cwd=build_dir - ) - - def build_cmake_extension(self) -> None: - """Configure and build using CMake""" - if USER_OPTIONS["use-system-libxgboost"][2]: - self.logger.info("Using system libxgboost.") - return - - build_dir = self.build_temp - global BUILD_TEMP_DIR # pylint: disable=global-statement - BUILD_TEMP_DIR = build_dir - libxgboost = os.path.abspath( - os.path.join(CURRENT_DIR, os.path.pardir, "lib", lib_name()) - ) - - if os.path.exists(libxgboost): - self.logger.info("Found shared library, skipping build.") - return - - src_dir = "xgboost" - try: - copy_tree( - os.path.join(CURRENT_DIR, os.path.pardir), - os.path.join(self.build_temp, src_dir), - ) - except Exception: # pylint: disable=broad-except - copy_tree(src_dir, os.path.join(self.build_temp, src_dir)) - - self.logger.info("Building from source. %s", libxgboost) - if not os.path.exists(build_dir): - os.mkdir(build_dir) - if shutil.which("ninja"): - build_tool = "ninja" - else: - build_tool = "make" - if sys.platform.startswith("os400"): - build_tool = "make" - - if system() == "Windows": - # Pick up from LGB, just test every possible tool chain. - for vs in ( - "-GVisual Studio 17 2022", - "-GVisual Studio 16 2019", - "-GVisual Studio 15 2017", - "-GVisual Studio 14 2015", - "-GMinGW Makefiles", - ): - try: - self.build(src_dir, build_dir, vs) - self.logger.info( - "%s is used for building Windows distribution.", vs - ) - break - except subprocess.CalledProcessError: - shutil.rmtree(build_dir) - os.mkdir(build_dir) - continue - else: - gen = "-GNinja" if build_tool == "ninja" else "-GUnix Makefiles" - try: - self.build(src_dir, build_dir, gen, build_tool, use_omp=1) - except subprocess.CalledProcessError: - self.logger.warning("Disabling OpenMP support.") - self.build(src_dir, build_dir, gen, build_tool, use_omp=0) - - def build_extension(self, ext: Extension) -> None: - """Override the method for dispatching.""" - if isinstance(ext, CMakeExtension): - self.build_cmake_extension() - else: - super().build_extension(ext) - - def copy_extensions_to_source(self) -> None: - """Dummy override. Invoked during editable installation. Our binary - should available in `lib`. - - """ - if not os.path.exists( - os.path.join(CURRENT_DIR, os.path.pardir, "lib", lib_name()) - ): - raise ValueError( - "For using editable installation, please " - + "build the shared object first with CMake." - ) - - -class Sdist(sdist.sdist): # pylint: disable=too-many-ancestors - """Copy c++ source into Python directory.""" - - logger = logging.getLogger("xgboost sdist") - - def run(self) -> None: - copy_tree( - os.path.join(CURRENT_DIR, os.path.pardir), - os.path.join(CURRENT_DIR, "xgboost"), - ) - libxgboost = os.path.join(CURRENT_DIR, os.path.pardir, "lib", lib_name()) - if os.path.exists(libxgboost): - self.logger.warning( - "Found shared library, removing to avoid being included in source distribution." - ) - os.remove(libxgboost) - super().run() - - -class InstallLib(install_lib.install_lib): - """Copy shared object into installation directory.""" - - logger = logging.getLogger("xgboost install_lib") - - def install(self) -> List[str]: - outfiles = super().install() - - if USER_OPTIONS["use-system-libxgboost"][2] != 0: - self.logger.info("Using system libxgboost.") - lib_path = os.path.join(sys.prefix, "lib") - msg = ( - "use-system-libxgboost is specified, but " - + lib_name() - + " is not found in: " - + lib_path - ) - assert os.path.exists(os.path.join(lib_path, lib_name())), msg - return [] - - lib_dir = os.path.join(self.install_dir, "xgboost", "lib") - if not os.path.exists(lib_dir): - os.mkdir(lib_dir) - dst = os.path.join(self.install_dir, "xgboost", "lib", lib_name()) - - libxgboost_path = lib_name() - - assert BUILD_TEMP_DIR is not None - dft_lib_dir = os.path.join(CURRENT_DIR, os.path.pardir, "lib") - build_dir = os.path.join(BUILD_TEMP_DIR, "xgboost", "lib") - - if os.path.exists(os.path.join(dft_lib_dir, libxgboost_path)): - # The library is built by CMake directly - src = os.path.join(dft_lib_dir, libxgboost_path) - else: - # The library is built by setup.py - src = os.path.join(build_dir, libxgboost_path) - self.logger.info("Installing shared library: %s", src) - dst, _ = self.copy_file(src, dst) - outfiles.append(dst) - return outfiles - - -class Install(install.install): # pylint: disable=too-many-instance-attributes - """An interface to install command, accepting XGBoost specific - arguments. - - """ - - user_options = install.install.user_options + [ - (k, v[0], v[1]) for k, v in USER_OPTIONS.items() - ] - - def initialize_options(self) -> None: - super().initialize_options() - self.use_openmp = 1 - self.use_cuda = 0 - self.use_nccl = 0 - self.build_with_shared_nccl = 0 - self.hide_cxx_symbols = 1 - - self.use_hdfs = 0 - self.use_azure = 0 - self.use_s3 = 0 - - self.plugin_dense_parser = 0 - - self.use_system_libxgboost = 0 - - def run(self) -> None: - # setuptools will configure the options according to user supplied command line - # arguments, then here we propagate them into `USER_OPTIONS` for visibility to - # other sub-commands like `build_ext`. - for k, v in USER_OPTIONS.items(): - arg = k.replace("-", "_") - if hasattr(self, arg): - USER_OPTIONS[k] = (v[0], v[1], getattr(self, arg)) - super().run() - - -if __name__ == "__main__": - # Supported commands: - # From internet: - # - pip install xgboost - # - pip install --no-binary :all: xgboost - - # From source tree `xgboost/python-package`: - # - python setup.py build - # - python setup.py build_ext - # - python setup.py install - # - python setup.py sdist && pip install - # - python setup.py bdist_wheel && pip install - - # When XGBoost is compiled directly with CMake: - # - pip install -e . - # - python setup.py develop # same as above - logging.basicConfig(level=logging.INFO) - - with open(os.path.join(CURRENT_DIR, "README.rst"), encoding="utf-8") as fd: - description = fd.read() - with open(os.path.join(CURRENT_DIR, "xgboost/VERSION"), encoding="ascii") as fd: - version = fd.read().strip() - - setup( - name="xgboost", - version=version, - description="XGBoost Python Package", - long_description=description, - long_description_content_type="text/x-rst", - install_requires=[ - "numpy", - "scipy", - ], - ext_modules=[CMakeExtension("libxgboost")], - # error: expected "str": "Type[Command]" - cmdclass={ - "build_ext": BuildExt, # type: ignore - "sdist": Sdist, # type: ignore - "install_lib": InstallLib, # type: ignore - "install": Install, # type: ignore - }, - extras_require={ - "pandas": ["pandas"], - "scikit-learn": ["scikit-learn"], - "dask": ["dask", "pandas", "distributed"], - "datatable": ["datatable"], - "plotting": ["graphviz", "matplotlib"], - "pyspark": ["pyspark", "scikit-learn", "cloudpickle"], - }, - maintainer="Hyunsu Cho", - maintainer_email="chohyu01@cs.washington.edu", - zip_safe=False, - packages=find_packages(), - include_package_data=True, - license="Apache-2.0", - classifiers=[ - "License :: OSI Approved :: Apache Software License", - "Development Status :: 5 - Production/Stable", - "Operating System :: OS Independent", - "Programming Language :: Python", - "Programming Language :: Python :: 3", - "Programming Language :: Python :: 3.8", - "Programming Language :: Python :: 3.9", - "Programming Language :: Python :: 3.10", - ], - python_requires=">=3.8", - url="https://github.com/dmlc/xgboost", - ) - - clean_up() From b3490dbca8221654756266edd2cd7f7d366c4ec2 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Mon, 10 Apr 2023 23:25:51 -0700 Subject: [PATCH 05/61] Fix --- python-package/pyproject.toml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/python-package/pyproject.toml b/python-package/pyproject.toml index 970d05a9ac11..717e796ebcc6 100644 --- a/python-package/pyproject.toml +++ b/python-package/pyproject.toml @@ -30,9 +30,9 @@ dependencies = [ ] [project.optional-dependencies] -pandas = ["pandas"], -scikit-learn = ["scikit-learn"], -dask = ["dask", "pandas", "distributed"], -datatable = ["datatable"], -plotting = ["graphviz", "matplotlib"], +pandas = ["pandas"] +scikit-learn = ["scikit-learn"] +dask = ["dask", "pandas", "distributed"] +datatable = ["datatable"] +plotting = ["graphviz", "matplotlib"] pyspark = ["pyspark", "scikit-learn", "cloudpickle"] From bae6d9bb72dcec581e36873350b9c91edc8efac4 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 15:42:53 -0700 Subject: [PATCH 06/61] Use hatchling backend to reduce boilerplate --- python-package/MANIFEST.in | 56 -------- .../{xgboost => }/packager/__init__.py | 0 .../{xgboost => }/packager/nativelib.py | 9 +- python-package/packager/pep517.py | 132 ++++++++++++++++++ .../{xgboost => }/packager/sdist.py | 0 python-package/pyproject.toml | 9 +- python-package/xgboost/packager/distinfo.py | 50 ------- python-package/xgboost/packager/pep517.py | 98 ------------- python-package/xgboost/packager/wheel.py | 33 ----- 9 files changed, 143 insertions(+), 244 deletions(-) delete mode 100644 python-package/MANIFEST.in rename python-package/{xgboost => }/packager/__init__.py (100%) rename python-package/{xgboost => }/packager/nativelib.py (90%) create mode 100644 python-package/packager/pep517.py rename python-package/{xgboost => }/packager/sdist.py (100%) delete mode 100644 python-package/xgboost/packager/distinfo.py delete mode 100644 python-package/xgboost/packager/pep517.py delete mode 100644 python-package/xgboost/packager/wheel.py diff --git a/python-package/MANIFEST.in b/python-package/MANIFEST.in deleted file mode 100644 index 23f2684c2ac4..000000000000 --- a/python-package/MANIFEST.in +++ /dev/null @@ -1,56 +0,0 @@ -include README.rst -include xgboost/LICENSE -include xgboost/VERSION -include xgboost/CMakeLists.txt - -include xgboost/py.typed -recursive-include xgboost *.py -recursive-include xgboost/cmake * -exclude xgboost/cmake/RPackageInstall.cmake.in -exclude xgboost/cmake/RPackageInstallTargetSetup.cmake -exclude xgboost/cmake/Sanitizer.cmake -exclude xgboost/cmake/modules/FindASan.cmake -exclude xgboost/cmake/modules/FindLSan.cmake -exclude xgboost/cmake/modules/FindLibR.cmake -exclude xgboost/cmake/modules/FindTSan.cmake -exclude xgboost/cmake/modules/FindUBSan.cmake -recursive-include xgboost/include * -recursive-include xgboost/plugin * -recursive-include xgboost/src * - -recursive-include xgboost/gputreeshap/GPUTreeShap * - -include xgboost/rabit/CMakeLists.txt -recursive-include xgboost/rabit/include * -recursive-include xgboost/rabit/src * -prune xgboost/rabit/doc -prune xgboost/rabit/guide - -include xgboost/dmlc-core/CMakeLists.txt - -recursive-include xgboost/dmlc-core/cmake * -exclude xgboost/dmlc-core/cmake/gtest_cmake.in -exclude xgboost/dmlc-core/cmake/lint.cmake -exclude xgboost/dmlc-core/cmake/Sanitizer.cmake -exclude xgboost/dmlc-core/cmake/Modules/FindASan.cmake -exclude xgboost/dmlc-core/cmake/Modules/FindLSan.cmake -exclude xgboost/dmlc-core/cmake/Modules/FindTSan.cmake -exclude xgboost/dmlc-core/cmake/Modules/FindUBSan.cmake - -recursive-include xgboost/dmlc-core/include * -recursive-include xgboost/dmlc-core/include * -recursive-include xgboost/dmlc-core/make * -recursive-include xgboost/dmlc-core/src * -include xgboost/dmlc-core/tracker/dmlc-submit -recursive-include xgboost/dmlc-core/tracker/dmlc_tracker *.py -include xgboost/dmlc-core/tracker/yarn/build.bat -include xgboost/dmlc-core/tracker/yarn/build.sh -include xgboost/dmlc-core/tracker/yarn/pom.xml -recursive-include xgboost/dmlc-core/tracker/yarn/src * -include xgboost/dmlc-core/windows/dmlc.sln -include xgboost/dmlc-core/windows/dmlc/dmlc.vcxproj - -prune xgboost/dmlc-core/doc -prune xgboost/dmlc-core/scripts/ - -global-exclude *.py[oc] diff --git a/python-package/xgboost/packager/__init__.py b/python-package/packager/__init__.py similarity index 100% rename from python-package/xgboost/packager/__init__.py rename to python-package/packager/__init__.py diff --git a/python-package/xgboost/packager/nativelib.py b/python-package/packager/nativelib.py similarity index 90% rename from python-package/xgboost/packager/nativelib.py rename to python-package/packager/nativelib.py index f631aa911f1e..6bf490dc1c21 100644 --- a/python-package/xgboost/packager/nativelib.py +++ b/python-package/packager/nativelib.py @@ -1,6 +1,7 @@ import os import shutil import subprocess +import tempfile from platform import system @@ -17,12 +18,10 @@ def _lib_name() -> str: return name -def build_libxgboost(cpp_src_dir, *, tempdir): +def build_libxgboost(cpp_src_dir, *, build_dir): if not cpp_src_dir.is_dir(): raise RuntimeError(f"Expected {cpp_src_dir} to be a directory") print(f"Building {_lib_name()} from the C++ source files in {cpp_src_dir}...") - build_dir = tempdir / "build" - build_dir.mkdir(exist_ok=True) if shutil.which("ninja"): build_tool = "ninja" @@ -52,7 +51,7 @@ def build_libxgboost(cpp_src_dir, *, tempdir): return build_dir / "lib" / _lib_name() -def locate_or_build_libxgboost(toplevel_dir, *, tempdir): +def locate_or_build_libxgboost(toplevel_dir, *, build_dir): libxgboost = toplevel_dir.parent / "lib" / _lib_name() if libxgboost.exists(): print(f"Found {libxgboost.name}") @@ -65,4 +64,4 @@ def locate_or_build_libxgboost(toplevel_dir, *, tempdir): "Before building a binary wheel, please build XGBoost shared library using CMake. " f"The setup script will expect to see {_lib_name()} at {libxgboost}" ) - return build_libxgboost(cpp_src_dir, tempdir=tempdir) + return build_libxgboost(cpp_src_dir, build_dir=build_dir) diff --git a/python-package/packager/pep517.py b/python-package/packager/pep517.py new file mode 100644 index 000000000000..d3f6c3169aa9 --- /dev/null +++ b/python-package/packager/pep517.py @@ -0,0 +1,132 @@ +""" +Custom build backend for XGBoost Python package. +Builds source distribution and binary wheels +Follows PEP 517 +""" +import os +import pathlib +import shutil +import subprocess +import sysconfig +import tempfile +from contextlib import contextmanager + +import hatchling.build + +from .nativelib import locate_or_build_libxgboost +from .sdist import copy_cpp_src_tree + + +@contextmanager +def cd(path): + if isinstance(path, pathlib.Path): + path = str(path) + path = os.path.realpath(path) + cwd = os.getcwd() + os.chdir(path) + print("cd " + path) + try: + yield path + finally: + os.chdir(cwd) + + +def get_tag(): + tag_platform = sysconfig.get_platform().replace("-", "_").replace(".", "_") + return f"py3-none-{tag_platform}" + + +TOPLEVEL_DIR = pathlib.Path(__file__).parent.parent.absolute().resolve() + + +def get_requires_for_build_wheel(config_settings=None): + return hatchling.build.get_requires_for_build_wheel(config_settings) + + +def get_requires_for_build_sdist(config_settings=None): + return hatchling.build.get_requires_for_build_sdist(config_settings) + + +def write_hatch_config(dest_dir): + """Write a small custom hook for Hatch, to set a custom tag.""" + template = ( + "from hatchling.builders.hooks.plugin.interface import BuildHookInterface\n" + "class CustomBuildHook(BuildHookInterface):\n" + " def initialize(self, version, build_data):\n" + " build_data['tag'] = '{tag}'\n" + ) + with open(dest_dir / "hatch_build.py", "w") as f: + f.write(template.format(tag=get_tag())) + + +def build_wheel( + wheel_directory, + config_settings=None, + metadata_directory=None, +): + if config_settings: + raise NotImplementedError( + f"XGBoost's custom build backend doesn't support config_settings option." + f"{config_settings=}" + ) + + # Create tempdir with Python package + libxgboost + with tempfile.TemporaryDirectory() as td: + td_path = pathlib.Path(td) + build_dir = td_path / "libbuild" + build_dir.mkdir() + + whl_workspace_path = td_path / "whl_workspace" + whl_workspace_path.mkdir() + shutil.copy(TOPLEVEL_DIR / "pyproject.toml", whl_workspace_path) + shutil.copy(TOPLEVEL_DIR / "README.rst", whl_workspace_path) + write_hatch_config(whl_workspace_path) + + pkg_path = whl_workspace_path / "xgboost" + shutil.copytree(TOPLEVEL_DIR / "xgboost", pkg_path) + lib_path = pkg_path / "lib" + lib_path.mkdir() + libxgboost = locate_or_build_libxgboost(TOPLEVEL_DIR, build_dir=build_dir) + shutil.copy(libxgboost, lib_path) + + subprocess.check_call(["find", str(whl_workspace_path)]) + + with cd(whl_workspace_path): + wheel_name = hatchling.build.build_wheel( + wheel_directory, config_settings, metadata_directory + ) + return wheel_name + + +def build_sdist(sdist_directory, config_settings=None): + if config_settings: + raise NotImplementedError( + f"XGBoost's custom build backend doesn't support config_settings option." + f"{config_settings=}" + ) + + cpp_src_dir = TOPLEVEL_DIR.parent + if not cpp_src_dir.joinpath("CMakeLists.txt").exists(): + raise RuntimeError(f"Did not find CMakeLists.txt from {cpp_src_dir}") + + # Create tempdir with Python package + C++ sources + with tempfile.TemporaryDirectory() as td: + td_path = pathlib.Path(td) + + sdist_workspace_path = td_path / "sdist_workspace" + sdist_workspace_path.mkdir() + shutil.copy(TOPLEVEL_DIR / "pyproject.toml", sdist_workspace_path) + shutil.copy(TOPLEVEL_DIR / "README.rst", sdist_workspace_path) + write_hatch_config(sdist_workspace_path) + + shutil.copytree(TOPLEVEL_DIR / "xgboost", sdist_workspace_path / "xgboost") + shutil.copytree(TOPLEVEL_DIR / "packager", sdist_workspace_path / "packager") + + temp_cpp_src_dir = sdist_workspace_path / "cpp_src" + copy_cpp_src_tree(cpp_src_dir, target_dir=temp_cpp_src_dir) + + subprocess.check_call(["find", str(sdist_workspace_path)]) + + with cd(sdist_workspace_path): + sdist_name = hatchling.build.build_sdist(sdist_directory, config_settings) + return sdist_name diff --git a/python-package/xgboost/packager/sdist.py b/python-package/packager/sdist.py similarity index 100% rename from python-package/xgboost/packager/sdist.py rename to python-package/packager/sdist.py diff --git a/python-package/pyproject.toml b/python-package/pyproject.toml index 717e796ebcc6..0a8f2267121b 100644 --- a/python-package/pyproject.toml +++ b/python-package/pyproject.toml @@ -1,6 +1,9 @@ [build-system] -requires = ["packaging"] -backend-path = ["xgboost"] +requires = [ + "packaging", + "hatchling" +] +backend-path = ["."] build-backend = "packager.pep517" [project] @@ -36,3 +39,5 @@ dask = ["dask", "pandas", "distributed"] datatable = ["datatable"] plotting = ["graphviz", "matplotlib"] pyspark = ["pyspark", "scikit-learn", "cloudpickle"] + +[tool.hatch.build.targets.wheel.hooks.custom] diff --git a/python-package/xgboost/packager/distinfo.py b/python-package/xgboost/packager/distinfo.py deleted file mode 100644 index 8c983da3c885..000000000000 --- a/python-package/xgboost/packager/distinfo.py +++ /dev/null @@ -1,50 +0,0 @@ -import base64 -import csv -import email.message -import hashlib - - -def create_dist_info_dir(container, name, version): - print("create_dist_info_dir()") - dist_info = container / f"{name}-{version}.dist-info" - dist_info.mkdir() - return dist_info - - -def write_metadata(dist_info, name, version): - print("write_metadata()") - m = email.message.EmailMessage() # RFC 822. - m["Metadata-Version"] = "2.1" - m["Name"] = name - m["Version"] = version - dist_info.joinpath("METADATA").write_bytes(bytes(m)) - - -def _record_row_from_path(path, relative): - file_data = path.read_bytes() - file_size = len(file_data) - file_hash = ( - base64.urlsafe_b64encode(hashlib.sha256(file_data).digest()) - .decode("latin1") - .rstrip("=") - ) - return [relative.as_posix(), f"sha256={file_hash}", str(file_size)] - - -def iter_files(roots): - for root in roots: - for path in root.glob("**/*"): - if not path.is_file(): - continue - if path.suffix == ".pyc" or path.parent.name == "__pycache__": - continue - yield path, path.relative_to(root.parent) - - -def write_record(dist_info, package): - print("write_record()") - with dist_info.joinpath("RECORD").open("w") as f: - w = csv.writer(f, lineterminator="\n") - for path, relative in iter_files((package, dist_info)): - w.writerow(_record_row_from_path(path, relative)) - w.writerow([f"{dist_info.name}/RECORD", "", ""]) diff --git a/python-package/xgboost/packager/pep517.py b/python-package/xgboost/packager/pep517.py deleted file mode 100644 index 7dee26d2d056..000000000000 --- a/python-package/xgboost/packager/pep517.py +++ /dev/null @@ -1,98 +0,0 @@ -""" -Custom build backend for XGBoost Python package. -Builds source distribution and binary wheels -Follows PEP 517 -""" -import pathlib -import sysconfig -import tarfile -import tempfile - -import packaging.version - -from .distinfo import iter_files -from .nativelib import locate_or_build_libxgboost -from .sdist import copy_cpp_src_tree -from .wheel import create_dist_info, create_wheel - - -def get_tag(): - tag_platform = sysconfig.get_platform().replace("-", "_").replace(".", "_") - return f"py3-none-{tag_platform}" - - -def get_version(toplevel_dir): - version = ( - open(toplevel_dir / "xgboost" / "VERSION", "r", encoding="utf-8").read().strip() - ) - return str(packaging.version.Version(version)) - - -TOPLEVEL_DIR = pathlib.Path(__file__).parent.parent.parent.absolute().resolve() -NAME = "xgboost" -VERSION = get_version(TOPLEVEL_DIR) -TAG = get_tag() -PACKAGE = pathlib.Path(TOPLEVEL_DIR / "xgboost") - - -def build_wheel( - wheel_directory, - config_settings=None, - metadata_directory=None, -): - print("build_wheel()") - with tempfile.TemporaryDirectory() as td: - if config_settings is not None: - raise NotImplementedError( - f"XGBoost's custom build backend doesn't support config_settings option" - ) - if metadata_directory is None: - td_path = pathlib.Path(td) - dist_info = create_dist_info( - NAME, - VERSION, - TAG, - PACKAGE, - td_path, - ) - else: - raise NotImplementedError( - f"XGBoost's custom build backend doesn't support metadata_directory option" - ) - - wheel_path = create_wheel( - NAME, - VERSION, - TAG, - PACKAGE, - dist_info=dist_info, - libxgboost=locate_or_build_libxgboost(TOPLEVEL_DIR, tempdir=td_path), - output_dir=pathlib.Path(wheel_directory).absolute().resolve(), - ) - - return wheel_path.name - - -def build_sdist(sdist_directory, config_settings=None): - if config_settings: - raise NotImplementedError( - f"XGBoost's custom build backend doesn't support config_settings option." - f"{config_settings=}" - ) - - cpp_src_dir = TOPLEVEL_DIR.parent - if not cpp_src_dir.joinpath("CMakeLists.txt").exists(): - raise RuntimeError(f"Did not find CMakeLists.txt from {cpp_src_dir}") - - sdist_path = pathlib.Path(sdist_directory, f"{NAME}-{VERSION}.tar.gz") - with tempfile.TemporaryDirectory() as td: - temp_cpp_src_dir = pathlib.Path(td) / "cpp_src" - copy_cpp_src_tree(cpp_src_dir, temp_cpp_src_dir) - with tarfile.open(sdist_path, "w:gz", format=tarfile.PAX_FORMAT) as tf: - for path, relative in iter_files((PACKAGE,)): - tf.add(path, relative.as_posix()) - for path, relative in iter_files((temp_cpp_src_dir,)): - tf.add(path, relative.as_posix()) - pyproject_path = TOPLEVEL_DIR / "pyproject.toml" - tf.add(pyproject_path, pyproject_path.relative_to(TOPLEVEL_DIR)) - return sdist_path.name diff --git a/python-package/xgboost/packager/wheel.py b/python-package/xgboost/packager/wheel.py deleted file mode 100644 index c94379898ceb..000000000000 --- a/python-package/xgboost/packager/wheel.py +++ /dev/null @@ -1,33 +0,0 @@ -import email.message -import zipfile - -from .distinfo import create_dist_info_dir, iter_files, write_metadata, write_record - - -def write_wheel_metadata(dist_info, tag): - print("write_wheel_metadata()") - m = email.message.EmailMessage() - m["Wheel-Version"] = "1.0" - m["Generator"] = "packager/wheel.py" - m["Root-Is-Purelib"] = "true" - m["Tag"] = tag - dist_info.joinpath("WHEEL").write_bytes(bytes(m)) - - -def create_dist_info(name, version, tag, package, output_dir): - print("create_dist_info()") - dist_info = create_dist_info_dir(output_dir, name, version) - write_metadata(dist_info, name, version) - write_wheel_metadata(dist_info, tag) - write_record(dist_info, package) - return dist_info - - -def create_wheel(name, version, tag, package, *, dist_info, libxgboost, output_dir): - print("create_wheel()") - wheel_path = output_dir / f"{name}-{version}-{tag}.whl" - with zipfile.ZipFile(wheel_path, "w") as zf: - for path, relative in iter_files((package, dist_info)): - zf.write(path, relative.as_posix()) - zf.write(libxgboost, f"xgboost/lib/{libxgboost.name}") - return wheel_path From e3585ff2f10376ba10bb5f67e3983ee53bfc6942 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 16:15:18 -0700 Subject: [PATCH 07/61] Use proper logger --- python-package/packager/nativelib.py | 16 ++++++--- python-package/packager/pep517.py | 52 +++++++++++++++++++--------- python-package/packager/sdist.py | 8 ++--- python-package/packager/util.py | 14 ++++++++ 4 files changed, 65 insertions(+), 25 deletions(-) create mode 100644 python-package/packager/util.py diff --git a/python-package/packager/nativelib.py b/python-package/packager/nativelib.py index 6bf490dc1c21..1d3f156aa98d 100644 --- a/python-package/packager/nativelib.py +++ b/python-package/packager/nativelib.py @@ -1,7 +1,7 @@ +import logging import os import shutil import subprocess -import tempfile from platform import system @@ -19,9 +19,13 @@ def _lib_name() -> str: def build_libxgboost(cpp_src_dir, *, build_dir): + logger = logging.getLogger("xgboost.packager.build_libxgboost") + if not cpp_src_dir.is_dir(): raise RuntimeError(f"Expected {cpp_src_dir} to be a directory") - print(f"Building {_lib_name()} from the C++ source files in {cpp_src_dir}...") + logger.info( + "Building %s from the C++ source files in %s...", _lib_name(), str(cpp_src_dir) + ) if shutil.which("ninja"): build_tool = "ninja" @@ -37,11 +41,11 @@ def build_libxgboost(cpp_src_dir, *, build_dir): ) generator = "-GNinja" if build_tool == "ninja" else "-GUnix Makefiles" - cmake_cmd = ["cmake", cpp_src_dir, generator] + cmake_cmd = ["cmake", str(cpp_src_dir), generator] cmake_cmd.append("-DKEEP_BUILD_ARTIFACTS_IN_BINARY_DIR=ON") # TODO(hcho3): handle CMake args - print(f"{cmake_cmd=}") + logger.info("CMake args: %s", str(cmake_cmd)) subprocess.check_call(cmake_cmd, cwd=build_dir) nproc = os.cpu_count() @@ -52,9 +56,11 @@ def build_libxgboost(cpp_src_dir, *, build_dir): def locate_or_build_libxgboost(toplevel_dir, *, build_dir): + logger = logging.getLogger("xgboost.packager.locate_or_build_libxgboost") + libxgboost = toplevel_dir.parent / "lib" / _lib_name() if libxgboost.exists(): - print(f"Found {libxgboost.name}") + logger.info("Found %s at %s", libxgboost.name, str(libxgboost.parent)) return libxgboost if toplevel_dir.joinpath("cpp_src").exists(): # Source distribution; all C++ source files to be found in cpp_src/ diff --git a/python-package/packager/pep517.py b/python-package/packager/pep517.py index d3f6c3169aa9..2a41b2fb69fa 100644 --- a/python-package/packager/pep517.py +++ b/python-package/packager/pep517.py @@ -3,10 +3,9 @@ Builds source distribution and binary wheels Follows PEP 517 """ +import logging import os import pathlib -import shutil -import subprocess import sysconfig import tempfile from contextlib import contextmanager @@ -15,6 +14,7 @@ from .nativelib import locate_or_build_libxgboost from .sdist import copy_cpp_src_tree +from .util import copy_with_logging, copytree_with_logging @contextmanager @@ -24,7 +24,6 @@ def cd(path): path = os.path.realpath(path) cwd = os.getcwd() os.chdir(path) - print("cd " + path) try: yield path finally: @@ -37,6 +36,7 @@ def get_tag(): TOPLEVEL_DIR = pathlib.Path(__file__).parent.parent.absolute().resolve() +logging.basicConfig(level=logging.INFO) def get_requires_for_build_wheel(config_settings=None): @@ -64,6 +64,8 @@ def build_wheel( config_settings=None, metadata_directory=None, ): + logger = logging.getLogger("xgboost.packager.build_wheel") + if config_settings: raise NotImplementedError( f"XGBoost's custom build backend doesn't support config_settings option." @@ -78,18 +80,24 @@ def build_wheel( whl_workspace_path = td_path / "whl_workspace" whl_workspace_path.mkdir() - shutil.copy(TOPLEVEL_DIR / "pyproject.toml", whl_workspace_path) - shutil.copy(TOPLEVEL_DIR / "README.rst", whl_workspace_path) + logger.info( + "Copying project files to temporary directory %s", str(whl_workspace_path) + ) + + copy_with_logging( + TOPLEVEL_DIR / "pyproject.toml", whl_workspace_path, logger=logger + ) + copy_with_logging( + TOPLEVEL_DIR / "README.rst", whl_workspace_path, logger=logger + ) write_hatch_config(whl_workspace_path) pkg_path = whl_workspace_path / "xgboost" - shutil.copytree(TOPLEVEL_DIR / "xgboost", pkg_path) + copytree_with_logging(TOPLEVEL_DIR / "xgboost", pkg_path, logger=logger) lib_path = pkg_path / "lib" lib_path.mkdir() libxgboost = locate_or_build_libxgboost(TOPLEVEL_DIR, build_dir=build_dir) - shutil.copy(libxgboost, lib_path) - - subprocess.check_call(["find", str(whl_workspace_path)]) + copy_with_logging(libxgboost, lib_path, logger=logger) with cd(whl_workspace_path): wheel_name = hatchling.build.build_wheel( @@ -99,6 +107,8 @@ def build_wheel( def build_sdist(sdist_directory, config_settings=None): + logger = logging.getLogger("xgboost.packager.build_sdist") + if config_settings: raise NotImplementedError( f"XGBoost's custom build backend doesn't support config_settings option." @@ -115,17 +125,27 @@ def build_sdist(sdist_directory, config_settings=None): sdist_workspace_path = td_path / "sdist_workspace" sdist_workspace_path.mkdir() - shutil.copy(TOPLEVEL_DIR / "pyproject.toml", sdist_workspace_path) - shutil.copy(TOPLEVEL_DIR / "README.rst", sdist_workspace_path) + logger.info( + "Copying project files to temporary directory %s", str(sdist_workspace_path) + ) + + copy_with_logging( + TOPLEVEL_DIR / "pyproject.toml", sdist_workspace_path, logger=logger + ) + copy_with_logging( + TOPLEVEL_DIR / "README.rst", sdist_workspace_path, logger=logger + ) write_hatch_config(sdist_workspace_path) - shutil.copytree(TOPLEVEL_DIR / "xgboost", sdist_workspace_path / "xgboost") - shutil.copytree(TOPLEVEL_DIR / "packager", sdist_workspace_path / "packager") + copytree_with_logging( + TOPLEVEL_DIR / "xgboost", sdist_workspace_path / "xgboost", logger=logger + ) + copytree_with_logging( + TOPLEVEL_DIR / "packager", sdist_workspace_path / "packager", logger=logger + ) temp_cpp_src_dir = sdist_workspace_path / "cpp_src" - copy_cpp_src_tree(cpp_src_dir, target_dir=temp_cpp_src_dir) - - subprocess.check_call(["find", str(sdist_workspace_path)]) + copy_cpp_src_tree(cpp_src_dir, target_dir=temp_cpp_src_dir, logger=logger) with cd(sdist_workspace_path): sdist_name = hatchling.build.build_sdist(sdist_directory, config_settings) diff --git a/python-package/packager/sdist.py b/python-package/packager/sdist.py index 31e3b185d5f3..9fc7af09c32e 100644 --- a/python-package/packager/sdist.py +++ b/python-package/packager/sdist.py @@ -1,7 +1,7 @@ -import shutil +from .util import copy_with_logging, copytree_with_logging -def copy_cpp_src_tree(cpp_src_dir, target_dir): +def copy_cpp_src_tree(cpp_src_dir, target_dir, *, logger): """Copy C++ source tree into build directory""" for subdir in [ @@ -13,7 +13,7 @@ def copy_cpp_src_tree(cpp_src_dir, target_dir): "cmake", "plugin", ]: - shutil.copytree(cpp_src_dir.joinpath(subdir), target_dir.joinpath(subdir)) + copytree_with_logging(cpp_src_dir / subdir, target_dir / subdir, logger=logger) for filename in ["CMakeLists.txt", "LICENSE"]: - shutil.copy(cpp_src_dir.joinpath(filename), target_dir) + copy_with_logging(cpp_src_dir.joinpath(filename), target_dir, logger=logger) diff --git a/python-package/packager/util.py b/python-package/packager/util.py new file mode 100644 index 000000000000..6a106d21b92f --- /dev/null +++ b/python-package/packager/util.py @@ -0,0 +1,14 @@ +import shutil + + +def copytree_with_logging(src, dest, *, logger): + logger.info("Copying %s -> %s", str(src), str(dest)) + shutil.copytree(src, dest) + + +def copy_with_logging(src, dest, *, logger): + if dest.is_dir(): + logger.info("Copying %s -> %s", str(src), str(dest / src.name)) + else: + logger.info("Copying %s -> %s", str(src), str(dest)) + shutil.copy(src, dest) From 0018b668d087af2c41a19a404ff10c32fcc267af Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 16:47:52 -0700 Subject: [PATCH 08/61] Don't include hatch_build.py in sdist --- python-package/packager/pep517.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/python-package/packager/pep517.py b/python-package/packager/pep517.py index 2a41b2fb69fa..5271c6908f4a 100644 --- a/python-package/packager/pep517.py +++ b/python-package/packager/pep517.py @@ -47,7 +47,7 @@ def get_requires_for_build_sdist(config_settings=None): return hatchling.build.get_requires_for_build_sdist(config_settings) -def write_hatch_config(dest_dir): +def write_hatch_config(dest_dir, *, logger): """Write a small custom hook for Hatch, to set a custom tag.""" template = ( "from hatchling.builders.hooks.plugin.interface import BuildHookInterface\n" @@ -55,7 +55,9 @@ def write_hatch_config(dest_dir): " def initialize(self, version, build_data):\n" " build_data['tag'] = '{tag}'\n" ) - with open(dest_dir / "hatch_build.py", "w") as f: + hatch_build_file = dest_dir / "hatch_build.py" + logger.info("Writing %s", str(hatch_build_file)) + with open(hatch_build_file, "w") as f: f.write(template.format(tag=get_tag())) @@ -90,7 +92,7 @@ def build_wheel( copy_with_logging( TOPLEVEL_DIR / "README.rst", whl_workspace_path, logger=logger ) - write_hatch_config(whl_workspace_path) + write_hatch_config(whl_workspace_path, logger=logger) pkg_path = whl_workspace_path / "xgboost" copytree_with_logging(TOPLEVEL_DIR / "xgboost", pkg_path, logger=logger) @@ -135,7 +137,6 @@ def build_sdist(sdist_directory, config_settings=None): copy_with_logging( TOPLEVEL_DIR / "README.rst", sdist_workspace_path, logger=logger ) - write_hatch_config(sdist_workspace_path) copytree_with_logging( TOPLEVEL_DIR / "xgboost", sdist_workspace_path / "xgboost", logger=logger From 95ad176b75917692bb15393653fb22124a32c90c Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 16:52:35 -0700 Subject: [PATCH 09/61] Rename to make it less verbose --- python-package/packager/pep517.py | 46 ++++++++++++------------------- 1 file changed, 17 insertions(+), 29 deletions(-) diff --git a/python-package/packager/pep517.py b/python-package/packager/pep517.py index 5271c6908f4a..79b08fd46585 100644 --- a/python-package/packager/pep517.py +++ b/python-package/packager/pep517.py @@ -80,28 +80,22 @@ def build_wheel( build_dir = td_path / "libbuild" build_dir.mkdir() - whl_workspace_path = td_path / "whl_workspace" - whl_workspace_path.mkdir() - logger.info( - "Copying project files to temporary directory %s", str(whl_workspace_path) - ) + workspace = td_path / "whl_workspace" + workspace.mkdir() + logger.info("Copying project files to temporary directory %s", str(workspace)) - copy_with_logging( - TOPLEVEL_DIR / "pyproject.toml", whl_workspace_path, logger=logger - ) - copy_with_logging( - TOPLEVEL_DIR / "README.rst", whl_workspace_path, logger=logger - ) - write_hatch_config(whl_workspace_path, logger=logger) + copy_with_logging(TOPLEVEL_DIR / "pyproject.toml", workspace, logger=logger) + copy_with_logging(TOPLEVEL_DIR / "README.rst", workspace, logger=logger) + write_hatch_config(workspace, logger=logger) - pkg_path = whl_workspace_path / "xgboost" + pkg_path = workspace / "xgboost" copytree_with_logging(TOPLEVEL_DIR / "xgboost", pkg_path, logger=logger) lib_path = pkg_path / "lib" lib_path.mkdir() libxgboost = locate_or_build_libxgboost(TOPLEVEL_DIR, build_dir=build_dir) copy_with_logging(libxgboost, lib_path, logger=logger) - with cd(whl_workspace_path): + with cd(workspace): wheel_name = hatchling.build.build_wheel( wheel_directory, config_settings, metadata_directory ) @@ -125,29 +119,23 @@ def build_sdist(sdist_directory, config_settings=None): with tempfile.TemporaryDirectory() as td: td_path = pathlib.Path(td) - sdist_workspace_path = td_path / "sdist_workspace" - sdist_workspace_path.mkdir() - logger.info( - "Copying project files to temporary directory %s", str(sdist_workspace_path) - ) + workspace = td_path / "sdist_workspace" + workspace.mkdir() + logger.info("Copying project files to temporary directory %s", str(workspace)) - copy_with_logging( - TOPLEVEL_DIR / "pyproject.toml", sdist_workspace_path, logger=logger - ) - copy_with_logging( - TOPLEVEL_DIR / "README.rst", sdist_workspace_path, logger=logger - ) + copy_with_logging(TOPLEVEL_DIR / "pyproject.toml", workspace, logger=logger) + copy_with_logging(TOPLEVEL_DIR / "README.rst", workspace, logger=logger) copytree_with_logging( - TOPLEVEL_DIR / "xgboost", sdist_workspace_path / "xgboost", logger=logger + TOPLEVEL_DIR / "xgboost", workspace / "xgboost", logger=logger ) copytree_with_logging( - TOPLEVEL_DIR / "packager", sdist_workspace_path / "packager", logger=logger + TOPLEVEL_DIR / "packager", workspace / "packager", logger=logger ) - temp_cpp_src_dir = sdist_workspace_path / "cpp_src" + temp_cpp_src_dir = workspace / "cpp_src" copy_cpp_src_tree(cpp_src_dir, target_dir=temp_cpp_src_dir, logger=logger) - with cd(sdist_workspace_path): + with cd(workspace): sdist_name = hatchling.build.build_sdist(sdist_directory, config_settings) return sdist_name From 359f1e437cfeabd063dc09b08a36b0bc00bff8f1 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 17:10:10 -0700 Subject: [PATCH 10/61] Support 'pip install .' --- python-package/packager/nativelib.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/python-package/packager/nativelib.py b/python-package/packager/nativelib.py index 1d3f156aa98d..27674d5269d4 100644 --- a/python-package/packager/nativelib.py +++ b/python-package/packager/nativelib.py @@ -66,8 +66,8 @@ def locate_or_build_libxgboost(toplevel_dir, *, build_dir): # Source distribution; all C++ source files to be found in cpp_src/ cpp_src_dir = toplevel_dir.joinpath("cpp_src") else: - raise RuntimeError( - "Before building a binary wheel, please build XGBoost shared library using CMake. " - f"The setup script will expect to see {_lib_name()} at {libxgboost}" - ) + # Probably running "pip install ." from python-package/ + cpp_src_dir = toplevel_dir.parent + if not cpp_src_dir.joinpath("CMakeLists.txt").exists(): + raise RuntimeError(f"Did not find CMakeLists.txt from {cpp_src_dir}") return build_libxgboost(cpp_src_dir, build_dir=build_dir) From a08ed15255f97da45021f42faf0cd824ecb62ace Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 17:16:02 -0700 Subject: [PATCH 11/61] Update build-time requirements --- python-package/pyproject.toml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/python-package/pyproject.toml b/python-package/pyproject.toml index 0a8f2267121b..15a87df1ebf5 100644 --- a/python-package/pyproject.toml +++ b/python-package/pyproject.toml @@ -1,7 +1,6 @@ [build-system] requires = [ - "packaging", - "hatchling" + "hatchling>=1.12.1" ] backend-path = ["."] build-backend = "packager.pep517" From 8505ea93aa4a43849e6fc133c9200fd7e05d85f2 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 17:23:47 -0700 Subject: [PATCH 12/61] Support editable installation --- .gitignore | 3 +++ python-package/packager/pep517.py | 13 +++++++++++++ 2 files changed, 16 insertions(+) diff --git a/.gitignore b/.gitignore index 3a606c847f0e..23a2826d3cca 100644 --- a/.gitignore +++ b/.gitignore @@ -149,3 +149,6 @@ model*.json *.rds Rplots.pdf *.zip + +# Python packaging +/python-package/hatch_build.py diff --git a/python-package/packager/pep517.py b/python-package/packager/pep517.py index 79b08fd46585..3764b302069c 100644 --- a/python-package/packager/pep517.py +++ b/python-package/packager/pep517.py @@ -47,6 +47,10 @@ def get_requires_for_build_sdist(config_settings=None): return hatchling.build.get_requires_for_build_sdist(config_settings) +def get_requires_for_build_editable(config_settings=None): + return hatchling.build.get_requires_for_build_editable(config_settings) + + def write_hatch_config(dest_dir, *, logger): """Write a small custom hook for Hatch, to set a custom tag.""" template = ( @@ -139,3 +143,12 @@ def build_sdist(sdist_directory, config_settings=None): with cd(workspace): sdist_name = hatchling.build.build_sdist(sdist_directory, config_settings) return sdist_name + + +def build_editable(wheel_directory, config_settings=None, metadata_directory=None): + logger = logging.getLogger("xgboost.packager.build_editable") + + write_hatch_config(TOPLEVEL_DIR, logger=logger) + return hatchling.build.build_editable( + wheel_directory, config_settings, metadata_directory + ) From ca0577098341b96f012e73cff18740fd80d463cb Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 17:43:11 -0700 Subject: [PATCH 13/61] Fix lint --- python-package/packager/nativelib.py | 7 ++++++- python-package/packager/pep517.py | 13 +++++++++++-- python-package/packager/sdist.py | 3 +++ python-package/packager/util.py | 5 +++++ 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/python-package/packager/nativelib.py b/python-package/packager/nativelib.py index 27674d5269d4..d1039dd1faa6 100644 --- a/python-package/packager/nativelib.py +++ b/python-package/packager/nativelib.py @@ -1,3 +1,6 @@ +""" +Functions for building libxgboost +""" import logging import os import shutil @@ -19,6 +22,7 @@ def _lib_name() -> str: def build_libxgboost(cpp_src_dir, *, build_dir): + """Build libxgboost in a temporary directory and obtain the path to built libxgboost""" logger = logging.getLogger("xgboost.packager.build_libxgboost") if not cpp_src_dir.is_dir(): @@ -34,7 +38,7 @@ def build_libxgboost(cpp_src_dir, *, build_dir): if system() == "Windows": raise NotImplementedError( - f"Installing from sdist is not supported on Windows. You have two alternatives:\n" + "Installing from sdist is not supported on Windows. You have two alternatives:\n" "1. Install XGBoost from the official wheel (recommended): pip install xgboost\n" "2. Build XGBoost from the source by running CMake at the project root folder. See " "documentation for details." @@ -56,6 +60,7 @@ def build_libxgboost(cpp_src_dir, *, build_dir): def locate_or_build_libxgboost(toplevel_dir, *, build_dir): + """Locate libxgboost; if not exist, build it""" logger = logging.getLogger("xgboost.packager.locate_or_build_libxgboost") libxgboost = toplevel_dir.parent / "lib" / _lib_name() diff --git a/python-package/packager/pep517.py b/python-package/packager/pep517.py index 3764b302069c..31e061bbcdf9 100644 --- a/python-package/packager/pep517.py +++ b/python-package/packager/pep517.py @@ -1,7 +1,8 @@ """ Custom build backend for XGBoost Python package. -Builds source distribution and binary wheels -Follows PEP 517 +Builds source distribution and binary wheels, following PEP 517 / PEP 660. +Re-uses components of Hatchling (https://github.com/pypa/hatch/tree/master/backend) for the sake +of brevity. """ import logging import os @@ -19,6 +20,7 @@ @contextmanager def cd(path): + """Temporarily change working directory""" if isinstance(path, pathlib.Path): path = str(path) path = os.path.realpath(path) @@ -31,6 +33,7 @@ def cd(path): def get_tag(): + """Get appropate wheel tag, according to system""" tag_platform = sysconfig.get_platform().replace("-", "_").replace(".", "_") return f"py3-none-{tag_platform}" @@ -40,14 +43,17 @@ def get_tag(): def get_requires_for_build_wheel(config_settings=None): + """A PEP 517 method. Delegate to Hatchling""" return hatchling.build.get_requires_for_build_wheel(config_settings) def get_requires_for_build_sdist(config_settings=None): + """A PEP 517 method. Delegate to Hatchling""" return hatchling.build.get_requires_for_build_sdist(config_settings) def get_requires_for_build_editable(config_settings=None): + """A PEP 517 method. Delegate to Hatchling""" return hatchling.build.get_requires_for_build_editable(config_settings) @@ -70,6 +76,7 @@ def build_wheel( config_settings=None, metadata_directory=None, ): + """Build a wheel""" logger = logging.getLogger("xgboost.packager.build_wheel") if config_settings: @@ -107,6 +114,7 @@ def build_wheel( def build_sdist(sdist_directory, config_settings=None): + """Build a source distribution""" logger = logging.getLogger("xgboost.packager.build_sdist") if config_settings: @@ -146,6 +154,7 @@ def build_sdist(sdist_directory, config_settings=None): def build_editable(wheel_directory, config_settings=None, metadata_directory=None): + """Build an editable installation. We mostly delegate to Hatchling.""" logger = logging.getLogger("xgboost.packager.build_editable") write_hatch_config(TOPLEVEL_DIR, logger=logger) diff --git a/python-package/packager/sdist.py b/python-package/packager/sdist.py index 9fc7af09c32e..1664218bce98 100644 --- a/python-package/packager/sdist.py +++ b/python-package/packager/sdist.py @@ -1,3 +1,6 @@ +""" +Functions for building sdist +""" from .util import copy_with_logging, copytree_with_logging diff --git a/python-package/packager/util.py b/python-package/packager/util.py index 6a106d21b92f..0db118c1accd 100644 --- a/python-package/packager/util.py +++ b/python-package/packager/util.py @@ -1,12 +1,17 @@ +""" +Utility functions for implementing PEP 517 backend +""" import shutil def copytree_with_logging(src, dest, *, logger): + """Call shutil.copytree() with logging""" logger.info("Copying %s -> %s", str(src), str(dest)) shutil.copytree(src, dest) def copy_with_logging(src, dest, *, logger): + """Call shutil.copy() with logging""" if dest.is_dir(): logger.info("Copying %s -> %s", str(src), str(dest / src.name)) else: From 9997b7b5c55fe62ac22db59cc18c56b3f8f3b53f Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 17:59:31 -0700 Subject: [PATCH 14/61] Add type annotation --- python-package/packager/nativelib.py | 11 ++++++-- python-package/packager/pep517.py | 38 +++++++++++++++++++--------- python-package/packager/sdist.py | 7 ++++- tests/ci_build/lint_python.py | 2 ++ 4 files changed, 43 insertions(+), 15 deletions(-) diff --git a/python-package/packager/nativelib.py b/python-package/packager/nativelib.py index d1039dd1faa6..988422285205 100644 --- a/python-package/packager/nativelib.py +++ b/python-package/packager/nativelib.py @@ -3,6 +3,7 @@ """ import logging import os +import pathlib import shutil import subprocess from platform import system @@ -21,7 +22,11 @@ def _lib_name() -> str: return name -def build_libxgboost(cpp_src_dir, *, build_dir): +def build_libxgboost( + cpp_src_dir: pathlib.Path, + *, + build_dir: pathlib.Path, +) -> pathlib.Path: """Build libxgboost in a temporary directory and obtain the path to built libxgboost""" logger = logging.getLogger("xgboost.packager.build_libxgboost") @@ -59,7 +64,9 @@ def build_libxgboost(cpp_src_dir, *, build_dir): return build_dir / "lib" / _lib_name() -def locate_or_build_libxgboost(toplevel_dir, *, build_dir): +def locate_or_build_libxgboost( + toplevel_dir: pathlib.Path, *, build_dir: pathlib.Path +) -> pathlib.Path: """Locate libxgboost; if not exist, build it""" logger = logging.getLogger("xgboost.packager.locate_or_build_libxgboost") diff --git a/python-package/packager/pep517.py b/python-package/packager/pep517.py index 31e061bbcdf9..2b9e2ecc4fb9 100644 --- a/python-package/packager/pep517.py +++ b/python-package/packager/pep517.py @@ -10,6 +10,7 @@ import sysconfig import tempfile from contextlib import contextmanager +from typing import Any, Optional, Union import hatchling.build @@ -19,7 +20,7 @@ @contextmanager -def cd(path): +def cd(path: Union[str, pathlib.Path]): """Temporarily change working directory""" if isinstance(path, pathlib.Path): path = str(path) @@ -32,7 +33,7 @@ def cd(path): os.chdir(cwd) -def get_tag(): +def get_tag() -> str: """Get appropate wheel tag, according to system""" tag_platform = sysconfig.get_platform().replace("-", "_").replace(".", "_") return f"py3-none-{tag_platform}" @@ -42,22 +43,28 @@ def get_tag(): logging.basicConfig(level=logging.INFO) -def get_requires_for_build_wheel(config_settings=None): +def get_requires_for_build_wheel( + config_settings: Optional[dict[str, Any]] = None +) -> list[str]: """A PEP 517 method. Delegate to Hatchling""" return hatchling.build.get_requires_for_build_wheel(config_settings) -def get_requires_for_build_sdist(config_settings=None): +def get_requires_for_build_sdist( + config_settings: Optional[dict[str, Any]] = None +) -> list[str]: """A PEP 517 method. Delegate to Hatchling""" return hatchling.build.get_requires_for_build_sdist(config_settings) -def get_requires_for_build_editable(config_settings=None): +def get_requires_for_build_editable( + config_settings: Optional[dict[str, Any]] = None +) -> list[str]: """A PEP 517 method. Delegate to Hatchling""" return hatchling.build.get_requires_for_build_editable(config_settings) -def write_hatch_config(dest_dir, *, logger): +def write_hatch_config(dest_dir: pathlib.Path, *, logger: logging.Logger): """Write a small custom hook for Hatch, to set a custom tag.""" template = ( "from hatchling.builders.hooks.plugin.interface import BuildHookInterface\n" @@ -72,10 +79,10 @@ def write_hatch_config(dest_dir, *, logger): def build_wheel( - wheel_directory, - config_settings=None, - metadata_directory=None, -): + wheel_directory: str, + config_settings: Optional[dict[str, Any]] = None, + metadata_directory: Optional[str] = None, +) -> str: """Build a wheel""" logger = logging.getLogger("xgboost.packager.build_wheel") @@ -113,7 +120,10 @@ def build_wheel( return wheel_name -def build_sdist(sdist_directory, config_settings=None): +def build_sdist( + sdist_directory: str, + config_settings: Optional[dict[str, Any]] = None, +) -> str: """Build a source distribution""" logger = logging.getLogger("xgboost.packager.build_sdist") @@ -153,7 +163,11 @@ def build_sdist(sdist_directory, config_settings=None): return sdist_name -def build_editable(wheel_directory, config_settings=None, metadata_directory=None): +def build_editable( + wheel_directory: str, + config_settings: Optional[dict[str, Any]] = None, + metadata_directory: Optional[str] = None, +) -> str: """Build an editable installation. We mostly delegate to Hatchling.""" logger = logging.getLogger("xgboost.packager.build_editable") diff --git a/python-package/packager/sdist.py b/python-package/packager/sdist.py index 1664218bce98..732d614c4fb8 100644 --- a/python-package/packager/sdist.py +++ b/python-package/packager/sdist.py @@ -1,10 +1,15 @@ """ Functions for building sdist """ +import logging +import pathlib + from .util import copy_with_logging, copytree_with_logging -def copy_cpp_src_tree(cpp_src_dir, target_dir, *, logger): +def copy_cpp_src_tree( + cpp_src_dir: pathlib.Path, target_dir: pathlib.Path, *, logger: logging.Logger +): """Copy C++ source tree into build directory""" for subdir in [ diff --git a/tests/ci_build/lint_python.py b/tests/ci_build/lint_python.py index 00791e19db8c..2f1c210c5af0 100644 --- a/tests/ci_build/lint_python.py +++ b/tests/ci_build/lint_python.py @@ -219,6 +219,8 @@ def main(args: argparse.Namespace) -> None: "tests/ci_build/test_r_package.py", "tests/ci_build/test_utils.py", "tests/ci_build/change_version.py", + # Python packaging + "python-package/packager/", ] ): subprocess.check_call(["mypy", "--version"]) From 9dab33382eacfb841d61822ba2bd36908573c414 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 18:06:42 -0700 Subject: [PATCH 15/61] Fix type annotation --- python-package/packager/pep517.py | 10 +++++----- python-package/packager/sdist.py | 2 +- python-package/packager/util.py | 8 ++++++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/python-package/packager/pep517.py b/python-package/packager/pep517.py index 2b9e2ecc4fb9..dc6799b480ad 100644 --- a/python-package/packager/pep517.py +++ b/python-package/packager/pep517.py @@ -10,7 +10,7 @@ import sysconfig import tempfile from contextlib import contextmanager -from typing import Any, Optional, Union +from typing import Any, Dict, Iterator, Optional, Union import hatchling.build @@ -20,7 +20,7 @@ @contextmanager -def cd(path: Union[str, pathlib.Path]): +def cd(path: Union[str, pathlib.Path]) -> Iterator[str]: """Temporarily change working directory""" if isinstance(path, pathlib.Path): path = str(path) @@ -44,21 +44,21 @@ def get_tag() -> str: def get_requires_for_build_wheel( - config_settings: Optional[dict[str, Any]] = None + config_settings: Optional[Dict[str, Any]] = None ) -> list[str]: """A PEP 517 method. Delegate to Hatchling""" return hatchling.build.get_requires_for_build_wheel(config_settings) def get_requires_for_build_sdist( - config_settings: Optional[dict[str, Any]] = None + config_settings: Optional[Dict[str, Any]] = None ) -> list[str]: """A PEP 517 method. Delegate to Hatchling""" return hatchling.build.get_requires_for_build_sdist(config_settings) def get_requires_for_build_editable( - config_settings: Optional[dict[str, Any]] = None + config_settings: Optional[Dict[str, Any]] = None ) -> list[str]: """A PEP 517 method. Delegate to Hatchling""" return hatchling.build.get_requires_for_build_editable(config_settings) diff --git a/python-package/packager/sdist.py b/python-package/packager/sdist.py index 732d614c4fb8..5c82001dad9e 100644 --- a/python-package/packager/sdist.py +++ b/python-package/packager/sdist.py @@ -9,7 +9,7 @@ def copy_cpp_src_tree( cpp_src_dir: pathlib.Path, target_dir: pathlib.Path, *, logger: logging.Logger -): +) -> None: """Copy C++ source tree into build directory""" for subdir in [ diff --git a/python-package/packager/util.py b/python-package/packager/util.py index 0db118c1accd..8b3f043ad8a4 100644 --- a/python-package/packager/util.py +++ b/python-package/packager/util.py @@ -1,16 +1,20 @@ """ Utility functions for implementing PEP 517 backend """ +import logging +import pathlib import shutil -def copytree_with_logging(src, dest, *, logger): +def copytree_with_logging( + src: pathlib.Path, dest: pathlib.Path, *, logger: logging.Logger +): """Call shutil.copytree() with logging""" logger.info("Copying %s -> %s", str(src), str(dest)) shutil.copytree(src, dest) -def copy_with_logging(src, dest, *, logger): +def copy_with_logging(src: pathlib.Path, dest: pathlib.Path, *, logger: logging.Logger): """Call shutil.copy() with logging""" if dest.is_dir(): logger.info("Copying %s -> %s", str(src), str(dest / src.name)) From 7c069566932ab2df15e8d674102eea219db27284 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 18:14:33 -0700 Subject: [PATCH 16/61] More fix --- python-package/packager/pep517.py | 16 ++++++++-------- python-package/packager/util.py | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/python-package/packager/pep517.py b/python-package/packager/pep517.py index dc6799b480ad..eef2c9102256 100644 --- a/python-package/packager/pep517.py +++ b/python-package/packager/pep517.py @@ -10,7 +10,7 @@ import sysconfig import tempfile from contextlib import contextmanager -from typing import Any, Dict, Iterator, Optional, Union +from typing import Any, Dict, Iterator, Optional, Union, List import hatchling.build @@ -45,26 +45,26 @@ def get_tag() -> str: def get_requires_for_build_wheel( config_settings: Optional[Dict[str, Any]] = None -) -> list[str]: +) -> List[str]: """A PEP 517 method. Delegate to Hatchling""" return hatchling.build.get_requires_for_build_wheel(config_settings) def get_requires_for_build_sdist( config_settings: Optional[Dict[str, Any]] = None -) -> list[str]: +) -> List[str]: """A PEP 517 method. Delegate to Hatchling""" return hatchling.build.get_requires_for_build_sdist(config_settings) def get_requires_for_build_editable( config_settings: Optional[Dict[str, Any]] = None -) -> list[str]: +) -> List[str]: """A PEP 517 method. Delegate to Hatchling""" return hatchling.build.get_requires_for_build_editable(config_settings) -def write_hatch_config(dest_dir: pathlib.Path, *, logger: logging.Logger): +def write_hatch_config(dest_dir: pathlib.Path, *, logger: logging.Logger) -> None: """Write a small custom hook for Hatch, to set a custom tag.""" template = ( "from hatchling.builders.hooks.plugin.interface import BuildHookInterface\n" @@ -80,7 +80,7 @@ def write_hatch_config(dest_dir: pathlib.Path, *, logger: logging.Logger): def build_wheel( wheel_directory: str, - config_settings: Optional[dict[str, Any]] = None, + config_settings: Optional[Dict[str, Any]] = None, metadata_directory: Optional[str] = None, ) -> str: """Build a wheel""" @@ -122,7 +122,7 @@ def build_wheel( def build_sdist( sdist_directory: str, - config_settings: Optional[dict[str, Any]] = None, + config_settings: Optional[Dict[str, Any]] = None, ) -> str: """Build a source distribution""" logger = logging.getLogger("xgboost.packager.build_sdist") @@ -165,7 +165,7 @@ def build_sdist( def build_editable( wheel_directory: str, - config_settings: Optional[dict[str, Any]] = None, + config_settings: Optional[Dict[str, Any]] = None, metadata_directory: Optional[str] = None, ) -> str: """Build an editable installation. We mostly delegate to Hatchling.""" diff --git a/python-package/packager/util.py b/python-package/packager/util.py index 8b3f043ad8a4..9569d22c1962 100644 --- a/python-package/packager/util.py +++ b/python-package/packager/util.py @@ -8,13 +8,13 @@ def copytree_with_logging( src: pathlib.Path, dest: pathlib.Path, *, logger: logging.Logger -): +) -> None: """Call shutil.copytree() with logging""" logger.info("Copying %s -> %s", str(src), str(dest)) shutil.copytree(src, dest) -def copy_with_logging(src: pathlib.Path, dest: pathlib.Path, *, logger: logging.Logger): +def copy_with_logging(src: pathlib.Path, dest: pathlib.Path, *, logger: logging.Logger) -> None: """Call shutil.copy() with logging""" if dest.is_dir(): logger.info("Copying %s -> %s", str(src), str(dest / src.name)) From 823f8f1b38c24600ca938f1b19e9bd8d8db2fb35 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 18:15:58 -0700 Subject: [PATCH 17/61] Fix formatting --- python-package/packager/pep517.py | 6 +++--- python-package/packager/util.py | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/python-package/packager/pep517.py b/python-package/packager/pep517.py index eef2c9102256..595d49f7339c 100644 --- a/python-package/packager/pep517.py +++ b/python-package/packager/pep517.py @@ -10,7 +10,7 @@ import sysconfig import tempfile from contextlib import contextmanager -from typing import Any, Dict, Iterator, Optional, Union, List +from typing import Any, Dict, Iterator, List, Optional, Union import hatchling.build @@ -20,7 +20,7 @@ @contextmanager -def cd(path: Union[str, pathlib.Path]) -> Iterator[str]: +def cd(path: Union[str, pathlib.Path]) -> Iterator[str]: # pylint: disable=C0103 """Temporarily change working directory""" if isinstance(path, pathlib.Path): path = str(path) @@ -74,7 +74,7 @@ def write_hatch_config(dest_dir: pathlib.Path, *, logger: logging.Logger) -> Non ) hatch_build_file = dest_dir / "hatch_build.py" logger.info("Writing %s", str(hatch_build_file)) - with open(hatch_build_file, "w") as f: + with open(hatch_build_file, "w", encoding="utf-8") as f: f.write(template.format(tag=get_tag())) diff --git a/python-package/packager/util.py b/python-package/packager/util.py index 9569d22c1962..eb6fa01869e3 100644 --- a/python-package/packager/util.py +++ b/python-package/packager/util.py @@ -14,7 +14,9 @@ def copytree_with_logging( shutil.copytree(src, dest) -def copy_with_logging(src: pathlib.Path, dest: pathlib.Path, *, logger: logging.Logger) -> None: +def copy_with_logging( + src: pathlib.Path, dest: pathlib.Path, *, logger: logging.Logger +) -> None: """Call shutil.copy() with logging""" if dest.is_dir(): logger.info("Copying %s -> %s", str(src), str(dest / src.name)) From d2e5a65e17722b7a2481f06ca19d250edc7f51f9 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 19:11:18 -0700 Subject: [PATCH 18/61] Support CMake args --- python-package/packager/build_config.py | 46 +++++++++++++++++++++++++ python-package/packager/nativelib.py | 34 +++++++++++++++--- python-package/packager/pep517.py | 22 +++++++----- python-package/xgboost/config.py | 2 +- python-package/xgboost/plotting.py | 6 ++-- 5 files changed, 93 insertions(+), 17 deletions(-) create mode 100644 python-package/packager/build_config.py diff --git a/python-package/packager/build_config.py b/python-package/packager/build_config.py new file mode 100644 index 000000000000..3637e9d109bc --- /dev/null +++ b/python-package/packager/build_config.py @@ -0,0 +1,46 @@ +"""Build configuration""" +import dataclasses +from typing import Any, Dict, Optional + + +@dataclasses.dataclass +class BuildConfiguration: # pylint: disable=R0902 + """Configurations use when building libxgboost""" + + use_openmp: bool = True + hide_cxx_symbols: bool = True + use_system_libxgboost: bool = False + use_cuda: bool = False + use_nccl: bool = False + use_hdfs: bool = False + use_azure: bool = False + use_s3: bool = False + plugin_dense_parser: bool = False + + def _set_config_setting(self, config_settings: Dict[str, Any], field_name: str): + if field_name in config_settings: + setattr( + self, + field_name, + ( + config_settings[field_name] + in ["TRUE", "True", "true", "1", "On", "ON", "on"] + ), + ) + + def update(self, config_settings: Optional[Dict[str, Any]]): + """Parse config_settings from Pip (or other PEP 517 frontend)""" + if config_settings is not None: + for field_name in [x.name for x in dataclasses.fields(self)]: + self._set_config_setting(config_settings, field_name) + + def get_cmake_args(self): + """Convert build configuration to CMake args""" + cmake_args = [] + for field_name in [x.name for x in dataclasses.fields(self)]: + if field_name == "use_system_libxgboost": + continue + cmake_option = field_name.upper() + cmake_value = "ON" if getattr(self, field_name) else "OFF" + cmake_args.append(f"-D{cmake_option}={cmake_value}") + return cmake_args diff --git a/python-package/packager/nativelib.py b/python-package/packager/nativelib.py index 988422285205..96cc6a60788a 100644 --- a/python-package/packager/nativelib.py +++ b/python-package/packager/nativelib.py @@ -6,8 +6,11 @@ import pathlib import shutil import subprocess +import sys from platform import system +from .build_config import BuildConfiguration + def _lib_name() -> str: """Return platform dependent shared object name.""" @@ -26,6 +29,7 @@ def build_libxgboost( cpp_src_dir: pathlib.Path, *, build_dir: pathlib.Path, + build_config: BuildConfiguration, ) -> pathlib.Path: """Build libxgboost in a temporary directory and obtain the path to built libxgboost""" logger = logging.getLogger("xgboost.packager.build_libxgboost") @@ -50,10 +54,14 @@ def build_libxgboost( ) generator = "-GNinja" if build_tool == "ninja" else "-GUnix Makefiles" - cmake_cmd = ["cmake", str(cpp_src_dir), generator] - cmake_cmd.append("-DKEEP_BUILD_ARTIFACTS_IN_BINARY_DIR=ON") + cmake_cmd = [ + "cmake", + str(cpp_src_dir), + generator, + "-DKEEP_BUILD_ARTIFACTS_IN_BINARY_DIR=ON", + ] + cmake_cmd.extend(build_config.get_cmake_args()) - # TODO(hcho3): handle CMake args logger.info("CMake args: %s", str(cmake_cmd)) subprocess.check_call(cmake_cmd, cwd=build_dir) @@ -65,7 +73,10 @@ def build_libxgboost( def locate_or_build_libxgboost( - toplevel_dir: pathlib.Path, *, build_dir: pathlib.Path + toplevel_dir: pathlib.Path, + *, + build_dir: pathlib.Path, + build_config: BuildConfiguration, ) -> pathlib.Path: """Locate libxgboost; if not exist, build it""" logger = logging.getLogger("xgboost.packager.locate_or_build_libxgboost") @@ -74,6 +85,19 @@ def locate_or_build_libxgboost( if libxgboost.exists(): logger.info("Found %s at %s", libxgboost.name, str(libxgboost.parent)) return libxgboost + if build_config.use_system_libxgboost: + # Find libxgboost from system prefix + sys_prefix = pathlib.Path(sys.prefix).absolute().resolve() + libxgboost = sys_prefix / "lib" / _lib_name() + if not libxgboost.exists(): + raise AssertionError( + f"use_system_libxgboost was specified but {_lib_name()} is " + f"not found in {libxgboost.parent}" + ) + + logger.info("Using system XGBoost: %s", str(libxgboost)) + return libxgboost + if toplevel_dir.joinpath("cpp_src").exists(): # Source distribution; all C++ source files to be found in cpp_src/ cpp_src_dir = toplevel_dir.joinpath("cpp_src") @@ -82,4 +106,4 @@ def locate_or_build_libxgboost( cpp_src_dir = toplevel_dir.parent if not cpp_src_dir.joinpath("CMakeLists.txt").exists(): raise RuntimeError(f"Did not find CMakeLists.txt from {cpp_src_dir}") - return build_libxgboost(cpp_src_dir, build_dir=build_dir) + return build_libxgboost(cpp_src_dir, build_dir=build_dir, build_config=build_config) diff --git a/python-package/packager/pep517.py b/python-package/packager/pep517.py index 595d49f7339c..c9b7d729d139 100644 --- a/python-package/packager/pep517.py +++ b/python-package/packager/pep517.py @@ -14,6 +14,7 @@ import hatchling.build +from .build_config import BuildConfiguration from .nativelib import locate_or_build_libxgboost from .sdist import copy_cpp_src_tree from .util import copy_with_logging, copytree_with_logging @@ -86,11 +87,8 @@ def build_wheel( """Build a wheel""" logger = logging.getLogger("xgboost.packager.build_wheel") - if config_settings: - raise NotImplementedError( - f"XGBoost's custom build backend doesn't support config_settings option." - f"{config_settings=}" - ) + build_config = BuildConfiguration() + build_config.update(config_settings) # Create tempdir with Python package + libxgboost with tempfile.TemporaryDirectory() as td: @@ -110,7 +108,9 @@ def build_wheel( copytree_with_logging(TOPLEVEL_DIR / "xgboost", pkg_path, logger=logger) lib_path = pkg_path / "lib" lib_path.mkdir() - libxgboost = locate_or_build_libxgboost(TOPLEVEL_DIR, build_dir=build_dir) + libxgboost = locate_or_build_libxgboost( + TOPLEVEL_DIR, build_dir=build_dir, build_config=build_config + ) copy_with_logging(libxgboost, lib_path, logger=logger) with cd(workspace): @@ -129,8 +129,8 @@ def build_sdist( if config_settings: raise NotImplementedError( - f"XGBoost's custom build backend doesn't support config_settings option." - f"{config_settings=}" + "XGBoost's custom build backend doesn't support config_settings option " + f"when building sdist. {config_settings=}" ) cpp_src_dir = TOPLEVEL_DIR.parent @@ -171,6 +171,12 @@ def build_editable( """Build an editable installation. We mostly delegate to Hatchling.""" logger = logging.getLogger("xgboost.packager.build_editable") + if config_settings: + raise NotImplementedError( + "XGBoost's custom build backend doesn't support config_settings option " + f"when building editable installation. {config_settings=}" + ) + write_hatch_config(TOPLEVEL_DIR, logger=logger) return hatchling.build.build_editable( wheel_directory, config_settings, metadata_directory diff --git a/python-package/xgboost/config.py b/python-package/xgboost/config.py index c08a13150508..1691d473fa70 100644 --- a/python-package/xgboost/config.py +++ b/python-package/xgboost/config.py @@ -16,7 +16,7 @@ def config_doc( extra_note: Optional[str] = None, parameters: Optional[str] = None, returns: Optional[str] = None, - see_also: Optional[str] = None + see_also: Optional[str] = None, ) -> Callable[[_F], _F]: """Decorator to format docstring for config functions. diff --git a/python-package/xgboost/plotting.py b/python-package/xgboost/plotting.py index 71058e8c952d..d9eb14d0f600 100644 --- a/python-package/xgboost/plotting.py +++ b/python-package/xgboost/plotting.py @@ -30,7 +30,7 @@ def plot_importance( grid: bool = True, show_values: bool = True, values_format: str = "{v}", - **kwargs: Any + **kwargs: Any, ) -> Axes: """Plot importance based on fitted trees. @@ -155,7 +155,7 @@ def to_graphviz( no_color: Optional[str] = None, condition_node_params: Optional[dict] = None, leaf_node_params: Optional[dict] = None, - **kwargs: Any + **kwargs: Any, ) -> GraphvizSource: """Convert specified tree to graphviz instance. IPython can automatically plot the returned graphviz instance. Otherwise, you should call .render() method @@ -250,7 +250,7 @@ def plot_tree( num_trees: int = 0, rankdir: Optional[str] = None, ax: Optional[Axes] = None, - **kwargs: Any + **kwargs: Any, ) -> Axes: """Plot specified tree. From c5460be96138b5aa2959892f05599d3bb8555f41 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 19:57:23 -0700 Subject: [PATCH 19/61] More formatting fix --- python-package/packager/build_config.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/python-package/packager/build_config.py b/python-package/packager/build_config.py index 3637e9d109bc..135d3ca70549 100644 --- a/python-package/packager/build_config.py +++ b/python-package/packager/build_config.py @@ -1,6 +1,6 @@ """Build configuration""" import dataclasses -from typing import Any, Dict, Optional +from typing import Any, Dict, List, Optional @dataclasses.dataclass @@ -17,7 +17,9 @@ class BuildConfiguration: # pylint: disable=R0902 use_s3: bool = False plugin_dense_parser: bool = False - def _set_config_setting(self, config_settings: Dict[str, Any], field_name: str): + def _set_config_setting( + self, config_settings: Dict[str, Any], field_name: str + ) -> None: if field_name in config_settings: setattr( self, @@ -28,13 +30,13 @@ def _set_config_setting(self, config_settings: Dict[str, Any], field_name: str): ), ) - def update(self, config_settings: Optional[Dict[str, Any]]): + def update(self, config_settings: Optional[Dict[str, Any]]) -> None: """Parse config_settings from Pip (or other PEP 517 frontend)""" if config_settings is not None: for field_name in [x.name for x in dataclasses.fields(self)]: self._set_config_setting(config_settings, field_name) - def get_cmake_args(self): + def get_cmake_args(self) -> List[str]: """Convert build configuration to CMake args""" cmake_args = [] for field_name in [x.name for x in dataclasses.fields(self)]: From 9b117780d9e68d17b185f65a4ed2978f010ed9c2 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 20:00:54 -0700 Subject: [PATCH 20/61] Add URL for build doc --- python-package/packager/nativelib.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python-package/packager/nativelib.py b/python-package/packager/nativelib.py index 96cc6a60788a..3c1f2aeb1f1a 100644 --- a/python-package/packager/nativelib.py +++ b/python-package/packager/nativelib.py @@ -50,7 +50,7 @@ def build_libxgboost( "Installing from sdist is not supported on Windows. You have two alternatives:\n" "1. Install XGBoost from the official wheel (recommended): pip install xgboost\n" "2. Build XGBoost from the source by running CMake at the project root folder. See " - "documentation for details." + "https://xgboost.readthedocs.io/en/latest/build.html for details." ) generator = "-GNinja" if build_tool == "ninja" else "-GUnix Makefiles" From 9260613ba90e82c15660b79d8d871548231ef717 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 20:02:24 -0700 Subject: [PATCH 21/61] Install hatchling when linting --- tests/ci_build/conda_env/python_lint.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/ci_build/conda_env/python_lint.yml b/tests/ci_build/conda_env/python_lint.yml index a64f649a21a4..3d42dfaf3ae6 100644 --- a/tests/ci_build/conda_env/python_lint.yml +++ b/tests/ci_build/conda_env/python_lint.yml @@ -18,6 +18,7 @@ dependencies: - cloudpickle - pytest - hypothesis +- hatchling - pip: # TODO: Replace it with pyspark>=3.4 once 3.4 released. - https://ml-team-public-read.s3.us-west-2.amazonaws.com/pyspark-3.4.0.dev0.tar.gz From d7799c844065e35f25d8fdff7e87608b42e5afc2 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 20:07:23 -0700 Subject: [PATCH 22/61] Remove setup.py sdist from CI --- .github/workflows/python_tests.yml | 7 +++++-- tests/ci_build/conda_env/sdist_test.yml | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/python_tests.yml b/.github/workflows/python_tests.yml index 0d8e6d653428..603e8af95083 100644 --- a/.github/workflows/python_tests.yml +++ b/.github/workflows/python_tests.yml @@ -65,7 +65,7 @@ jobs: run: | cd python-package python --version - python setup.py sdist + python -m build --sdist pip install -v ./dist/xgboost-*.tar.gz cd .. python -c 'import xgboost' @@ -92,6 +92,9 @@ jobs: auto-update-conda: true python-version: ${{ matrix.python-version }} activate-environment: test + - name: Install build + run: | + conda install -c conda-forge build - name: Display Conda env run: | conda info @@ -100,7 +103,7 @@ jobs: run: | cd python-package python --version - python setup.py sdist + python -m build --sdist pip install -v ./dist/xgboost-*.tar.gz cd .. python -c 'import xgboost' diff --git a/tests/ci_build/conda_env/sdist_test.yml b/tests/ci_build/conda_env/sdist_test.yml index acc4607ad722..67a9324f7006 100644 --- a/tests/ci_build/conda_env/sdist_test.yml +++ b/tests/ci_build/conda_env/sdist_test.yml @@ -8,5 +8,6 @@ dependencies: - wheel - cmake - ninja +- python-build - c-compiler - cxx-compiler From 2c73f754ce2aab39b804a1aaee74d33903a056a6 Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Thu, 13 Apr 2023 20:37:04 -0700 Subject: [PATCH 23/61] Support building libxgboost in Windows --- python-package/packager/nativelib.py | 76 ++++++++++++++++++---------- 1 file changed, 50 insertions(+), 26 deletions(-) diff --git a/python-package/packager/nativelib.py b/python-package/packager/nativelib.py index 3c1f2aeb1f1a..8637279b8fc2 100644 --- a/python-package/packager/nativelib.py +++ b/python-package/packager/nativelib.py @@ -8,6 +8,7 @@ import subprocess import sys from platform import system +from typing import Optional from .build_config import BuildConfiguration @@ -40,34 +41,57 @@ def build_libxgboost( "Building %s from the C++ source files in %s...", _lib_name(), str(cpp_src_dir) ) - if shutil.which("ninja"): - build_tool = "ninja" - else: - build_tool = "make" + def _build(*, generator: str, build_tool: Optional[str] = None) -> None: + cmake_cmd = [ + "cmake", + str(cpp_src_dir), + generator, + "-DKEEP_BUILD_ARTIFACTS_IN_BINARY_DIR=ON", + ] + cmake_cmd.extend(build_config.get_cmake_args()) + + logger.info("CMake args: %s", str(cmake_cmd)) + subprocess.check_call(cmake_cmd, cwd=build_dir) + + if system() == "Windows": + subprocess.check_call( + ["cmake", "--build", ".", "--config", "Release"], cwd=build_dir + ) + else: + nproc = os.cpu_count() + assert build_tool is not None + subprocess.check_call([build_tool, f"-j{nproc}"], cwd=build_dir) if system() == "Windows": - raise NotImplementedError( - "Installing from sdist is not supported on Windows. You have two alternatives:\n" - "1. Install XGBoost from the official wheel (recommended): pip install xgboost\n" - "2. Build XGBoost from the source by running CMake at the project root folder. See " - "https://xgboost.readthedocs.io/en/latest/build.html for details." - ) - - generator = "-GNinja" if build_tool == "ninja" else "-GUnix Makefiles" - cmake_cmd = [ - "cmake", - str(cpp_src_dir), - generator, - "-DKEEP_BUILD_ARTIFACTS_IN_BINARY_DIR=ON", - ] - cmake_cmd.extend(build_config.get_cmake_args()) - - logger.info("CMake args: %s", str(cmake_cmd)) - subprocess.check_call(cmake_cmd, cwd=build_dir) - - nproc = os.cpu_count() - assert build_tool is not None - subprocess.check_call([build_tool, f"-j{nproc}"], cwd=build_dir) + for generator in ( + "-GVisual Studio 17 2022", + "-GVisual Studio 16 2019", + "-GVisual Studio 15 2017", + "-GMinGW Makefiles", + ): + try: + _build(generator=generator) + logger.info( + "Successfully built %s using generator %s", _lib_name(), generator + ) + break + except subprocess.CalledProcessError as e: + logger.info( + "Tried building with generator %s but failed with exception %s", + generator, + str(e), + ) + # Empty build directory + shutil.rmtree(build_dir) + build_dir.mkdir() + continue + else: + if shutil.which("ninja"): + build_tool = "ninja" + else: + build_tool = "make" + generator = "-GNinja" if build_tool == "ninja" else "-GUnix Makefiles" + _build(generator=generator, build_tool=build_tool) return build_dir / "lib" / _lib_name() From 10482b91004754e78a6a1b0ab2d82904a764117b Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 21:13:47 -0700 Subject: [PATCH 24/61] Remove all uses of setup.py from CI --- .github/workflows/python_tests.yml | 6 +++--- tests/buildkite/build-cpu-arm64.sh | 2 +- tests/buildkite/build-cuda.sh | 2 +- tests/buildkite/build-win64-gpu.ps1 | 2 +- tests/ci_build/build_python_wheels.sh | 2 +- tests/ci_build/test_python.sh | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/python_tests.yml b/.github/workflows/python_tests.yml index 603e8af95083..2ce161641700 100644 --- a/.github/workflows/python_tests.yml +++ b/.github/workflows/python_tests.yml @@ -150,7 +150,7 @@ jobs: run: | cd python-package python --version - python setup.py install + pip install -v . - name: Test Python package run: | @@ -197,7 +197,7 @@ jobs: run: | cd python-package python --version - python setup.py bdist_wheel --universal + pip wheel -v . --wheel-dir dist/ pip install ./dist/*.whl - name: Test Python package @@ -241,7 +241,7 @@ jobs: run: | cd python-package python --version - python setup.py install + pip install -v . - name: Test Python package run: | diff --git a/tests/buildkite/build-cpu-arm64.sh b/tests/buildkite/build-cpu-arm64.sh index 1a95a880a515..47ead65e4c0c 100755 --- a/tests/buildkite/build-cpu-arm64.sh +++ b/tests/buildkite/build-cpu-arm64.sh @@ -18,7 +18,7 @@ $command_wrapper bash -c "cd build && ctest --extra-verbose" echo "--- Build binary wheel" $command_wrapper bash -c \ - "cd python-package && rm -rf dist/* && python setup.py bdist_wheel --universal" + "cd python-package && rm -rf dist/* && pip wheel -v . --wheel-dir dist/" $command_wrapper python tests/ci_build/rename_whl.py python-package/dist/*.whl \ ${BUILDKITE_COMMIT} ${WHEEL_TAG} diff --git a/tests/buildkite/build-cuda.sh b/tests/buildkite/build-cuda.sh index b25345b1bbb1..f8db963d0fb3 100755 --- a/tests/buildkite/build-cuda.sh +++ b/tests/buildkite/build-cuda.sh @@ -27,7 +27,7 @@ $command_wrapper tests/ci_build/build_via_cmake.sh -DCMAKE_PREFIX_PATH=/opt/grpc -DNCCL_LIBRARY=/workspace/libnccl_static.a ${arch_flag} echo "--- Build binary wheel" $command_wrapper bash -c \ - "cd python-package && rm -rf dist/* && python setup.py bdist_wheel --universal" + "cd python-package && rm -rf dist/* && pip wheel -v . --wheel-dir dist/" $command_wrapper python tests/ci_build/rename_whl.py python-package/dist/*.whl \ ${BUILDKITE_COMMIT} ${WHEEL_TAG} diff --git a/tests/buildkite/build-win64-gpu.ps1 b/tests/buildkite/build-win64-gpu.ps1 index 05d7aefb9048..8d59742a0e38 100644 --- a/tests/buildkite/build-win64-gpu.ps1 +++ b/tests/buildkite/build-win64-gpu.ps1 @@ -24,7 +24,7 @@ if ($LASTEXITCODE -ne 0) { throw "Last command failed" } Write-Host "--- Build binary wheel" cd ../python-package conda activate -& python setup.py bdist_wheel --universal +& pip wheel -v . --wheel-dir dist/ Get-ChildItem . -Filter dist/*.whl | Foreach-Object { & python ../tests/ci_build/rename_whl.py $_.FullName $Env:BUILDKITE_COMMIT win_amd64 diff --git a/tests/ci_build/build_python_wheels.sh b/tests/ci_build/build_python_wheels.sh index d91df2286fd2..205b3b695fcd 100644 --- a/tests/ci_build/build_python_wheels.sh +++ b/tests/ci_build/build_python_wheels.sh @@ -26,7 +26,7 @@ if [[ "$platform_id" == macosx_* ]]; then # cibuildwheel will take care of cross-compilation. wheel_tag=macosx_12_0_arm64 cpython_ver=38 - setup_env_var='CIBW_TARGET_OSX_ARM64=1' # extra flag to be passed to setup.py + setup_env_var='CIBW_TARGET_OSX_ARM64=1' # extra flag to be passed to xgboost.packager backend export PYTHON_CROSSENV=1 export MACOSX_DEPLOYMENT_TARGET=12.0 #OPENMP_URL="https://anaconda.org/conda-forge/llvm-openmp/11.1.0/download/osx-arm64/llvm-openmp-11.1.0-hf3c4609_1.tar.bz2" diff --git a/tests/ci_build/test_python.sh b/tests/ci_build/test_python.sh index 7375b4c9f872..a70b2796130f 100755 --- a/tests/ci_build/test_python.sh +++ b/tests/ci_build/test_python.sh @@ -28,7 +28,7 @@ function install_xgboost { then pushd . cd python-package - python setup.py install --user + pip install --user -v . popd fi } From da2e8bb15d5a304255581f12b2faa2710d481f10 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 21:17:33 -0700 Subject: [PATCH 25/61] Support Apple Silicon + cibuildwheel --- python-package/packager/nativelib.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/python-package/packager/nativelib.py b/python-package/packager/nativelib.py index 8637279b8fc2..e95b470bd5c0 100644 --- a/python-package/packager/nativelib.py +++ b/python-package/packager/nativelib.py @@ -50,6 +50,12 @@ def _build(*, generator: str, build_tool: Optional[str] = None) -> None: ] cmake_cmd.extend(build_config.get_cmake_args()) + # Flag for cross-compiling for Apple Silicon + # We use environment variable because it's the only way to pass down custom flags + # through the cibuildwheel package, which calls `pip wheel` command. + if "CIBW_TARGET_OSX_ARM64" in os.environ: + cmake_cmd.append("-DCMAKE_OSX_ARCHITECTURES=arm64") + logger.info("CMake args: %s", str(cmake_cmd)) subprocess.check_call(cmake_cmd, cwd=build_dir) From 3eec9f79e9e3cf016d6a30238b8a76580667634f Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Thu, 13 Apr 2023 20:55:54 -0700 Subject: [PATCH 26/61] Fallback if OpenMP isn't available --- python-package/packager/nativelib.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/python-package/packager/nativelib.py b/python-package/packager/nativelib.py index e95b470bd5c0..823dcd4d0c24 100644 --- a/python-package/packager/nativelib.py +++ b/python-package/packager/nativelib.py @@ -97,7 +97,12 @@ def _build(*, generator: str, build_tool: Optional[str] = None) -> None: else: build_tool = "make" generator = "-GNinja" if build_tool == "ninja" else "-GUnix Makefiles" - _build(generator=generator, build_tool=build_tool) + try: + _build(generator=generator, build_tool=build_tool) + except subprocess.CalledProcessError as e: + logger.info("Failed to build with OpenMP. Exception: %s", str(e)) + build_config.use_openmp = False + _build(generator=generator, build_tool=build_tool) return build_dir / "lib" / _lib_name() From 7ee3ff8732e44e883b0da0492d2f3a3a3907124c Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Thu, 13 Apr 2023 21:08:10 -0700 Subject: [PATCH 27/61] Check libxgboost if editable --- python-package/packager/nativelib.py | 20 +++++++++++++++++--- python-package/packager/pep517.py | 8 +++++++- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/python-package/packager/nativelib.py b/python-package/packager/nativelib.py index 823dcd4d0c24..35aa14e3b2db 100644 --- a/python-package/packager/nativelib.py +++ b/python-package/packager/nativelib.py @@ -107,6 +107,21 @@ def _build(*, generator: str, build_tool: Optional[str] = None) -> None: return build_dir / "lib" / _lib_name() +def locate_local_libxgboost( + toplevel_dir: pathlib.Path, + *, + logger: logging.Logger, +) -> Optional[pathlib.Path]: + """ + Locate libxgboost from the local project directory's lib/ subdirectory. + """ + libxgboost = toplevel_dir.parent / "lib" / _lib_name() + if libxgboost.exists(): + logger.info("Found %s at %s", libxgboost.name, str(libxgboost.parent)) + return libxgboost + return None + + def locate_or_build_libxgboost( toplevel_dir: pathlib.Path, *, @@ -116,9 +131,8 @@ def locate_or_build_libxgboost( """Locate libxgboost; if not exist, build it""" logger = logging.getLogger("xgboost.packager.locate_or_build_libxgboost") - libxgboost = toplevel_dir.parent / "lib" / _lib_name() - if libxgboost.exists(): - logger.info("Found %s at %s", libxgboost.name, str(libxgboost.parent)) + libxgboost = locate_local_libxgboost(toplevel_dir, logger=logger) + if libxgboost is not None: return libxgboost if build_config.use_system_libxgboost: # Find libxgboost from system prefix diff --git a/python-package/packager/pep517.py b/python-package/packager/pep517.py index c9b7d729d139..cd91515cfcd5 100644 --- a/python-package/packager/pep517.py +++ b/python-package/packager/pep517.py @@ -15,7 +15,7 @@ import hatchling.build from .build_config import BuildConfiguration -from .nativelib import locate_or_build_libxgboost +from .nativelib import locate_local_libxgboost, locate_or_build_libxgboost from .sdist import copy_cpp_src_tree from .util import copy_with_logging, copytree_with_logging @@ -177,6 +177,12 @@ def build_editable( f"when building editable installation. {config_settings=}" ) + if locate_local_libxgboost(TOPLEVEL_DIR, logger=logger) is None: + raise AssertionError( + "To use the editable installation, first build libxgboost with CMake. " + "See https://xgboost.readthedocs.io/en/latest/build.html for detailed instructions." + ) + write_hatch_config(TOPLEVEL_DIR, logger=logger) return hatchling.build.build_editable( wheel_directory, config_settings, metadata_directory From 63d88dc59f25776ccec856af12f1e2b4f5413273 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 21:28:47 -0700 Subject: [PATCH 28/61] On Conda, it's python-build --- .github/workflows/python_tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python_tests.yml b/.github/workflows/python_tests.yml index 2ce161641700..78a17d3f705a 100644 --- a/.github/workflows/python_tests.yml +++ b/.github/workflows/python_tests.yml @@ -94,7 +94,7 @@ jobs: activate-environment: test - name: Install build run: | - conda install -c conda-forge build + conda install -c conda-forge python-build - name: Display Conda env run: | conda info From cea68998264335032c56e4379d605a51b9e2642c Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 22:12:08 -0700 Subject: [PATCH 29/61] Update change_version.py script to update pyproject.toml --- python-package/pyproject.toml | 2 +- tests/ci_build/change_version.py | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/python-package/pyproject.toml b/python-package/pyproject.toml index 15a87df1ebf5..8f120df5dcd0 100644 --- a/python-package/pyproject.toml +++ b/python-package/pyproject.toml @@ -7,7 +7,7 @@ build-backend = "packager.pep517" [project] name = "xgboost" -version = "2.0.0-dev0" +version = "2.0.0-dev" authors = [ {name = "Hyunsu Cho", email = "chohyu01@cs.washington.edu"}, {name = "Jiaming Yuan", email = "jm.yuan@outlook.com"} diff --git a/tests/ci_build/change_version.py b/tests/ci_build/change_version.py index 62cb894dcb75..ac3d71f060f5 100644 --- a/tests/ci_build/change_version.py +++ b/tests/ci_build/change_version.py @@ -40,14 +40,24 @@ def pypkg( major: int, minor: int, patch: int, rc: int, is_rc: bool, is_dev: bool ) -> None: version = f"{major}.{minor}.{patch}" - pyver_path = os.path.join("xgboost", "VERSION") pyver = version if is_rc: pyver = pyver + f"rc{rc}" if is_dev: pyver = pyver + "-dev" + + pyver_path = os.path.join("xgboost", "VERSION") with open(pyver_path, "w") as fd: - fd.write(pyver) + fd.write(pyver + "\n") + + pyprj_path = os.path.join("pyproject.toml") + with open(pyprj_path, "r") as fd: + pyprj = fd.read() + matched = re.search('version = "' + r"([0-9]+\.[0-9]+\.[0-9]+.*)" + '"', pyprj) + print(matched.group(1)) + pyprj = pyprj[: matched.start(1)] + pyver + pyprj[matched.end(1) :] + with open(pyprj_path, "w") as fd: + pyprj = fd.write(pyprj) @cd(R_PACKAGE) From 9d002fc44ff69cb0a38d039ebf0123b849ab2eea Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 22:14:47 -0700 Subject: [PATCH 30/61] Remove setup.py from dev script --- dev/release-artifacts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/release-artifacts.py b/dev/release-artifacts.py index 18c317a915e4..eab64ff0c9a3 100644 --- a/dev/release-artifacts.py +++ b/dev/release-artifacts.py @@ -105,7 +105,7 @@ def make_pysrc_wheel(release: str, outdir: str) -> None: os.mkdir(dist) with DirectoryExcursion(os.path.join(ROOT, "python-package")): - subprocess.check_call(["python", "setup.py", "sdist"]) + subprocess.check_call(["python", "-m", "build", "--sdist"]) src = os.path.join(DIST, f"xgboost-{release}.tar.gz") subprocess.check_call(["twine", "check", src]) shutil.move(src, os.path.join(dist, f"xgboost-{release}.tar.gz")) From c0b49990c2255e053eedc597c409ca311da500fe Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Thu, 13 Apr 2023 23:48:26 -0700 Subject: [PATCH 31/61] Update docs --- doc/build.rst | 126 ++++++++++-------------- doc/contrib/ci.rst | 5 +- plugin/federated/README.md | 2 +- python-package/packager/build_config.py | 4 +- 4 files changed, 59 insertions(+), 78 deletions(-) diff --git a/doc/build.rst b/doc/build.rst index 53d9a3209dc4..7a2e81a312c0 100644 --- a/doc/build.rst +++ b/doc/build.rst @@ -152,11 +152,11 @@ On Windows, run CMake as follows: mkdir build cd build - cmake .. -G"Visual Studio 14 2015 Win64" -DUSE_CUDA=ON + cmake .. -G"Visual Studio 17 2022" -A x64 -DUSE_CUDA=ON (Change the ``-G`` option appropriately if you have a different version of Visual Studio installed.) -The above cmake configuration run will create an ``xgboost.sln`` solution file in the build directory. Build this solution in release mode as a x64 build, either from Visual studio or from command line: +The above cmake configuration run will create an ``xgboost.sln`` solution file in the build directory. Build this solution in Release mode, either from Visual studio or from command line: .. code-block:: bash @@ -176,109 +176,89 @@ Building Python Package with Default Toolchains =============================================== There are several ways to build and install the package from source: -1. Use Python setuptools directly +1. Install the Python package directly - The XGBoost Python package supports most of the setuptools commands, here is a list of tested commands: + You can navigate to ``python-package/`` directory and install the Python package directly + by running - .. code-block:: bash - - python setup.py install # Install the XGBoost to your current Python environment. - python setup.py build # Build the Python package. - python setup.py build_ext # Build only the C++ core. - python setup.py sdist # Create a source distribution - python setup.py bdist # Create a binary distribution - python setup.py bdist_wheel # Create a binary distribution with wheel format - - Running ``python setup.py install`` will compile XGBoost using default CMake flags. For - passing additional compilation options, append the flags to the command. For example, - to enable CUDA acceleration and NCCL (distributed GPU) support: - - .. code-block:: bash - - python setup.py install --use-cuda --use-nccl - - Please refer to ``setup.py`` for a complete list of available options. Some other - options used for development are only available for using CMake directly. See next - section on how to use CMake with setuptools manually. + .. code-block:: console - You can install the created distribution packages using pip. For example, after running - ``sdist`` setuptools command, a tar ball similar to ``xgboost-1.0.0.tar.gz`` will be - created under the ``dist`` directory. Then you can install it by invoking the following - command under ``dist`` directory: + $ pip install -v . - .. code-block:: bash - - # under python-package directory - cd dist - pip install ./xgboost-1.0.0.tar.gz + which will compile XGBoost's native (C++) code using default CMake flags. + To enable additional compilation options, pass corresponding ``--config-settings``: + .. code-block:: console - For details about these commands, please refer to the official document of `setuptools - `_, or just Google "how to install Python - package from source". XGBoost Python package follows the general convention. - Setuptools is usually available with your Python distribution, if not you can install it - via system command. For example on Debian or Ubuntu: - - .. code-block:: bash + $ pip install -v . --config-settings use_cuda=True --config-settings use_nccl=True - sudo apt-get install python-setuptools + Here are the available options for ``--config-settings``: + .. literalinclude:: ../python-package/packager/build_config.py + :language: python + :start-at: @dataclasses.dataclass + :end-before: def _set_config_setting( - For cleaning up the directory after running above commands, ``python setup.py clean`` is - not sufficient. After copying out the build result, simply running ``git clean -xdf`` - under ``python-package`` is an efficient way to remove generated cache files. If you - find weird behaviors in Python build or running linter, it might be caused by those - cached files. + (The ``use_system_libxgboost`` option is special. See Item 3 below for + detailed description.) - For using develop command (editable installation), see next section. + .. note:: Verbose flag recommended - .. code-block:: - - python setup.py develop # Create a editable installation. - pip install -e . # Same as above, but carried out by pip. + As ``pip install .`` will build C++ code, it will take a while to complete. + To ensure that the build is progressing successfully, we suggest that + you add the verbose flag (``-v``) when invoking ``pip install``. 2. Build C++ core with CMake first - This is mostly for C++ developers who don't want to go through the hooks in Python - setuptools. You can build C++ library directly using CMake as described in above - sections. After compilation, a shared object (or called dynamic linked library, jargon - depending on your platform) will appear in XGBoost's source tree under ``lib/`` - directory. On Linux distributions it's ``lib/libxgboost.so``. From there all Python - setuptools commands will reuse that shared object instead of compiling it again. This - is especially convenient if you are using the editable installation, where the installed - package is simply a link to the source tree. We can perform rapid testing during - development. Here is a simple bash script does that: + This is mostly for C++ developers who iterate frequently on the C++ part of XGBoost. + You can build C++ library directly using CMake as described in :ref:`build_shared_lib`. + After compilation, a shared library (or called dynamic linked library; the jargon + depends on your platform) will appear in XGBoost's source tree under ``lib/`` + directory. On Linux distributions, the shared library is ``lib/libxgboost.so``. + The install script ``pip install .`` will reuse the shared library instead of compiling + it from scratch, making it quite fast to run. + + To further enable rapid development and iteration, we provide an **editable installation**. + In an editable installation, the installed package is simply a symbolic link to your + working copy of the XGBoost source code. So every changes you make to your source + directory will be immediately visible to the Python interpreter. Here is how to + install XGBoost as editable installation: .. code-block:: bash - # Under xgboost source tree. + # Under xgboost source directory mkdir build cd build - cmake .. - make -j$(nproc) + # Build shared library libxgboost.so + cmake .. -GNinja + ninja + # Install as editable installation cd ../python-package - pip install -e . # or equivalently python setup.py develop + pip install -e . 3. Use ``libxgboost.so`` on system path. - This is for distributing xgboost in a language independent manner, where - ``libxgboost.so`` is separately packaged with Python package. Assuming `libxgboost.so` - is already presented in system library path, which can be queried via: + This option is useful for package managers that wish to separately package + ``libxgboost.so`` and the XGBoost Python package. For example, Conda + publishes ``libxgboost`` (for the shared library) and ``py-xgboost`` + (for the Python package). + + To use this option, first make sure that ``libxgboost.so`` exists in the system library path: .. code-block:: python import sys - import os - os.path.join(sys.prefix, 'lib') + import pathlib + libpath = pathlib.Path(sys.prefix).joinpath("lib", "libxgboost.so") + assert libpath.exists() - Then one only needs to provide an user option when installing Python package to reuse the - shared object in system path: + Then pass ``use_system_libxgboost=True`` option to ``pip install``: .. code-block:: bash cd xgboost/python-package - python setup.py install --use-system-libxgboost + pip install . --config-settings use_system_libxgboost=True .. _python_mingw: @@ -297,7 +277,7 @@ So you may want to build XGBoost with GCC own your own risk. This presents some 2. ``-O3`` is OK. 3. ``-mtune=native`` is also OK. 4. Don't use ``-march=native`` gcc flag. Using it causes the Python interpreter to crash if the DLL was actually used. -5. You may need to provide the lib with the runtime libs. If ``mingw32/bin`` is not in ``PATH``, build a wheel (``python setup.py bdist_wheel``), open it with an archiver and put the needed dlls to the directory where ``xgboost.dll`` is situated. Then you can install the wheel with ``pip``. +5. You may need to provide the lib with the runtime libs. If ``mingw32/bin`` is not in ``PATH``, build a wheel (``pip wheel``), open it with an archiver and put the needed dlls to the directory where ``xgboost.dll`` is situated. Then you can install the wheel with ``pip``. ****************************** Building R Package From Source diff --git a/doc/contrib/ci.rst b/doc/contrib/ci.rst index 6073e646ad99..76e06de352fa 100644 --- a/doc/contrib/ci.rst +++ b/doc/contrib/ci.rst @@ -35,8 +35,9 @@ calls ``cibuildwheel`` to build the wheel. The ``cibuildwheel`` is a library tha suitable Python environment for each OS and processor target. Since we don't have Apple Silion machine in GitHub Actions, cross-compilation is needed; ``cibuildwheel`` takes care of the complex task of cross-compiling a Python wheel. (Note that ``cibuildwheel`` will call -``setup.py bdist_wheel``. Since XGBoost has a native library component, ``setup.py`` contains -a glue code to call CMake and a C++ compiler to build the native library on the fly.) +``pip wheel``. Since XGBoost has a native library component, we created a customized build +backend that hooks into ``pip``. The customized backend contains the glue code to compile the native +library on the fly.) ********************************************************* Reproduce CI testing environments using Docker containers diff --git a/plugin/federated/README.md b/plugin/federated/README.md index d83db6be1ee2..631c44cee26f 100644 --- a/plugin/federated/README.md +++ b/plugin/federated/README.md @@ -19,7 +19,7 @@ cmake .. -GNinja \ -DUSE_NCCL=ON ninja cd ../python-package -pip install -e . # or equivalently python setup.py develop +pip install -e . ``` If CMake fails to locate gRPC, you may need to pass `-DCMAKE_PREFIX_PATH=` to CMake. diff --git a/python-package/packager/build_config.py b/python-package/packager/build_config.py index 135d3ca70549..c45a5b8aa455 100644 --- a/python-package/packager/build_config.py +++ b/python-package/packager/build_config.py @@ -7,15 +7,15 @@ class BuildConfiguration: # pylint: disable=R0902 """Configurations use when building libxgboost""" - use_openmp: bool = True hide_cxx_symbols: bool = True - use_system_libxgboost: bool = False + use_openmp: bool = True use_cuda: bool = False use_nccl: bool = False use_hdfs: bool = False use_azure: bool = False use_s3: bool = False plugin_dense_parser: bool = False + use_system_libxgboost: bool = False def _set_config_setting( self, config_settings: Dict[str, Any], field_name: str From 3bf0357d7533733a6e5607e70b3c35c1e1f412f3 Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Thu, 13 Apr 2023 23:51:51 -0700 Subject: [PATCH 32/61] Integrate insert_vcomp140 into build backend --- python-package/packager/pep517.py | 7 ++ tests/buildkite/build-win64-gpu.ps1 | 6 -- tests/ci_build/insert_vcomp140.py | 102 ---------------------------- 3 files changed, 7 insertions(+), 108 deletions(-) delete mode 100644 tests/ci_build/insert_vcomp140.py diff --git a/python-package/packager/pep517.py b/python-package/packager/pep517.py index cd91515cfcd5..6af4d0a764e4 100644 --- a/python-package/packager/pep517.py +++ b/python-package/packager/pep517.py @@ -10,6 +10,7 @@ import sysconfig import tempfile from contextlib import contextmanager +from platform import system from typing import Any, Dict, Iterator, List, Optional, Union import hatchling.build @@ -112,6 +113,12 @@ def build_wheel( TOPLEVEL_DIR, build_dir=build_dir, build_config=build_config ) copy_with_logging(libxgboost, lib_path, logger=logger) + if system() == "Windows": + copy_with_logging( + pathlib.Path(r"C:\Windows\System32\vcomp140.dll"), + lib_path, + logger=logger, + ) with cd(workspace): wheel_name = hatchling.build.build_wheel( diff --git a/tests/buildkite/build-win64-gpu.ps1 b/tests/buildkite/build-win64-gpu.ps1 index 8d59742a0e38..f64995dd076b 100644 --- a/tests/buildkite/build-win64-gpu.ps1 +++ b/tests/buildkite/build-win64-gpu.ps1 @@ -31,12 +31,6 @@ Foreach-Object { if ($LASTEXITCODE -ne 0) { throw "Last command failed" } } -Write-Host "--- Insert vcomp140.dll (OpenMP runtime) into the wheel" -cd dist -Copy-Item -Path ../../tests/ci_build/insert_vcomp140.py -Destination . -& python insert_vcomp140.py *.whl -if ($LASTEXITCODE -ne 0) { throw "Last command failed" } - Write-Host "--- Upload Python wheel" cd ../.. Get-ChildItem . -Filter python-package/dist/*.whl | diff --git a/tests/ci_build/insert_vcomp140.py b/tests/ci_build/insert_vcomp140.py deleted file mode 100644 index cfa8d792dee2..000000000000 --- a/tests/ci_build/insert_vcomp140.py +++ /dev/null @@ -1,102 +0,0 @@ -import argparse -import base64 -import glob -import hashlib -import os -import pathlib -import re -import shutil -import tempfile - -VCOMP140_PATH = "C:\\Windows\\System32\\vcomp140.dll" - - -def get_sha256sum(path): - return ( - base64.urlsafe_b64encode(hashlib.sha256(open(path, "rb").read()).digest()) - .decode("latin1") - .rstrip("=") - ) - - -def update_record(*, wheel_content_dir, xgboost_version): - vcomp140_size = os.path.getsize(VCOMP140_PATH) - vcomp140_hash = get_sha256sum(VCOMP140_PATH) - - record_path = wheel_content_dir / pathlib.Path( - f"xgboost-{xgboost_version}.dist-info/RECORD" - ) - with open(record_path, "r") as f: - record_content = f.read() - record_content += f"xgboost-{xgboost_version}.data/data/xgboost/vcomp140.dll," - record_content += f"sha256={vcomp140_hash},{vcomp140_size}\n" - with open(record_path, "w") as f: - f.write(record_content) - - -def main(args): - candidates = list(sorted(glob.glob(args.wheel_path))) - for wheel_path in candidates: - print(f"Processing wheel {wheel_path}") - m = re.search(r"xgboost-(.*)\+.*-py3", wheel_path) - if not m: - raise ValueError(f"Wheel {wheel_path} has unexpected name") - version = m.group(1) - print(f" Detected version for {wheel_path}: {version}") - print(f" Inserting vcomp140.dll into {wheel_path}...") - with tempfile.TemporaryDirectory() as tempdir: - wheel_content_dir = pathlib.Path(tempdir) / "wheel_content" - print(f" Extract {wheel_path} into {wheel_content_dir}") - shutil.unpack_archive( - wheel_path, extract_dir=wheel_content_dir, format="zip" - ) - data_dir = wheel_content_dir / pathlib.Path( - f"xgboost-{version}.data/data/xgboost" - ) - data_dir.mkdir(parents=True, exist_ok=True) - - print(f" Copy {VCOMP140_PATH} -> {data_dir}") - shutil.copy(VCOMP140_PATH, data_dir) - - print(f" Update RECORD") - update_record(wheel_content_dir=wheel_content_dir, xgboost_version=version) - - print(f" Content of {wheel_content_dir}:") - for e in sorted(wheel_content_dir.rglob("*")): - if e.is_file(): - r = e.relative_to(wheel_content_dir) - print(f" {r}") - - print(f" Create new wheel...") - new_wheel_tmp_path = pathlib.Path(tempdir) / "new_wheel" - shutil.make_archive( - str(new_wheel_tmp_path.resolve()), - format="zip", - root_dir=wheel_content_dir, - ) - new_wheel_tmp_path = new_wheel_tmp_path.resolve().with_suffix(".zip") - new_wheel_tmp_path = new_wheel_tmp_path.rename( - new_wheel_tmp_path.with_suffix(".whl") - ) - print(f" Created new wheel {new_wheel_tmp_path}") - - # Rename the old wheel with suffix .bak - # The new wheel takes the name of the old wheel - wheel_path_obj = pathlib.Path(wheel_path).resolve() - backup_path = wheel_path_obj.with_suffix(".whl.bak") - print(f" Rename {wheel_path_obj} -> {backup_path}") - wheel_path_obj.replace(backup_path) - print(f" Rename {new_wheel_tmp_path} -> {wheel_path_obj}") - new_wheel_tmp_path.replace(wheel_path_obj) - - shutil.rmtree(wheel_content_dir) - - -if __name__ == "__main__": - parser = argparse.ArgumentParser() - parser.add_argument( - "wheel_path", type=str, help="Path to wheel (wildcard permitted)" - ) - args = parser.parse_args() - - main(args) From 702e4eadd81fd4b221f224a563f88a7207d9360b Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Fri, 14 Apr 2023 00:08:08 -0700 Subject: [PATCH 33/61] Ensure that vcomp140.dll exists --- python-package/packager/pep517.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/python-package/packager/pep517.py b/python-package/packager/pep517.py index 6af4d0a764e4..0569a7e15669 100644 --- a/python-package/packager/pep517.py +++ b/python-package/packager/pep517.py @@ -114,8 +114,15 @@ def build_wheel( ) copy_with_logging(libxgboost, lib_path, logger=logger) if system() == "Windows": + vcomp140_path = pathlib.Path(r"C:\Windows\System32\vcomp140.dll") + if not vcomp140_path.exists(): + raise RuntimeError( + "To build XGBoost from the source, please install Microsoft " + "Visual C++ Redistributable for Visual Studio 2015 at " + "https://www.microsoft.com/en-ca/download/details.aspx?id=48145." + ) copy_with_logging( - pathlib.Path(r"C:\Windows\System32\vcomp140.dll"), + vcomp140_path, lib_path, logger=logger, ) From 25856c14cbc6eda73c8db12c319840ec39ff6c77 Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Fri, 14 Apr 2023 00:16:01 -0700 Subject: [PATCH 34/61] Add optional flag bundle_vcomp140_dll --- python-package/packager/build_config.py | 11 +++++++++++ python-package/packager/pep517.py | 6 ++++-- tests/buildkite/build-win64-gpu.ps1 | 2 +- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/python-package/packager/build_config.py b/python-package/packager/build_config.py index c45a5b8aa455..250c2a0d5a1a 100644 --- a/python-package/packager/build_config.py +++ b/python-package/packager/build_config.py @@ -7,14 +7,25 @@ class BuildConfiguration: # pylint: disable=R0902 """Configurations use when building libxgboost""" + # Whether to hide C++ symbols in libxgboost.so hide_cxx_symbols: bool = True + # Whether to enable OpenMP use_openmp: bool = True + # Whether to enable CUDA use_cuda: bool = False + # Whether to enable NCCL use_nccl: bool = False + # Whether to enable HDFS use_hdfs: bool = False + # Whether to enable Azure Storage use_azure: bool = False + # Whether to enable AWS S3 use_s3: bool = False + # Whether to enable the dense parser plugin plugin_dense_parser: bool = False + # Whether to bundle OpenMP library from Microsoft + bundle_vcomp140_dll: bool = False + # Special option: See explanation below use_system_libxgboost: bool = False def _set_config_setting( diff --git a/python-package/packager/pep517.py b/python-package/packager/pep517.py index 0569a7e15669..6d515ae6c698 100644 --- a/python-package/packager/pep517.py +++ b/python-package/packager/pep517.py @@ -4,6 +4,7 @@ Re-uses components of Hatchling (https://github.com/pypa/hatch/tree/master/backend) for the sake of brevity. """ +import dataclasses import logging import os import pathlib @@ -90,6 +91,7 @@ def build_wheel( build_config = BuildConfiguration() build_config.update(config_settings) + logger.info("Parsed build configuration: %s", dataclasses.asdict(build_config)) # Create tempdir with Python package + libxgboost with tempfile.TemporaryDirectory() as td: @@ -113,11 +115,11 @@ def build_wheel( TOPLEVEL_DIR, build_dir=build_dir, build_config=build_config ) copy_with_logging(libxgboost, lib_path, logger=logger) - if system() == "Windows": + if system() == "Windows" and build_config.bundle_vcomp140_dll: vcomp140_path = pathlib.Path(r"C:\Windows\System32\vcomp140.dll") if not vcomp140_path.exists(): raise RuntimeError( - "To build XGBoost from the source, please install Microsoft " + "vcomp140.dll is missing. Please install Microsoft " "Visual C++ Redistributable for Visual Studio 2015 at " "https://www.microsoft.com/en-ca/download/details.aspx?id=48145." ) diff --git a/tests/buildkite/build-win64-gpu.ps1 b/tests/buildkite/build-win64-gpu.ps1 index f64995dd076b..a81fc97f912a 100644 --- a/tests/buildkite/build-win64-gpu.ps1 +++ b/tests/buildkite/build-win64-gpu.ps1 @@ -24,7 +24,7 @@ if ($LASTEXITCODE -ne 0) { throw "Last command failed" } Write-Host "--- Build binary wheel" cd ../python-package conda activate -& pip wheel -v . --wheel-dir dist/ +& pip wheel -v . --wheel-dir dist/ --config-settings bundle_vcomp140_dll=True Get-ChildItem . -Filter dist/*.whl | Foreach-Object { & python ../tests/ci_build/rename_whl.py $_.FullName $Env:BUILDKITE_COMMIT win_amd64 From 58b2dc2dd67bda9382d268cad3aa5d568c3ab210 Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Fri, 14 Apr 2023 00:22:00 -0700 Subject: [PATCH 35/61] Better doc --- doc/build.rst | 10 ++++++++-- python-package/packager/build_config.py | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/doc/build.rst b/doc/build.rst index 7a2e81a312c0..e90b43bbe6c4 100644 --- a/doc/build.rst +++ b/doc/build.rst @@ -199,8 +199,14 @@ There are several ways to build and install the package from source: :start-at: @dataclasses.dataclass :end-before: def _set_config_setting( - (The ``use_system_libxgboost`` option is special. See Item 3 below for - detailed description.) + Some notes on the options: + + * ``bundle_vcomp140_dll`` is not useful for ``pip install``, since + your computer would already have ``vcomp140.dll`` installed. However, + the option is highly useful if :ref:`you are building binary wheeels for + redistributing XGBoost `. + * The ``use_system_libxgboost`` option is special. See Item 3 below for + detailed description. .. note:: Verbose flag recommended diff --git a/python-package/packager/build_config.py b/python-package/packager/build_config.py index 250c2a0d5a1a..47718a7051b7 100644 --- a/python-package/packager/build_config.py +++ b/python-package/packager/build_config.py @@ -23,7 +23,7 @@ class BuildConfiguration: # pylint: disable=R0902 use_s3: bool = False # Whether to enable the dense parser plugin plugin_dense_parser: bool = False - # Whether to bundle OpenMP library from Microsoft + # Whether to bundle vcomp140.dll, OpenMP library from Microsoft bundle_vcomp140_dll: bool = False # Special option: See explanation below use_system_libxgboost: bool = False From a38d26cfb254c906cf9219a1745c61a3188eec70 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Fri, 14 Apr 2023 08:38:22 -0700 Subject: [PATCH 36/61] Add doc for Python packaging --- doc/build.rst | 9 ++-- doc/contrib/index.rst | 1 + doc/contrib/python_packaging.rst | 77 ++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 3 deletions(-) create mode 100644 doc/contrib/python_packaging.rst diff --git a/doc/build.rst b/doc/build.rst index e90b43bbe6c4..86c8fbadd0b7 100644 --- a/doc/build.rst +++ b/doc/build.rst @@ -12,6 +12,7 @@ systems. If the instructions do not work for you, please feel free to ask quest Consider installing XGBoost from a pre-built binary, to avoid the trouble of building XGBoost from the source. Checkout :doc:`Installation Guide `. .. contents:: Contents + :local: .. _get_source: @@ -203,9 +204,11 @@ There are several ways to build and install the package from source: * ``bundle_vcomp140_dll`` is not useful for ``pip install``, since your computer would already have ``vcomp140.dll`` installed. However, - the option is highly useful if :ref:`you are building binary wheeels for - redistributing XGBoost `. - * The ``use_system_libxgboost`` option is special. See Item 3 below for + the option is highly useful if you are building + :ref:`binary wheels to re-distribute XGBoost `, since + other users may not have ``vcomp140.dll`` installed on their + Windows machine. + * ``use_system_libxgboost`` is a special option. See Item 3 below for detailed description. .. note:: Verbose flag recommended diff --git a/doc/contrib/index.rst b/doc/contrib/index.rst index c9c5f93a2e8b..6a36cb1086c1 100644 --- a/doc/contrib/index.rst +++ b/doc/contrib/index.rst @@ -23,6 +23,7 @@ Here are guidelines for contributing to various aspect of the XGBoost project: Community Guideline donate coding_guide + python_packaging unit_tests Docs and Examples git_guide diff --git a/doc/contrib/python_packaging.rst b/doc/contrib/python_packaging.rst new file mode 100644 index 000000000000..0208cdf2df4e --- /dev/null +++ b/doc/contrib/python_packaging.rst @@ -0,0 +1,77 @@ +######################### +Notes on Python packaging +######################### + + +.. contents:: Contents + :local: + +.. _binary_wheels: + +*************************************************** +How to build binary wheels and source distributions +*************************************************** + +Binary wheels and source distributions (sdist for short) are two main +mechanisms for packaging and distributing Python packages. + +A **source distribution** (sdist) is a tarball (``.tar.gz`` extension) that +contains source code. In the case of XGBoost, an sdist contains +both the Python code as well as the C++. You can obtain an sdist as follows: + +.. code-block:: console + + $ python -m build --sdist . + +(You'll need to install the ``build`` package first: +``pip install build`` or ``conda install python-build``.) + +Running ``pip install`` with an sdist will launch CMake and a C++ compiler +to compile the bundled C++ code into native library ``libxgboost.so``: + +.. code-block:: console + + $ pip install -v xgboost-2.0.0.tar.gz # Add -v to show build progress + +A **binary wheel** is a ZIP-compressed archive (``.whl`` extension) that +contains Python source code as well as compiled extensions. In the case of +XGBoost, a binary wheel contains the Python code as well as a pre-built +native library ``libxgboost.so``. Build a binary wheel as follows: + +.. code-block:: console + + $ pip wheel -v . + +Running ``pip install`` with a binary wheel will extract the content of +the wheel into the current Python environment. Crucially, since the +wheel already contains a pre-built native library ``libxgboost.so``, +it does not have to be built. So ``pip install`` with a binary wheel +completes quickly. + +.. code-block:: console + + $ pip install -v xgboost-2.0.0-py3-none-linux_x86_64.whl + +.. note:: Bundling OpenMP library on Windows + + XGBoost uses OpenMP to implement parallel algorithms on CPUs. + Consequently, on the Windows platform, XGBoost requires access + to the system library ``vcomp140.dll``. Not every Windows + machine has this library installed. So we have two choices + when it comes to distributing XGBoost: + + 1. Ask all users to install + `Visual C++ Redistributable for Visual Studio 2015 + `_. + 2. Inject ``vcomp140.dll`` into the binary wheel. In this + case, ``vcomp140.dll`` will be installed in the same directory + as XGBoost. To enable bundling, pass ``bundle_vcomp140_dll`` + option to Pip: + + .. code-block:: console + + $ pip install . --config-settings bundle_vcomp140_dll=True + + The XGBoost project uses Option 2: we bundle ``vcomp140.dll`` + in the binary wheel targeting Windows, before we upload it to + Python Packaging Index (PyPI). From e3d4cf1c9cf9e37b2eb1484de96183839dd69ea5 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Fri, 14 Apr 2023 08:42:32 -0700 Subject: [PATCH 37/61] Fix mypy check --- tests/ci_build/change_version.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ci_build/change_version.py b/tests/ci_build/change_version.py index ac3d71f060f5..25561859c5e4 100644 --- a/tests/ci_build/change_version.py +++ b/tests/ci_build/change_version.py @@ -54,10 +54,10 @@ def pypkg( with open(pyprj_path, "r") as fd: pyprj = fd.read() matched = re.search('version = "' + r"([0-9]+\.[0-9]+\.[0-9]+.*)" + '"', pyprj) - print(matched.group(1)) + assert matched, "Couldn't find version string in pyproject.toml." pyprj = pyprj[: matched.start(1)] + pyver + pyprj[matched.end(1) :] with open(pyprj_path, "w") as fd: - pyprj = fd.write(pyprj) + fd.write(pyprj) @cd(R_PACKAGE) From 37aeac8fd2268a97e8b8c2e2d661f5863e33b813 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Fri, 14 Apr 2023 08:59:03 -0700 Subject: [PATCH 38/61] Clean up build instructions --- doc/build.rst | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/doc/build.rst b/doc/build.rst index 86c8fbadd0b7..6628b15d3bed 100644 --- a/doc/build.rst +++ b/doc/build.rst @@ -177,13 +177,27 @@ Building Python Package with Default Toolchains =============================================== There are several ways to build and install the package from source: -1. Install the Python package directly +1. Build C++ core with CMake first + + You can first build C++ library using CMake as described in :ref:`build_shared_lib`. + After compilation, a shared library will appear in ``lib/`` directory. + On Linux distributions, the shared library is ``lib/libxgboost.so``. + The install script ``pip install .`` will reuse the shared library instead of compiling + it from scratch, making it quite fast to run. + + .. code-block:: console + + $ cd python-package/ + $ pip install . # Will re-use lib/libxgboost.so + +2. Install the Python package directly You can navigate to ``python-package/`` directory and install the Python package directly by running .. code-block:: console + $ cd python-package/ $ pip install -v . which will compile XGBoost's native (C++) code using default CMake flags. @@ -218,15 +232,7 @@ There are several ways to build and install the package from source: you add the verbose flag (``-v``) when invoking ``pip install``. -2. Build C++ core with CMake first - - This is mostly for C++ developers who iterate frequently on the C++ part of XGBoost. - You can build C++ library directly using CMake as described in :ref:`build_shared_lib`. - After compilation, a shared library (or called dynamic linked library; the jargon - depends on your platform) will appear in XGBoost's source tree under ``lib/`` - directory. On Linux distributions, the shared library is ``lib/libxgboost.so``. - The install script ``pip install .`` will reuse the shared library instead of compiling - it from scratch, making it quite fast to run. +3. Editable installation To further enable rapid development and iteration, we provide an **editable installation**. In an editable installation, the installed package is simply a symbolic link to your @@ -246,7 +252,7 @@ There are several ways to build and install the package from source: cd ../python-package pip install -e . -3. Use ``libxgboost.so`` on system path. +4. Use ``libxgboost.so`` on system path. This option is useful for package managers that wish to separately package ``libxgboost.so`` and the XGBoost Python package. For example, Conda @@ -266,7 +272,7 @@ There are several ways to build and install the package from source: .. code-block:: bash - cd xgboost/python-package + cd python-package pip install . --config-settings use_system_libxgboost=True From 886802a8fcc2f9e6e0bae2f074d90038db670c09 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Fri, 14 Apr 2023 10:15:47 -0700 Subject: [PATCH 39/61] Fix pip wheel --- doc/contrib/python_packaging.rst | 2 +- tests/buildkite/build-cpu-arm64.sh | 2 +- tests/buildkite/build-cuda.sh | 2 +- tests/buildkite/build-win64-gpu.ps1 | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/contrib/python_packaging.rst b/doc/contrib/python_packaging.rst index 0208cdf2df4e..8dd119be4a03 100644 --- a/doc/contrib/python_packaging.rst +++ b/doc/contrib/python_packaging.rst @@ -40,7 +40,7 @@ native library ``libxgboost.so``. Build a binary wheel as follows: .. code-block:: console - $ pip wheel -v . + $ pip wheel --no-deps -v . Running ``pip install`` with a binary wheel will extract the content of the wheel into the current Python environment. Crucially, since the diff --git a/tests/buildkite/build-cpu-arm64.sh b/tests/buildkite/build-cpu-arm64.sh index 47ead65e4c0c..fd00a7971101 100755 --- a/tests/buildkite/build-cpu-arm64.sh +++ b/tests/buildkite/build-cpu-arm64.sh @@ -18,7 +18,7 @@ $command_wrapper bash -c "cd build && ctest --extra-verbose" echo "--- Build binary wheel" $command_wrapper bash -c \ - "cd python-package && rm -rf dist/* && pip wheel -v . --wheel-dir dist/" + "cd python-package && rm -rf dist/* && pip wheel --no-deps -v . --wheel-dir dist/" $command_wrapper python tests/ci_build/rename_whl.py python-package/dist/*.whl \ ${BUILDKITE_COMMIT} ${WHEEL_TAG} diff --git a/tests/buildkite/build-cuda.sh b/tests/buildkite/build-cuda.sh index f8db963d0fb3..c180695e820f 100755 --- a/tests/buildkite/build-cuda.sh +++ b/tests/buildkite/build-cuda.sh @@ -27,7 +27,7 @@ $command_wrapper tests/ci_build/build_via_cmake.sh -DCMAKE_PREFIX_PATH=/opt/grpc -DNCCL_LIBRARY=/workspace/libnccl_static.a ${arch_flag} echo "--- Build binary wheel" $command_wrapper bash -c \ - "cd python-package && rm -rf dist/* && pip wheel -v . --wheel-dir dist/" + "cd python-package && rm -rf dist/* && pip wheel --no-deps -v . --wheel-dir dist/" $command_wrapper python tests/ci_build/rename_whl.py python-package/dist/*.whl \ ${BUILDKITE_COMMIT} ${WHEEL_TAG} diff --git a/tests/buildkite/build-win64-gpu.ps1 b/tests/buildkite/build-win64-gpu.ps1 index a81fc97f912a..47abf917092a 100644 --- a/tests/buildkite/build-win64-gpu.ps1 +++ b/tests/buildkite/build-win64-gpu.ps1 @@ -24,7 +24,7 @@ if ($LASTEXITCODE -ne 0) { throw "Last command failed" } Write-Host "--- Build binary wheel" cd ../python-package conda activate -& pip wheel -v . --wheel-dir dist/ --config-settings bundle_vcomp140_dll=True +& pip wheel --no-deps -v . --wheel-dir dist/ --config-settings bundle_vcomp140_dll=True Get-ChildItem . -Filter dist/*.whl | Foreach-Object { & python ../tests/ci_build/rename_whl.py $_.FullName $Env:BUILDKITE_COMMIT win_amd64 From 8ffcc208ef92108683cc99b6c9bee0f8af2a9e9a Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Fri, 14 Apr 2023 10:30:01 -0700 Subject: [PATCH 40/61] Upgrade to Pip 23+ on Windows --- tests/buildkite/build-win64-gpu.ps1 | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/buildkite/build-win64-gpu.ps1 b/tests/buildkite/build-win64-gpu.ps1 index 47abf917092a..8167af0c7e71 100644 --- a/tests/buildkite/build-win64-gpu.ps1 +++ b/tests/buildkite/build-win64-gpu.ps1 @@ -24,6 +24,7 @@ if ($LASTEXITCODE -ne 0) { throw "Last command failed" } Write-Host "--- Build binary wheel" cd ../python-package conda activate +mamba install -y "pip>=23" & pip wheel --no-deps -v . --wheel-dir dist/ --config-settings bundle_vcomp140_dll=True Get-ChildItem . -Filter dist/*.whl | Foreach-Object { From 5d58da02db02c11ae02d5cfd143fc0c39f219652 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Fri, 14 Apr 2023 10:34:25 -0700 Subject: [PATCH 41/61] Add Pip version requirement in doc --- doc/build.rst | 2 ++ doc/contrib/python_packaging.rst | 1 + 2 files changed, 3 insertions(+) diff --git a/doc/build.rst b/doc/build.rst index 6628b15d3bed..a842a46e01cc 100644 --- a/doc/build.rst +++ b/doc/build.rst @@ -207,6 +207,8 @@ There are several ways to build and install the package from source: $ pip install -v . --config-settings use_cuda=True --config-settings use_nccl=True + Use Pip 22.1 or later to use ``--config-settings`` option. + Here are the available options for ``--config-settings``: .. literalinclude:: ../python-package/packager/build_config.py diff --git a/doc/contrib/python_packaging.rst b/doc/contrib/python_packaging.rst index 8dd119be4a03..b1c30267d584 100644 --- a/doc/contrib/python_packaging.rst +++ b/doc/contrib/python_packaging.rst @@ -70,6 +70,7 @@ completes quickly. .. code-block:: console + $ # Use Pip 22.1+ $ pip install . --config-settings bundle_vcomp140_dll=True The XGBoost project uses Option 2: we bundle ``vcomp140.dll`` From d15d6232c37bcb6d0db0191ca7e91ced702a2d94 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Fri, 14 Apr 2023 10:39:14 -0700 Subject: [PATCH 42/61] Filter bundle_vcomp140_dll from CMake args --- python-package/packager/build_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python-package/packager/build_config.py b/python-package/packager/build_config.py index 47718a7051b7..19caed37f7d3 100644 --- a/python-package/packager/build_config.py +++ b/python-package/packager/build_config.py @@ -51,7 +51,7 @@ def get_cmake_args(self) -> List[str]: """Convert build configuration to CMake args""" cmake_args = [] for field_name in [x.name for x in dataclasses.fields(self)]: - if field_name == "use_system_libxgboost": + if field_name in ["use_system_libxgboost", "bundle_vcomp140_dll"]: continue cmake_option = field_name.upper() cmake_value = "ON" if getattr(self, field_name) else "OFF" From d2fd761eee02b4d51464214aa5675667a9708b2e Mon Sep 17 00:00:00 2001 From: Philip Hyunsu Cho Date: Fri, 14 Apr 2023 11:06:12 -0700 Subject: [PATCH 43/61] Update build-win64-gpu.ps1 --- tests/buildkite/build-win64-gpu.ps1 | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/buildkite/build-win64-gpu.ps1 b/tests/buildkite/build-win64-gpu.ps1 index 8167af0c7e71..a66bda8f4a6e 100644 --- a/tests/buildkite/build-win64-gpu.ps1 +++ b/tests/buildkite/build-win64-gpu.ps1 @@ -25,6 +25,7 @@ Write-Host "--- Build binary wheel" cd ../python-package conda activate mamba install -y "pip>=23" +& pip --version & pip wheel --no-deps -v . --wheel-dir dist/ --config-settings bundle_vcomp140_dll=True Get-ChildItem . -Filter dist/*.whl | Foreach-Object { From 9f43ad7d00a824bfb9f808a7ebe3debc6db0271f Mon Sep 17 00:00:00 2001 From: Philip Hyunsu Cho Date: Fri, 14 Apr 2023 11:59:13 -0700 Subject: [PATCH 44/61] Update build-win64-gpu.ps1 --- tests/buildkite/build-win64-gpu.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/buildkite/build-win64-gpu.ps1 b/tests/buildkite/build-win64-gpu.ps1 index a66bda8f4a6e..00ec063a54fa 100644 --- a/tests/buildkite/build-win64-gpu.ps1 +++ b/tests/buildkite/build-win64-gpu.ps1 @@ -24,7 +24,7 @@ if ($LASTEXITCODE -ne 0) { throw "Last command failed" } Write-Host "--- Build binary wheel" cd ../python-package conda activate -mamba install -y "pip>=23" +& pip install -v pip>=23 & pip --version & pip wheel --no-deps -v . --wheel-dir dist/ --config-settings bundle_vcomp140_dll=True Get-ChildItem . -Filter dist/*.whl | From bca703a6e03dc227110bb84aa6ff0394713f74ab Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Fri, 14 Apr 2023 17:13:32 -0700 Subject: [PATCH 45/61] Last fix --- tests/buildkite/build-win64-gpu.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/buildkite/build-win64-gpu.ps1 b/tests/buildkite/build-win64-gpu.ps1 index 00ec063a54fa..17d7c5476fd3 100644 --- a/tests/buildkite/build-win64-gpu.ps1 +++ b/tests/buildkite/build-win64-gpu.ps1 @@ -34,7 +34,7 @@ Foreach-Object { } Write-Host "--- Upload Python wheel" -cd ../.. +cd .. Get-ChildItem . -Filter python-package/dist/*.whl | Foreach-Object { & buildkite-agent artifact upload python-package/dist/$_ From 31cec823c49c891f6923fddccccc26ecb815d62c Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Sat, 15 Apr 2023 00:21:22 -0700 Subject: [PATCH 46/61] Don't bundle vcomp140.dll --- doc/build.rst | 12 ++---------- doc/contrib/python_packaging.rst | 25 ------------------------- python-package/packager/build_config.py | 4 +--- python-package/packager/pep517.py | 13 ------------- tests/buildkite/build-win64-gpu.ps1 | 4 +--- 5 files changed, 4 insertions(+), 54 deletions(-) diff --git a/doc/build.rst b/doc/build.rst index a842a46e01cc..d868a5718966 100644 --- a/doc/build.rst +++ b/doc/build.rst @@ -216,16 +216,8 @@ There are several ways to build and install the package from source: :start-at: @dataclasses.dataclass :end-before: def _set_config_setting( - Some notes on the options: - - * ``bundle_vcomp140_dll`` is not useful for ``pip install``, since - your computer would already have ``vcomp140.dll`` installed. However, - the option is highly useful if you are building - :ref:`binary wheels to re-distribute XGBoost `, since - other users may not have ``vcomp140.dll`` installed on their - Windows machine. - * ``use_system_libxgboost`` is a special option. See Item 3 below for - detailed description. + ``use_system_libxgboost`` is a special option. See Item 3 below for + detailed description. .. note:: Verbose flag recommended diff --git a/doc/contrib/python_packaging.rst b/doc/contrib/python_packaging.rst index b1c30267d584..9b633d707f97 100644 --- a/doc/contrib/python_packaging.rst +++ b/doc/contrib/python_packaging.rst @@ -51,28 +51,3 @@ completes quickly. .. code-block:: console $ pip install -v xgboost-2.0.0-py3-none-linux_x86_64.whl - -.. note:: Bundling OpenMP library on Windows - - XGBoost uses OpenMP to implement parallel algorithms on CPUs. - Consequently, on the Windows platform, XGBoost requires access - to the system library ``vcomp140.dll``. Not every Windows - machine has this library installed. So we have two choices - when it comes to distributing XGBoost: - - 1. Ask all users to install - `Visual C++ Redistributable for Visual Studio 2015 - `_. - 2. Inject ``vcomp140.dll`` into the binary wheel. In this - case, ``vcomp140.dll`` will be installed in the same directory - as XGBoost. To enable bundling, pass ``bundle_vcomp140_dll`` - option to Pip: - - .. code-block:: console - - $ # Use Pip 22.1+ - $ pip install . --config-settings bundle_vcomp140_dll=True - - The XGBoost project uses Option 2: we bundle ``vcomp140.dll`` - in the binary wheel targeting Windows, before we upload it to - Python Packaging Index (PyPI). diff --git a/python-package/packager/build_config.py b/python-package/packager/build_config.py index 19caed37f7d3..89fef7efeacc 100644 --- a/python-package/packager/build_config.py +++ b/python-package/packager/build_config.py @@ -23,8 +23,6 @@ class BuildConfiguration: # pylint: disable=R0902 use_s3: bool = False # Whether to enable the dense parser plugin plugin_dense_parser: bool = False - # Whether to bundle vcomp140.dll, OpenMP library from Microsoft - bundle_vcomp140_dll: bool = False # Special option: See explanation below use_system_libxgboost: bool = False @@ -51,7 +49,7 @@ def get_cmake_args(self) -> List[str]: """Convert build configuration to CMake args""" cmake_args = [] for field_name in [x.name for x in dataclasses.fields(self)]: - if field_name in ["use_system_libxgboost", "bundle_vcomp140_dll"]: + if field_name in ["use_system_libxgboost"]: continue cmake_option = field_name.upper() cmake_value = "ON" if getattr(self, field_name) else "OFF" diff --git a/python-package/packager/pep517.py b/python-package/packager/pep517.py index 6d515ae6c698..9796fd33d857 100644 --- a/python-package/packager/pep517.py +++ b/python-package/packager/pep517.py @@ -115,19 +115,6 @@ def build_wheel( TOPLEVEL_DIR, build_dir=build_dir, build_config=build_config ) copy_with_logging(libxgboost, lib_path, logger=logger) - if system() == "Windows" and build_config.bundle_vcomp140_dll: - vcomp140_path = pathlib.Path(r"C:\Windows\System32\vcomp140.dll") - if not vcomp140_path.exists(): - raise RuntimeError( - "vcomp140.dll is missing. Please install Microsoft " - "Visual C++ Redistributable for Visual Studio 2015 at " - "https://www.microsoft.com/en-ca/download/details.aspx?id=48145." - ) - copy_with_logging( - vcomp140_path, - lib_path, - logger=logger, - ) with cd(workspace): wheel_name = hatchling.build.build_wheel( diff --git a/tests/buildkite/build-win64-gpu.ps1 b/tests/buildkite/build-win64-gpu.ps1 index 17d7c5476fd3..4edcd9801e1f 100644 --- a/tests/buildkite/build-win64-gpu.ps1 +++ b/tests/buildkite/build-win64-gpu.ps1 @@ -24,9 +24,7 @@ if ($LASTEXITCODE -ne 0) { throw "Last command failed" } Write-Host "--- Build binary wheel" cd ../python-package conda activate -& pip install -v pip>=23 -& pip --version -& pip wheel --no-deps -v . --wheel-dir dist/ --config-settings bundle_vcomp140_dll=True +& pip wheel --no-deps -v . --wheel-dir dist/ Get-ChildItem . -Filter dist/*.whl | Foreach-Object { & python ../tests/ci_build/rename_whl.py $_.FullName $Env:BUILDKITE_COMMIT win_amd64 From 3374512230505a6062c24756c8c60c742a531cdd Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Sat, 15 Apr 2023 00:31:33 -0700 Subject: [PATCH 47/61] Document that redist is required --- doc/install.rst | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/doc/install.rst b/doc/install.rst index 03daf465f605..cdeb80ae670f 100644 --- a/doc/install.rst +++ b/doc/install.rst @@ -16,7 +16,7 @@ Stable Release Python ------ -Pre-built binary are uploaded to PyPI (Python Package Index) for each release. Supported platforms are Linux (x86_64, aarch64), Windows (x86_64) and MacOS (x86_64, Apple Silicon). +Pre-built binary wheels are uploaded to PyPI (Python Package Index) for each release. Supported platforms are Linux (x86_64, aarch64), Windows (x86_64) and MacOS (x86_64, Apple Silicon). .. code-block:: bash @@ -24,7 +24,22 @@ Pre-built binary are uploaded to PyPI (Python Package Index) for each release. You might need to run the command with ``--user`` flag or use ``virtualenv`` if you run -into permission errors. Python pre-built binary capability for each platform: +into permission errors. + +.. note:: Windows users need to install Visual C++ Redistributable + + If you are not using Windows, ignore this note. + + XGBoost requires DLLs from `Visual C++ Redistributable + `_ + in order to function, so make sure to install it. Exception: If + you have Visual Studio installed, you already have access to + necessary libraries and thus don't need to Install Visual C++ + Redistributable. + +.. code-bloack:: + +Capabilities of binary wheels for each platform: .. |tick| unicode:: U+2714 .. |cross| unicode:: U+2718 From 3d5c12674192e4d750a913d50eb4bddc52cf48f4 Mon Sep 17 00:00:00 2001 From: Philip Hyunsu Cho Date: Sat, 15 Apr 2023 00:46:26 -0700 Subject: [PATCH 48/61] Update install.rst --- doc/install.rst | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/doc/install.rst b/doc/install.rst index cdeb80ae670f..c8ad2a1cec81 100644 --- a/doc/install.rst +++ b/doc/install.rst @@ -28,16 +28,13 @@ into permission errors. .. note:: Windows users need to install Visual C++ Redistributable - If you are not using Windows, ignore this note. - XGBoost requires DLLs from `Visual C++ Redistributable `_ in order to function, so make sure to install it. Exception: If you have Visual Studio installed, you already have access to - necessary libraries and thus don't need to Install Visual C++ + necessary libraries and thus don't need to install Visual C++ Redistributable. -.. code-bloack:: Capabilities of binary wheels for each platform: From 7d5e422eb39344a6f4f94ee74a54b303fbda57f4 Mon Sep 17 00:00:00 2001 From: Philip Hyunsu Cho Date: Sat, 15 Apr 2023 00:49:29 -0700 Subject: [PATCH 49/61] Remove unused import --- python-package/packager/pep517.py | 1 - 1 file changed, 1 deletion(-) diff --git a/python-package/packager/pep517.py b/python-package/packager/pep517.py index 9796fd33d857..e52349196430 100644 --- a/python-package/packager/pep517.py +++ b/python-package/packager/pep517.py @@ -11,7 +11,6 @@ import sysconfig import tempfile from contextlib import contextmanager -from platform import system from typing import Any, Dict, Iterator, List, Optional, Union import hatchling.build From ae7c03c5b9315001d981456f7b76c585ccfc1d0e Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Sat, 15 Apr 2023 10:31:00 -0700 Subject: [PATCH 50/61] Document that pip 21.3+ is required --- doc/install.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/install.rst b/doc/install.rst index c8ad2a1cec81..0e155f647731 100644 --- a/doc/install.rst +++ b/doc/install.rst @@ -20,6 +20,7 @@ Pre-built binary wheels are uploaded to PyPI (Python Package Index) for each rel .. code-block:: bash + # Pip 21.3+ is required pip install xgboost From 65ac71ff318f881def81bbd653e3bb5f26bbff8c Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Sat, 15 Apr 2023 10:32:33 -0700 Subject: [PATCH 51/61] Use Pip 21.3+ --- tests/buildkite/build-win64-gpu.ps1 | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/buildkite/build-win64-gpu.ps1 b/tests/buildkite/build-win64-gpu.ps1 index 4edcd9801e1f..32cd2806adcd 100644 --- a/tests/buildkite/build-win64-gpu.ps1 +++ b/tests/buildkite/build-win64-gpu.ps1 @@ -24,6 +24,8 @@ if ($LASTEXITCODE -ne 0) { throw "Last command failed" } Write-Host "--- Build binary wheel" cd ../python-package conda activate +& pip install --user -v "pip>=23" +& pip --version & pip wheel --no-deps -v . --wheel-dir dist/ Get-ChildItem . -Filter dist/*.whl | Foreach-Object { From 346472ffa3efe2b004a3e0ffd92aef66f96aadc0 Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Sat, 15 Apr 2023 12:36:00 -0700 Subject: [PATCH 52/61] Simplify lint_python.py --- tests/ci_build/lint_python.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/ci_build/lint_python.py b/tests/ci_build/lint_python.py index 2f1c210c5af0..3f553da9f79e 100644 --- a/tests/ci_build/lint_python.py +++ b/tests/ci_build/lint_python.py @@ -198,7 +198,7 @@ def main(args: argparse.Namespace) -> None: run_mypy(path) for path in [ # core - "python-package/xgboost/", + "python-package/", # demo "demo/json-model/json_parser.py", "demo/guide-python/external_memory.py", @@ -219,8 +219,6 @@ def main(args: argparse.Namespace) -> None: "tests/ci_build/test_r_package.py", "tests/ci_build/test_utils.py", "tests/ci_build/change_version.py", - # Python packaging - "python-package/packager/", ] ): subprocess.check_call(["mypy", "--version"]) From 143d7ef2b3088a62e71e7ba2801c8e04dc116379 Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Tue, 18 Apr 2023 20:32:47 -0700 Subject: [PATCH 53/61] Revise python_packaging.rst --- doc/build.rst | 5 +++ doc/contrib/python_packaging.rst | 68 +++++++++++++++++++++++--------- 2 files changed, 54 insertions(+), 19 deletions(-) diff --git a/doc/build.rst b/doc/build.rst index d868a5718966..b3070dd4e06f 100644 --- a/doc/build.rst +++ b/doc/build.rst @@ -270,6 +270,11 @@ There are several ways to build and install the package from source: pip install . --config-settings use_system_libxgboost=True +.. note:: + + See :doc:`contrib/python_packaging` for instructions on packaging + and distributing XGBoost as Python distributions. + .. _python_mingw: Building Python Package for Windows with MinGW-w64 (Advanced) diff --git a/doc/contrib/python_packaging.rst b/doc/contrib/python_packaging.rst index 9b633d707f97..3fcf8c731796 100644 --- a/doc/contrib/python_packaging.rst +++ b/doc/contrib/python_packaging.rst @@ -1,23 +1,43 @@ -######################### -Notes on Python packaging -######################### +########################################### +Notes on packaging XGBoost's Python package +########################################### .. contents:: Contents :local: -.. _binary_wheels: +.. _packaging_python_xgboost: *************************************************** How to build binary wheels and source distributions *************************************************** -Binary wheels and source distributions (sdist for short) are two main +Wheels and source distributions (sdist for short) are two main mechanisms for packaging and distributing Python packages. -A **source distribution** (sdist) is a tarball (``.tar.gz`` extension) that -contains source code. In the case of XGBoost, an sdist contains -both the Python code as well as the C++. You can obtain an sdist as follows: +* A **source distribution** (sdist) is a tarball (``.tar.gz`` extension) that + contains the source code. +* A **wheel** is a ZIP-compressed archive (with ``.whl`` extension) + representing a *built* distribution. Unlike an sdist, a wheel can contain + compiled components. The compiled components are compiled prior to distribution, + making it more convenient for end-users to install a wheel. Wheels containing + compiled components are referred to as **binary wheels**. + +See `Python Packaging User Guide `_ +to learn more about how Python packages in general are packaged and +distributed. + +For the remainder of this document, we will focus on packaging and +distributing XGBoost. + +Building sdists +=============== + +In the case of XGBoost, an sdist contains both the Python code as well as +the C++ code, so that the core part of XGBoost can be compiled into the +shared libary ``libxgboost.so`` [#shared_lib_name]_. + +You can obtain an sdist as follows: .. code-block:: console @@ -27,27 +47,37 @@ both the Python code as well as the C++. You can obtain an sdist as follows: ``pip install build`` or ``conda install python-build``.) Running ``pip install`` with an sdist will launch CMake and a C++ compiler -to compile the bundled C++ code into native library ``libxgboost.so``: +to compile the bundled C++ code into ``libxgboost.so``: .. code-block:: console $ pip install -v xgboost-2.0.0.tar.gz # Add -v to show build progress -A **binary wheel** is a ZIP-compressed archive (``.whl`` extension) that -contains Python source code as well as compiled extensions. In the case of -XGBoost, a binary wheel contains the Python code as well as a pre-built -native library ``libxgboost.so``. Build a binary wheel as follows: +Building binary wheels +====================== + +You can also build a wheel as follows: .. code-block:: console $ pip wheel --no-deps -v . -Running ``pip install`` with a binary wheel will extract the content of -the wheel into the current Python environment. Crucially, since the -wheel already contains a pre-built native library ``libxgboost.so``, -it does not have to be built. So ``pip install`` with a binary wheel -completes quickly. +Notably, the resulting wheel contains a copy of the shared library +``libxgboost.so`` [#shared_lib_name]_. The wheel is a **binary wheel**, +since it contains a compiled binary. + + +Running ``pip install`` with the binary wheel will extract the content of +the wheel into the current Python environment. Since the wheel already +contains a pre-built copy of ``libxgboost.so``, it does not have to be +built at the time of install. So ``pip install`` with the binary wheel +completes quickly: .. code-block:: console - $ pip install -v xgboost-2.0.0-py3-none-linux_x86_64.whl + $ pip install xgboost-2.0.0-py3-none-linux_x86_64.whl # Completes quickly + +.. rubric:: Foonotes + +.. [#shared_lib_name] The name of the shared library file will differ + depending on the operating system in use. See :ref:`build_shared_lib`. From 73e3c77b4b1649dcd2f7e9f71811f1e9269b6dd2 Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Tue, 18 Apr 2023 20:56:36 -0700 Subject: [PATCH 54/61] Address reviewer's comment --- .gitignore | 3 -- python-package/hatch_build.py | 14 +++++++ python-package/packager/build_config.py | 9 ++-- python-package/packager/nativelib.py | 24 ++++++----- python-package/packager/pep517.py | 56 +++++-------------------- 5 files changed, 42 insertions(+), 64 deletions(-) create mode 100644 python-package/hatch_build.py diff --git a/.gitignore b/.gitignore index 23a2826d3cca..3a606c847f0e 100644 --- a/.gitignore +++ b/.gitignore @@ -149,6 +149,3 @@ model*.json *.rds Rplots.pdf *.zip - -# Python packaging -/python-package/hatch_build.py diff --git a/python-package/hatch_build.py b/python-package/hatch_build.py new file mode 100644 index 000000000000..7e56091cbf22 --- /dev/null +++ b/python-package/hatch_build.py @@ -0,0 +1,14 @@ +import sysconfig + +from hatchling.builders.hooks.plugin.interface import BuildHookInterface + + +def get_tag() -> str: + """Get appropriate wheel tag according to system""" + tag_platform = sysconfig.get_platform().replace("-", "_").replace(".", "_") + return f"py3-none-{tag_platform}" + + +class CustomBuildHook(BuildHookInterface): + def initialize(self, version, build_data): + build_data["tag"] = get_tag() diff --git a/python-package/packager/build_config.py b/python-package/packager/build_config.py index 89fef7efeacc..a83b3572e2d6 100644 --- a/python-package/packager/build_config.py +++ b/python-package/packager/build_config.py @@ -33,11 +33,10 @@ def _set_config_setting( setattr( self, field_name, - ( - config_settings[field_name] - in ["TRUE", "True", "true", "1", "On", "ON", "on"] - ), + (config_settings[field_name].lower() in ["true", "1", "on"]), ) + else: + raise ValueError(f"Field {field_name} is not a valid config_settings") def update(self, config_settings: Optional[Dict[str, Any]]) -> None: """Parse config_settings from Pip (or other PEP 517 frontend)""" @@ -52,6 +51,6 @@ def get_cmake_args(self) -> List[str]: if field_name in ["use_system_libxgboost"]: continue cmake_option = field_name.upper() - cmake_value = "ON" if getattr(self, field_name) else "OFF" + cmake_value = "ON" if getattr(self, field_name) == True else "OFF" cmake_args.append(f"-D{cmake_option}={cmake_value}") return cmake_args diff --git a/python-package/packager/nativelib.py b/python-package/packager/nativelib.py index 35aa14e3b2db..c81e971a7a46 100644 --- a/python-package/packager/nativelib.py +++ b/python-package/packager/nativelib.py @@ -41,7 +41,7 @@ def build_libxgboost( "Building %s from the C++ source files in %s...", _lib_name(), str(cpp_src_dir) ) - def _build(*, generator: str, build_tool: Optional[str] = None) -> None: + def _build(*, generator: str) -> None: cmake_cmd = [ "cmake", str(cpp_src_dir), @@ -69,12 +69,13 @@ def _build(*, generator: str, build_tool: Optional[str] = None) -> None: subprocess.check_call([build_tool, f"-j{nproc}"], cwd=build_dir) if system() == "Windows": - for generator in ( + supported_generators = ( "-GVisual Studio 17 2022", "-GVisual Studio 16 2019", "-GVisual Studio 15 2017", "-GMinGW Makefiles", - ): + ) + for generator in supported_generators: try: _build(generator=generator) logger.info( @@ -90,19 +91,20 @@ def _build(*, generator: str, build_tool: Optional[str] = None) -> None: # Empty build directory shutil.rmtree(build_dir) build_dir.mkdir() - continue - else: - if shutil.which("ninja"): - build_tool = "ninja" else: - build_tool = "make" + raise RuntimeError( + "None of the supported generators produced a successful build!" + f"Supported generators: {supported_generators}" + ) + else: + build_tool = "ninja" if shutil.which("ninja") else "make" generator = "-GNinja" if build_tool == "ninja" else "-GUnix Makefiles" try: - _build(generator=generator, build_tool=build_tool) + _build(generator=generator) except subprocess.CalledProcessError as e: logger.info("Failed to build with OpenMP. Exception: %s", str(e)) build_config.use_openmp = False - _build(generator=generator, build_tool=build_tool) + _build(generator=generator) return build_dir / "lib" / _lib_name() @@ -139,7 +141,7 @@ def locate_or_build_libxgboost( sys_prefix = pathlib.Path(sys.prefix).absolute().resolve() libxgboost = sys_prefix / "lib" / _lib_name() if not libxgboost.exists(): - raise AssertionError( + raise RuntimeError( f"use_system_libxgboost was specified but {_lib_name()} is " f"not found in {libxgboost.parent}" ) diff --git a/python-package/packager/pep517.py b/python-package/packager/pep517.py index e52349196430..86eccfb06607 100644 --- a/python-package/packager/pep517.py +++ b/python-package/packager/pep517.py @@ -1,14 +1,13 @@ """ Custom build backend for XGBoost Python package. Builds source distribution and binary wheels, following PEP 517 / PEP 660. -Re-uses components of Hatchling (https://github.com/pypa/hatch/tree/master/backend) for the sake +Reuses components of Hatchling (https://github.com/pypa/hatch/tree/master/backend) for the sake of brevity. """ import dataclasses import logging import os import pathlib -import sysconfig import tempfile from contextlib import contextmanager from typing import Any, Dict, Iterator, List, Optional, Union @@ -23,9 +22,11 @@ @contextmanager def cd(path: Union[str, pathlib.Path]) -> Iterator[str]: # pylint: disable=C0103 - """Temporarily change working directory""" - if isinstance(path, pathlib.Path): - path = str(path) + """ + Temporarily change working directory. + TODO(hcho3): Remove this once we adopt Python 3.11, which implements contextlib.chdir. + """ + path = str(path) path = os.path.realpath(path) cwd = os.getcwd() os.chdir(path) @@ -35,49 +36,14 @@ def cd(path: Union[str, pathlib.Path]) -> Iterator[str]: # pylint: disable=C010 os.chdir(cwd) -def get_tag() -> str: - """Get appropate wheel tag, according to system""" - tag_platform = sysconfig.get_platform().replace("-", "_").replace(".", "_") - return f"py3-none-{tag_platform}" - - TOPLEVEL_DIR = pathlib.Path(__file__).parent.parent.absolute().resolve() logging.basicConfig(level=logging.INFO) -def get_requires_for_build_wheel( - config_settings: Optional[Dict[str, Any]] = None -) -> List[str]: - """A PEP 517 method. Delegate to Hatchling""" - return hatchling.build.get_requires_for_build_wheel(config_settings) - - -def get_requires_for_build_sdist( - config_settings: Optional[Dict[str, Any]] = None -) -> List[str]: - """A PEP 517 method. Delegate to Hatchling""" - return hatchling.build.get_requires_for_build_sdist(config_settings) - - -def get_requires_for_build_editable( - config_settings: Optional[Dict[str, Any]] = None -) -> List[str]: - """A PEP 517 method. Delegate to Hatchling""" - return hatchling.build.get_requires_for_build_editable(config_settings) - - -def write_hatch_config(dest_dir: pathlib.Path, *, logger: logging.Logger) -> None: - """Write a small custom hook for Hatch, to set a custom tag.""" - template = ( - "from hatchling.builders.hooks.plugin.interface import BuildHookInterface\n" - "class CustomBuildHook(BuildHookInterface):\n" - " def initialize(self, version, build_data):\n" - " build_data['tag'] = '{tag}'\n" - ) - hatch_build_file = dest_dir / "hatch_build.py" - logger.info("Writing %s", str(hatch_build_file)) - with open(hatch_build_file, "w", encoding="utf-8") as f: - f.write(template.format(tag=get_tag())) +# Aliases +get_requires_for_build_sdist = hatchling.build.get_requires_for_build_sdist +get_requires_for_build_wheel = hatchling.build.get_requires_for_build_wheel +get_requires_for_build_editable = hatchling.build.get_requires_for_build_editable def build_wheel( @@ -103,8 +69,8 @@ def build_wheel( logger.info("Copying project files to temporary directory %s", str(workspace)) copy_with_logging(TOPLEVEL_DIR / "pyproject.toml", workspace, logger=logger) + copy_with_logging(TOPLEVEL_DIR / "hatch_build.py", workspace, logger=logger) copy_with_logging(TOPLEVEL_DIR / "README.rst", workspace, logger=logger) - write_hatch_config(workspace, logger=logger) pkg_path = workspace / "xgboost" copytree_with_logging(TOPLEVEL_DIR / "xgboost", pkg_path, logger=logger) From 953c9bd14b48e354a7dddc20b356b7798f7631fb Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Tue, 18 Apr 2023 21:04:12 -0700 Subject: [PATCH 55/61] Insert missing 'the' --- doc/contrib/python_packaging.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/contrib/python_packaging.rst b/doc/contrib/python_packaging.rst index 3fcf8c731796..6e42992f2833 100644 --- a/doc/contrib/python_packaging.rst +++ b/doc/contrib/python_packaging.rst @@ -12,7 +12,7 @@ Notes on packaging XGBoost's Python package How to build binary wheels and source distributions *************************************************** -Wheels and source distributions (sdist for short) are two main +Wheels and source distributions (sdist for short) are the two main mechanisms for packaging and distributing Python packages. * A **source distribution** (sdist) is a tarball (``.tar.gz`` extension) that From fd7f1eb1e138777e135656efd93c57c4e018c6f8 Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Tue, 18 Apr 2023 21:06:37 -0700 Subject: [PATCH 56/61] Fix typo --- doc/build.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/build.rst b/doc/build.rst index b3070dd4e06f..e78d2d2f464e 100644 --- a/doc/build.rst +++ b/doc/build.rst @@ -216,7 +216,7 @@ There are several ways to build and install the package from source: :start-at: @dataclasses.dataclass :end-before: def _set_config_setting( - ``use_system_libxgboost`` is a special option. See Item 3 below for + ``use_system_libxgboost`` is a special option. See Item 4 below for detailed description. .. note:: Verbose flag recommended From 889b60cf1495c815f485652ec4f4beb6fc389bc5 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Tue, 18 Apr 2023 23:53:09 -0700 Subject: [PATCH 57/61] Fix formatting --- python-package/hatch_build.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python-package/hatch_build.py b/python-package/hatch_build.py index 7e56091cbf22..5af2e28332ca 100644 --- a/python-package/hatch_build.py +++ b/python-package/hatch_build.py @@ -1,4 +1,5 @@ import sysconfig +from typing import Any, Dict from hatchling.builders.hooks.plugin.interface import BuildHookInterface @@ -10,5 +11,5 @@ def get_tag() -> str: class CustomBuildHook(BuildHookInterface): - def initialize(self, version, build_data): + def initialize(self, version: str, build_data: Dict[str, Any]) -> None: build_data["tag"] = get_tag() From c62e2d01fe49f7294f4bab32e6ad8e3e6d21f2e1 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Tue, 18 Apr 2023 23:54:22 -0700 Subject: [PATCH 58/61] Fix build --- python-package/packager/pep517.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python-package/packager/pep517.py b/python-package/packager/pep517.py index 86eccfb06607..2dcb3cd0a163 100644 --- a/python-package/packager/pep517.py +++ b/python-package/packager/pep517.py @@ -114,6 +114,7 @@ def build_sdist( logger.info("Copying project files to temporary directory %s", str(workspace)) copy_with_logging(TOPLEVEL_DIR / "pyproject.toml", workspace, logger=logger) + copy_with_logging(TOPLEVEL_DIR / "hatch_build.py", workspace, logger=logger) copy_with_logging(TOPLEVEL_DIR / "README.rst", workspace, logger=logger) copytree_with_logging( @@ -151,7 +152,6 @@ def build_editable( "See https://xgboost.readthedocs.io/en/latest/build.html for detailed instructions." ) - write_hatch_config(TOPLEVEL_DIR, logger=logger) return hatchling.build.build_editable( wheel_directory, config_settings, metadata_directory ) From dbd9590b4d36f27aa29aa80136bfe85ba412b5b9 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Wed, 19 Apr 2023 00:04:09 -0700 Subject: [PATCH 59/61] Fix formatting --- python-package/hatch_build.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/python-package/hatch_build.py b/python-package/hatch_build.py index 5af2e28332ca..696787fa2fe6 100644 --- a/python-package/hatch_build.py +++ b/python-package/hatch_build.py @@ -1,3 +1,7 @@ +""" +Custom hook to customize the behavior of Hatchling. +Here, we customize the tag of the generated wheels. +""" import sysconfig from typing import Any, Dict @@ -11,5 +15,8 @@ def get_tag() -> str: class CustomBuildHook(BuildHookInterface): + """A custom build hook""" + def initialize(self, version: str, build_data: Dict[str, Any]) -> None: + """This step ccurs immediately before each build.""" build_data["tag"] = get_tag() From 77df44383e97719c631bfbf072bfa250dcc1f100 Mon Sep 17 00:00:00 2001 From: Hyunsu Philip Cho Date: Wed, 19 Apr 2023 00:13:16 -0700 Subject: [PATCH 60/61] Fix formatting --- python-package/packager/build_config.py | 2 +- python-package/packager/pep517.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/python-package/packager/build_config.py b/python-package/packager/build_config.py index a83b3572e2d6..290cf15db608 100644 --- a/python-package/packager/build_config.py +++ b/python-package/packager/build_config.py @@ -51,6 +51,6 @@ def get_cmake_args(self) -> List[str]: if field_name in ["use_system_libxgboost"]: continue cmake_option = field_name.upper() - cmake_value = "ON" if getattr(self, field_name) == True else "OFF" + cmake_value = "ON" if getattr(self, field_name) is True else "OFF" cmake_args.append(f"-D{cmake_option}={cmake_value}") return cmake_args diff --git a/python-package/packager/pep517.py b/python-package/packager/pep517.py index 2dcb3cd0a163..faff4f723010 100644 --- a/python-package/packager/pep517.py +++ b/python-package/packager/pep517.py @@ -10,7 +10,7 @@ import pathlib import tempfile from contextlib import contextmanager -from typing import Any, Dict, Iterator, List, Optional, Union +from typing import Any, Dict, Iterator, Optional, Union import hatchling.build From a85657b21445b826c6c6a27cb0a94cb5f3ca7431 Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Wed, 19 Apr 2023 19:14:03 -0700 Subject: [PATCH 61/61] Address reviewer's feedback --- doc/contrib/python_packaging.rst | 2 +- python-package/packager/nativelib.py | 3 --- python-package/packager/pep517.py | 2 +- python-package/packager/sdist.py | 2 +- python-package/packager/util.py | 4 ++-- 5 files changed, 5 insertions(+), 8 deletions(-) diff --git a/doc/contrib/python_packaging.rst b/doc/contrib/python_packaging.rst index 6e42992f2833..5cf0856851d9 100644 --- a/doc/contrib/python_packaging.rst +++ b/doc/contrib/python_packaging.rst @@ -77,7 +77,7 @@ completes quickly: $ pip install xgboost-2.0.0-py3-none-linux_x86_64.whl # Completes quickly -.. rubric:: Foonotes +.. rubric:: Footnotes .. [#shared_lib_name] The name of the shared library file will differ depending on the operating system in use. See :ref:`build_shared_lib`. diff --git a/python-package/packager/nativelib.py b/python-package/packager/nativelib.py index c81e971a7a46..f7f5b4e79e79 100644 --- a/python-package/packager/nativelib.py +++ b/python-package/packager/nativelib.py @@ -28,7 +28,6 @@ def _lib_name() -> str: def build_libxgboost( cpp_src_dir: pathlib.Path, - *, build_dir: pathlib.Path, build_config: BuildConfiguration, ) -> pathlib.Path: @@ -111,7 +110,6 @@ def _build(*, generator: str) -> None: def locate_local_libxgboost( toplevel_dir: pathlib.Path, - *, logger: logging.Logger, ) -> Optional[pathlib.Path]: """ @@ -126,7 +124,6 @@ def locate_local_libxgboost( def locate_or_build_libxgboost( toplevel_dir: pathlib.Path, - *, build_dir: pathlib.Path, build_config: BuildConfiguration, ) -> pathlib.Path: diff --git a/python-package/packager/pep517.py b/python-package/packager/pep517.py index faff4f723010..56583e117d99 100644 --- a/python-package/packager/pep517.py +++ b/python-package/packager/pep517.py @@ -147,7 +147,7 @@ def build_editable( ) if locate_local_libxgboost(TOPLEVEL_DIR, logger=logger) is None: - raise AssertionError( + raise RuntimeError( "To use the editable installation, first build libxgboost with CMake. " "See https://xgboost.readthedocs.io/en/latest/build.html for detailed instructions." ) diff --git a/python-package/packager/sdist.py b/python-package/packager/sdist.py index 5c82001dad9e..af9fbca0d9ec 100644 --- a/python-package/packager/sdist.py +++ b/python-package/packager/sdist.py @@ -8,7 +8,7 @@ def copy_cpp_src_tree( - cpp_src_dir: pathlib.Path, target_dir: pathlib.Path, *, logger: logging.Logger + cpp_src_dir: pathlib.Path, target_dir: pathlib.Path, logger: logging.Logger ) -> None: """Copy C++ source tree into build directory""" diff --git a/python-package/packager/util.py b/python-package/packager/util.py index eb6fa01869e3..0fff062d7275 100644 --- a/python-package/packager/util.py +++ b/python-package/packager/util.py @@ -7,7 +7,7 @@ def copytree_with_logging( - src: pathlib.Path, dest: pathlib.Path, *, logger: logging.Logger + src: pathlib.Path, dest: pathlib.Path, logger: logging.Logger ) -> None: """Call shutil.copytree() with logging""" logger.info("Copying %s -> %s", str(src), str(dest)) @@ -15,7 +15,7 @@ def copytree_with_logging( def copy_with_logging( - src: pathlib.Path, dest: pathlib.Path, *, logger: logging.Logger + src: pathlib.Path, dest: pathlib.Path, logger: logging.Logger ) -> None: """Call shutil.copy() with logging""" if dest.is_dir():