Vendor Rust crates into source distributions #49

tiran · 2024-06-04T10:16:09Z

vendor_rust() takes an existing sdist tar ball and creates a new sdist with all Rust crates vendored. This allows offline compilation without downloading any Rust crates at build time.

Switch from tomli to toml. The tomli package does not have write support.

I have successfully wheel builds with vendored crates for

cryptography-42.0.7.tar.gz
maturin-1.5.1.tar.gz
pydantic_core-2.18.4.tar.gz
setuptools-rust-1.9.0.tar.gz

with CARGO_NET_OFFLINE=true python3 -m build -w.

dhellmann

This is cool, thank you!

I left some notes about how it might be integrated in an inline comment. Let me know what you think, maybe you had a different idea in mind.

One global comment is I'm trying to log fairly verbosely. Debug messages are usually written to a file. Info messages go to the console as progress messages, indicating that some action (especially anything that might take time) is starting.

dhellmann · 2024-06-04T13:39:01Z

src/fromager/vendor_rust.py

+
+
+def vendor_rust(
+    sdist: pathlib.Path,


We should work out how to integrate this into the flow.

Today we have

download-source

prepare-source

unpack

patch

prepare-build

build-wheel

That leaves the patched source tree with build artifacts in it on the disk, and leaves the original sdist unmodified.

I think we want instead something like

download-source

prepare-source

unpack

patch

vendor rust

re-roll-sdist (to preserve a copy of our patched sources with the vendored rust code, before any build artifacts are written, so we can distribute the sources we use for the build)

prepare-build

build-wheel

If that's the flow, then I think for input we'll want this function to take the path to the source root instead of the sdist file.

That sounds like a good plan. I like the idea to re-roll sdist. Eventually we want to store new sdists with downstream patches into a separate repo, too.

I have changed the function. It now takes a project_dir argument and returns a bool vendor_rust(project_dir: pathlib.Path, *, shrink_vendored: bool = True) -> bool. The project directory is the first directory in the root of a tar ball.

dhellmann · 2024-06-04T13:40:22Z

src/fromager/vendor_rust.py

+    # Remove large, unused archive and lib files. They are only
+    # used on Windows, macOS, and iOS. This makes the vendor bundle up to
+    # 60% smaller.
+    for pattern in VENDOR_FILTER_PATTERNS:


Should we make the pattern configurable, eventually? Are we likely to want to have it be part of the per-package customization interface?

I have refactored the code and moved the cleanup to a separate function. The cleanup now removes all **/*.a and **/*.lib files. It didn't work before because I didn't update the crate's checksum file. That's now fixed.

The removal of pre-compiled archive and lib files serves two purposes:

Reduce size of vendored crates

Prevent use of pre-compiled files

If we ever need the files, then we have to figure out how to build the crates from source.

OK, if it's not going to be variable keeping it in the source makes sense. We can always make it configurable later, if we need to.

dhellmann · 2024-06-04T13:41:23Z

src/fromager/vendor_rust.py

+    with open(fileame, "r", encoding="utf-8") as f:
+        cfg = toml.load(f)
+
+    modified = False


I'm trying to keep project-specific knowledge out of the core bits, and provide an override hook or other mechanism for a build system to inject that sort of knowledge based on the packages it wants to build. In this case, for setting the rust level, I could see us setting a global config in the settings file, or I could see a patch being applied to the package.

Understood! That makes sense.

I have removed the code that patches the MSRV in Cargo.toml files. Let's fix this in a separate patch.

I also filed an issue and PR with pydantic-core to lower their MSRV to 1.75. Hopefully their next upstream version can be built without a patch.

src/fromager/vendor_rust.py

dhellmann · 2024-06-04T13:49:31Z

src/fromager/vendor_rust.py

+            raise FileNotFoundError(project_dir)
+
+        manifests = list(project_dir.glob("**/Cargo.toml"))
+        for manifest in manifests:


An info log message here would provide good progress output.

I have added a bunch of log statement (info and debug) to the code.

tiran

I have addressed your suggestions and updated the code.

I also added a minimal Rust project with pyo3 and setuptools-rust to e2e directory for future testing.

tiran · 2024-06-05T06:52:08Z

src/fromager/vendor_rust.py

+    # Remove large, unused archive and lib files. They are only
+    # used on Windows, macOS, and iOS. This makes the vendor bundle up to
+    # 60% smaller.
+    for pattern in VENDOR_FILTER_PATTERNS:


I have refactored the code and moved the cleanup to a separate function. The cleanup now removes all **/*.a and **/*.lib files. It didn't work before because I didn't update the crate's checksum file. That's now fixed.

The removal of pre-compiled archive and lib files serves two purposes:

Reduce size of vendored crates

Prevent use of pre-compiled files

If we ever need the files, then we have to figure out how to build the crates from source.

tiran · 2024-06-05T06:53:57Z

src/fromager/vendor_rust.py

+    with open(fileame, "r", encoding="utf-8") as f:
+        cfg = toml.load(f)
+
+    modified = False


Understood! That makes sense.

I have removed the code that patches the MSRV in Cargo.toml files. Let's fix this in a separate patch.

I also filed an issue and PR with pydantic-core to lower their MSRV to 1.75. Hopefully their next upstream version can be built without a patch.

tiran · 2024-06-05T06:54:20Z

src/fromager/vendor_rust.py

+            raise FileNotFoundError(project_dir)
+
+        manifests = list(project_dir.glob("**/Cargo.toml"))
+        for manifest in manifests:


I have added a bunch of log statement (info and debug) to the code.

tiran · 2024-06-05T07:01:04Z

src/fromager/vendor_rust.py

+
+
+def vendor_rust(
+    sdist: pathlib.Path,


That sounds like a good plan. I like the idea to re-roll sdist. Eventually we want to store new sdists with downstream patches into a separate repo, too.

I have changed the function. It now takes a project_dir argument and returns a bool vendor_rust(project_dir: pathlib.Path, *, shrink_vendored: bool = True) -> bool. The project directory is the first directory in the root of a tar ball.

`vendor_rust()` takes an existing sdist tar ball and creates a new sdist with all Rust crates vendored. This allows offline compilation without downloading any Rust crates at build time. Switch from `tomli` to `toml`. The `tomli` package does not have write support. I have successfully wheel builds with vendored crates for - `cryptography-42.0.7.tar.gz` - `maturin-1.5.1.tar.gz` - `pydantic_core-2.18.4.tar.gz` - `setuptools-rust-1.9.0.tar.gz` with `CARGO_NET_OFFLINE=true python3 -m build -w`. Signed-off-by: Christian Heimes <[email protected]>

dhellmann

Thank you!

dhellmann · 2024-06-06T19:10:27Z

src/fromager/wheels.py

@@ -15,6 +15,9 @@ def build_wheel(ctx, req, sdist_root_dir, build_env):
    if not builder:
        builder = default_build_wheel
    extra_environ = overrides.extra_environ_for_pkg(ctx.envs_dir, req.name, ctx.variant)
+    # TODO: refactor?
+    # Build Rust without network access
+    extra_environ["CARGO_NET_OFFLINE"] = "true"


I wonder if this should go in default_build_wheel? Maybe it's better to do it here so any overrides also use the same setting without having to remember to add it themselves.

dhellmann · 2024-06-06T19:16:53Z

@Mergifyio refresh

mergify · 2024-06-06T19:16:57Z

refresh

✅ Pull request refreshed

mergify bot added the ci label Jun 4, 2024

tiran force-pushed the vendor-rust branch from 760f4b7 to 6f1d1ba Compare June 4, 2024 10:49

dhellmann reviewed Jun 4, 2024

View reviewed changes

tiran force-pushed the vendor-rust branch from 6f1d1ba to b0f8958 Compare June 5, 2024 07:01

tiran commented Jun 5, 2024

View reviewed changes

tiran force-pushed the vendor-rust branch 2 times, most recently from 7f1499b to 6bc1bcd Compare June 5, 2024 07:54

tiran marked this pull request as ready for review June 5, 2024 11:24

tiran force-pushed the vendor-rust branch from 6bc1bcd to 04331f1 Compare June 5, 2024 11:29

dhellmann approved these changes Jun 6, 2024

View reviewed changes

Merge branch 'main' into vendor-rust

a891f0c

dhellmann merged commit 5ef505f into python-wheel-build:main Jun 6, 2024
19 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Vendor Rust crates into source distributions #49

Vendor Rust crates into source distributions #49

tiran commented Jun 4, 2024

dhellmann left a comment

dhellmann Jun 4, 2024

tiran Jun 5, 2024

dhellmann Jun 4, 2024

tiran Jun 5, 2024

dhellmann Jun 6, 2024

dhellmann Jun 4, 2024

tiran Jun 5, 2024

dhellmann Jun 4, 2024

tiran Jun 5, 2024

tiran left a comment

tiran Jun 5, 2024

tiran Jun 5, 2024

tiran Jun 5, 2024

tiran Jun 5, 2024

dhellmann left a comment

dhellmann Jun 6, 2024

dhellmann commented Jun 6, 2024

mergify bot commented Jun 6, 2024

Vendor Rust crates into source distributions #49

Vendor Rust crates into source distributions #49

Conversation

tiran commented Jun 4, 2024

dhellmann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tiran left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhellmann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhellmann commented Jun 6, 2024

mergify bot commented Jun 6, 2024

✅ Pull request refreshed