Add support for building without isolation #152

agriyakhetarpal · 2025-03-29T01:12:38Z

Description

This PR is the first in a series of PRs aiming to unblock #109 and also towards strides in other areas; such as being able to build a package from one directory twice, see pyodide/pyodide#5031. In particular, this adds two new flags: --no-isolation/-n and --skip-dependency-check/-x, to the pyodide build CLI. These will allow us not to use an isolated virtual environment when building a wheel for a package if required.

Closes #136

Some TODO items

Get "unisolated" builds working
Get tests passing
Investigate and fix the issue where the compiler wrappers are ending up in temporary directories
Gotta add tests!!!
- Basic tests that see if the isolation is working
- Integration test with NumPy/SciPy/etc.
Add a CHANGELOG entry

agriyakhetarpal · 2025-03-30T01:45:08Z

Hi @ryanking13, I've been able to build and test out the feature(s) I've added in this PR, but one problem still exists: the compiler wrappers for Emscripten are still being placed inside temporary directories as a part of the build process for some reason: https://github.com/pyodide/pyodide-build/actions/runs/14151180877/job/39644562112?pr=152#step:11:449

What this means in action is that if I were to build NumPy without isolation via the below command invocation in a persistent build directory, say, in build/ inside the NumPy repo root:

pyodide build . \
-Csetup-args="--cross-file=$(pyodide config get meson_cross_file)" \
-Csetup-args="-Dblas=none" -Csetup-args="-Dlapack=none" \
-nx \ # i.e., --no-isolation and --skip-dependency-check args, which this PR adds
-Cbuild-dir=build`

I am able to build a NumPy wheel. However, if I were to rerun the same command, hoping to make use of the caching provided by the persistent build directory and achieve a speedup for the subsequent run(s) , it would fail because Meson stores its build metadata inside the build directory, which also contains information about the compiler, etc. Meson then fails to find the compiler, as the temporary directory used the previous wheel build where the compiler wrappers were stored no longer exists.

When I build NumPy with a persistent build dir with native compilation on my macOS machine, the compilers are picked up correctly from conda-forge each time – this makes me think that this is a pyodide-build specific issue. Maybe it has something to do with pywasmcross?

I think you would be the best person to know why this is the case and on how we can address it, so I thought I should ask you about it! :)

ryanking13 · 2025-03-30T06:14:53Z

Hi @agriyakhetarpal, thanks for working on this.

the compiler wrappers for Emscripten are still being placed inside temporary directories as a part of the build process for some reason:

Yep, we are using temporary directory for now, but I think you can change it. We've already changed it in recipe build to store the symlinks in the build directory (#96), so I think you can do the similar thing in the pyodide build.

The directory where the symlinks are stored is decided here and here. I think you can pass the build_dir parameter here to store the symlink.

integration_tests/Makefile

integration_tests/src/numpy.sh

pyodide_build/pypabuild.py

pyodide_build/cli/build.py

pyodide_build/pypabuild.py

ryanking13 · 2025-03-30T06:25:16Z

pyodide_build/out_of_tree/pypi.py

    config_settings: ConfigSettingsType,
-    output_lockfile: str | None,
+    isolation: bool = True,
+    skip_dependency_check: bool = False,


(No action is required for this PR)

It may be weird to build all the dependencies in a no-isolation mode. But let's go as-is for now. I personally want to deprecate the feature to build the dependencies if package maintainers in Python ecosystem starts to build Pyodide wheel themselves.

Joe Marshall added this and we were always a bit unsure if it made sense as a feature.

Yes, I added these extra args to normalise what passes through these functions, even if unused. I agree that it makes sense to drop it entirely. I don't think I've used it personally, and I I haven't seen it being used in the main Pyodide repo.

pyodide_build/pypabuild.py

ryanking13 · 2025-03-30T06:37:16Z

pyodide_build/pypabuild.py

+            builder = _ProjectBuilder(srcdir, runner=_gen_runner(build_env))
+
+            if not skip_dependency_check:
+                missing = builder.check_dependencies(


👍

check_dependencies looks like a union of

get_requires_for_build (== dependencies for the build backend such as setuptools, meson)

build_system_requires (== dependencies in the pyproject.toml)

In the isolated build, we've been dealing with them separately. I guess we can use check_dependencies there too (no action required for this PR).

Moved this to a new issue: #175.

pyodide_build/pypabuild.py

Co-Authored-By: Gyeongjae Choi <def6488@gmail.com>

agriyakhetarpal · 2025-03-31T00:44:52Z

Yep, we are using temporary directory for now, but I think you can change it. We've already changed it in recipe build to store the symlinks in the build directory (#96), so I think you can do the similar thing in the pyodide build.

The directory where the symlinks are stored is decided here and here. I think you can pass the build_dir parameter here to store the symlink.

Thanks a lot for the pointer! I'm able to get subsequent builds for NumPy working. :D

I was able to compile NumPy in 77 seconds on my machine, and taking advantage of the build caching through the persistence of the build dir, numpy-tests compiles in 8 seconds.

This reverts commit abad178.

integration_tests/src/numpy.sh

integration_tests/src/numpy_no_isolation.sh

The integration tests have been failing on `main` since CMake 4 came out due to an uninterpretable caching issue. Upgrading to `geos`'s latest v3.13.1 release seems to fix it. This PR has been stemmed off from #152.

ryanking13 · 2025-04-02T14:16:31Z

Thanks again Agriya! I didn't forget this PR, but I've been a little busy this week and haven't been keeping up with all the PRs. I would like to review this in detail, so please wait for me until the weekend! 🙏

agriyakhetarpal · 2025-04-02T19:50:10Z

No rush, thank you! :)

ryanking13

I left some minor comments, but it looks good to me overall.

I was able to compile NumPy in 77 seconds on my machine, and taking advantage of the build caching through the persistence of the build dir, numpy-tests compiles in 8 seconds.

Nice!

BTW, some of the Google's Bazel-based packages were relying on no-isolated builds, such as JAX. There were a lot of other build issues as well, but I think would make one step closer to supporting those packages. Thank you!

pyodide_build/common.py

pyodide_build/pywasmcross.py

pyodide_build/out_of_tree/build.py

pyodide_build/pypabuild.py

ryanking13 · 2025-04-05T05:18:18Z

pyodide_build/pypabuild.py

 @contextmanager
-def _create_symlink_dir(env: dict[str, str], build_dir: Path | None):
-    if build_dir:
-        # If we're running under build-recipes, leave the symlinks in
-        # the build directory. This helps with reproducing.
-        symlink_dir = build_dir / "pywasmcross_symlinks"
-        shutil.rmtree(symlink_dir, ignore_errors=True)
-        symlink_dir.mkdir()
-        yield symlink_dir
-        return
-
-    # Running from "pyodide build". Put symlinks in a temporary directory.
-    # TODO: Add a debug option to save the symlinks.
-    with TemporaryDirectory() as symlink_dir_str:
-        yield Path(symlink_dir_str)
+def _create_symlink_dir(


We are not using a temporary directory anymore, so I don't think this needs to be a context manager anymore. No cleanup is done at the function end.

I tried this, and it still requires a temporary directory to be created in one more case, as get_build_env also allows build_dir to be None. It looks like it's a little more complex than expected, so I hope it's okay to leave this and try it out in a follow-up PR?

Moved it to a new issue: #176

agriyakhetarpal · 2025-04-05T22:26:53Z

Many thanks for the detailed review, @ryanking13! I'll merge this now, and I've opened issues for adding follow-ups.

agriyakhetarpal added 7 commits March 29, 2025 06:31

Support building without isolation

967e2cf

Pass along extra args, fix tests [integration]

f796810

Add a few stub tests

c2ae4af

Add [integration] test to build NumPy with -nx

b52ea77

Don't run tests on pushes with PRs

4515ef7

Trigger [integration] tests

523e3d3

Drop [integration] no-isolation test on macOS

ed61759

agriyakhetarpal marked this pull request as ready for review March 30, 2025 01:20

agriyakhetarpal requested a review from ryanking13 March 30, 2025 01:45

ryanking13 reviewed Mar 30, 2025

View reviewed changes

agriyakhetarpal and others added 7 commits March 31, 2025 05:00

No need to determine sysconfigdata again

8530171

Accept any args to numpy.sh

e38a704

Move helper functions to pyodide_build.common

0aed06b

Mention effect of --skip-dependency-check

6c5da50

Save symlinks in the build directory

8325e80

Co-Authored-By: Gyeongjae Choi <def6488@gmail.com>

Git-ignore the created symlinks directory

efee932

Trigger [integration] tests

8d17f99

agriyakhetarpal marked this pull request as draft March 31, 2025 00:35

agriyakhetarpal added 7 commits March 31, 2025 06:16

Merge branch 'main' into feat/support-non-isolated-builds

a5e8463

Trigger [integration] tests

97e41b6

Fix test

47d458b

Add better "no isolation" [integration] test

4f793fd

Use bash shell for [integration] tests

abad178

Revert "Use bash shell for [integration] tests"

d586dc4

This reverts commit abad178.

source doesn't work for some reason [integration]

b81e525

agriyakhetarpal commented Mar 31, 2025

View reviewed changes

integration_tests/src/numpy.sh Outdated Show resolved Hide resolved

integration_tests/src/numpy_no_isolation.sh Outdated Show resolved Hide resolved

agriyakhetarpal added 3 commits March 31, 2025 08:04

Miscellaneous edits

2c5ce4b

Add a CHANGELOG entry for #152

f654818

Trigger [integration] tests

1b879bd

agriyakhetarpal marked this pull request as ready for review March 31, 2025 02:48

agriyakhetarpal requested review from hoodmane and ryanking13 March 31, 2025 02:49

agriyakhetarpal added 4 commits April 1, 2025 22:32

Resolve conflicts

83e6536

Trigger [integration] tests

689c284

Try to fix geos recipe [integration]

21562ec

--fresh is no longer needed [integration]

6088aba

agriyakhetarpal force-pushed the feat/support-non-isolated-builds branch from 217185c to 6088aba Compare April 1, 2025 17:58

agriyakhetarpal mentioned this pull request Apr 1, 2025

Fix libgeos compilation that's failing on the main branch #166

Merged

ryanking13 approved these changes Apr 5, 2025

View reviewed changes

agriyakhetarpal added 7 commits April 5, 2025 15:51

Add type annotation for _format_missing_dependencies

206a28e

Extract away directory building logic

de93d2b

Extract away version control ignore even more

6ef79be

no_isolation is no longer used

920aec7

Add TODO for _create_symlink_dir context manager

ba3e8ba

Merge branch 'main' into feat/support-non-isolated-builds

72474e8

Trigger [integration] tests

db53bdb

This was referenced Apr 5, 2025

Use build.ProjectBuilder.check_dependencies for the isolated build #175

Open

_create_symlink_dir should no longer be a context manager #176

Open

agriyakhetarpal merged commit 557762c into main Apr 5, 2025
18 checks passed

agriyakhetarpal deleted the feat/support-non-isolated-builds branch April 5, 2025 22:29

This was referenced Apr 5, 2025

Don't use a context manager for _create_symlink_dir #178

Open

PKG Add jax pyodide/pyodide-recipes#187

Open

Add support for building without isolation #152

Add support for building without isolation #152

Uh oh!

Conversation

agriyakhetarpal commented Mar 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

See also

Some TODO items

Uh oh!

agriyakhetarpal commented Mar 30, 2025

Uh oh!

ryanking13 commented Mar 30, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

agriyakhetarpal commented Mar 31, 2025

Uh oh!

Uh oh!

Uh oh!

ryanking13 commented Apr 2, 2025

Uh oh!

agriyakhetarpal commented Apr 2, 2025

Uh oh!

ryanking13 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agriyakhetarpal commented Apr 5, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

agriyakhetarpal commented Mar 29, 2025 •

edited

Loading