Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[python-package] upgrade to scikit-build-core 0.9.3 #6263

Merged
merged 12 commits into from
May 8, 2024
Merged

[python-package] upgrade to scikit-build-core 0.9.3 #6263

Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
63aacef
[python-package] upgrade to scikit-build-core 0.7.0
jameslamb Jan 10, 2024
8c6a8e3
0.7.1
jameslamb Jan 10, 2024
220f7db
Merge branch 'master' into newer-scikit-build-core
jameslamb Jan 28, 2024
089a1e8
Merge branch 'master' into newer-scikit-build-core
jameslamb Feb 4, 2024
fb8e4e1
updatte version config
jameslamb Feb 4, 2024
e2990fc
Merge branch 'master' into newer-scikit-build-core
jameslamb Feb 10, 2024
e014047
Merge branch 'master' into newer-scikit-build-core
jameslamb Apr 20, 2024
3d194c8
newer scikit-built-core
jameslamb Apr 20, 2024
f14c303
Merge branch 'master' into newer-scikit-build-core
jameslamb May 5, 2024
754b5c9
0.9.3
jameslamb May 5, 2024
048423e
Merge branch 'master' into newer-scikit-build-core
jameslamb May 8, 2024
a20f27f
opt out of stripping binaries
jameslamb May 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions python-package/pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,26 +57,30 @@ changelog = "https://github.com/microsoft/LightGBM/releases"
# start:build-system
[build-system]

requires = ["scikit-build-core>=0.4.4"]
requires = ["scikit-build-core>=0.9.3"]
build-backend = "scikit_build_core.build"

# based on https://github.com/scikit-build/scikit-build-core#configuration
[tool.scikit-build]

cmake.minimum-version = "3.18"
ninja.minimum-version = "1.11"
cmake.version = ">=3.18"
ninja.version = ">=1.11"
ninja.make-fallback = true
cmake.args = [
"-D__BUILD_FOR_PYTHON:BOOL=ON"
]
cmake.verbose = false
cmake.build-type = "Release"
cmake.targets = ["_lightgbm"]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensures that only the shared library is built, and not other targets like the CLI or headers.

scikit-build-core didn't support that back when we did #5759, which is why this exists:

LightGBM/CMakeLists.txt

Lines 54 to 60 in 88cec47

if(__BUILD_FOR_PYTHON OR __BUILD_FOR_R)
# the Python and R package don't require the CLI
set(BUILD_CLI OFF)
# installing the R and Python package shouldn't place LightGBM's headers
# outside of where the package is installed
set(INSTALL_HEADERS OFF)
endif()

I'd like to leave that in CMakeLists.txt as extra protection for now. It's only a tiny bit of complexity in exchange for protection against some potentially-impactful side effects (like pip install of an sdist placing LightGBM's headers somewhere outside of where pip manages files).

# stripping binaries should be turned back on once this is fixed:
# https://github.com/jameslamb/pydistcheck/issues/235
install.strip = false
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See jameslamb/pydistcheck#235 for a full description of this.

Short description: it's great that scikit-build-core strips binaries by default now, but we don't need it in LightGBM. And pydistcheck's check on debug symbols (which runs in CI) gives us confidence that no debug symbols sneak into compiled objects in wheels.

logging.level = "INFO"
sdist.reproducible = true
wheel.py-api = "py3"
experimental = false
strict-config = true
minimum-version = "0.4.4"
minimum-version = "0.9.3"

# end:build-system

Expand Down
Loading