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

Jump to bottom

Directly call uv build even when no [build-system] is specified in the pyproject.toml #307

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft

Directly call uv build even when no [build-system] is specified in the pyproject.toml #307

wants to merge 1 commit into from

Conversation

antoinebrl
Copy link
Contributor

Extract from the uv documentation on build systems:

While uv will not build and install the current project without a build system definition, the presence of a [build-system] table is not required in other packages. For legacy reasons, if a build system is not defined, then setuptools.build_meta:legacy is used to build the package. Packages you depend on may not explicitly declare their build system but are still installable. Similarly, if you add a dependency on a local package or install it with uv pip, uv will always attempt to build and install it.

As such, this MR is proposing to remove the reliance on pyproject-build in favor of uv build.

antoinebrl
antoinebrl commented Jan 27, 2025
View reviewed changes
kraken-build/src/kraken/std/python/buildsystem/uv.py

return dst_files
_run_uv_command_with_indexes(
["uv", "build", f"--out-dir={output_directory.absolute()}"], settings, self.project_directory
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Tpt @vsiles uv build support multiple backends: hatch, flit, pdm, setuptools and maturin and scikit. Do you think using uv build here would remove the need for MaturinUvPythonBuildSystem? If that is the case, we would have a unified interface for all things uv independently of the build backend.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sadly not: MaturinUvPythonBuildSystem takes care of building wheels targetting Python stable ABI (abi3) for multiple platforms using cross-compilation whereas maturin called via uv build will by default only build a wheel for the current system.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. The main goal here is cross-compilation. I don't think uv is providing anything like that out of the box (nor that it should)

@antoinebrl antoinebrl marked this pull request as draft January 27, 2025 18:12
Tpt
Tpt reviewed Jan 27, 2025
View reviewed changes
Copy link
Contributor

@Tpt Tpt left a comment

Choose a reason for hiding this comment

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

Thanks!

kraken-build/src/kraken/std/python/buildsystem/uv.py

return dst_files
_run_uv_command_with_indexes(
["uv", "build", f"--out-dir={output_directory.absolute()}"], settings, self.project_directory
Copy link
Contributor

Choose a reason for hiding this comment

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

Sadly not: MaturinUvPythonBuildSystem takes care of building wheels targetting Python stable ABI (abi3) for multiple platforms using cross-compilation whereas maturin called via uv build will by default only build a wheel for the current system.

@antoinebrl antoinebrl force-pushed the ab/uv-subprocess-env-vars branch from 979d8fc to 64c61ea Compare January 27, 2025 22:29
srstevenson
srstevenson approved these changes Jan 27, 2025
View reviewed changes
.changelog/_unreleased.toml
[[entries]]
id = "6a2d1204-cc1d-43f1-9b45-577aec55a104"
type = "improvement"
description = "Directly call `uv build` even when no \\[build-system\\] is specified"
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this some escaping to pass as a command line argument to slap?

Suggested change
description = "Directly call `uv build` even when no \\[build-system\\] is specified"
description = "Directly call `uv build` even when no [build-system] is specified"

Base automatically changed from ab/uv-subprocess-env-vars to develop January 27, 2025 22:42
vsiles
vsiles reviewed Jan 28, 2025
View reviewed changes
kraken-build/src/kraken/std/python/buildsystem/pdm.py Outdated
Comment on lines 135 to 136
logger.info("Run '%s'", " ".join(command))
sp.check_call(command, cwd=self.project_directory)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: we would really benefit to provide a single function for all these calls ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vsiles vsiles vsiles approved these changes

@srstevenson srstevenson srstevenson approved these changes

@Tpt Tpt Tpt approved these changes

@Ghelfi Ghelfi Awaiting requested review from Ghelfi

Assignees
No one assigned
Labels
None yet
Projects
Kraken
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@antoinebrl @Tpt @srstevenson @vsiles