Finish out of tree build system (except xbuildenv deploy) by hoodmane · Pull Request #2823 · pyodide/pyodide

hoodmane · 2022-07-01T21:55:06Z

This completes the out of tree build CLI. This PR is paired up with:
numpy/numpy#21895
I have also successfully built scikit-learn, statsmodels, pandas, and astropy with this.

The last thing we need to do after this is set up deployment of the cross build environment. We can deploy one version to s3 for each tagged commit. I will do that in a separate PR after this is merged.

This is on top of #2821.

Checklists

Add a CHANGELOG entry
Add / update tests
Add new / update outdated documentation

…of tree

hoodmane · 2022-07-01T22:57:16Z

pyodide-build/pyodide_build/common.py

-    ]
-    os.environ["PYTHONPATH"] = ":".join(pythonpath)
+    try:
+        hostsitepackages = get_hostsitepackages()


This call will fail if we haven't installed the xbuildenv yet.

# Conflicts: # pyodide-build/pyodide_build/pywasmcross.py

ryanking13

Thanks, @hoodmane. Probably Roman should review this too, but I think we are going in the proper direction. There are some hard-coded strings that would better be parameterized, but I think that can be done in a separate PR.

So if I understand correctly, this PR is to make the following available?

git clone https://github.com/a-package-that-can-be-built-with-pypa-build/pkg
cd pkg
pip install pyodide-build
pywasm
# pkg-1.0.0-wasm-cp310-cp310-emscripten_3_1_14_wasm32.whl

ryanking13 · 2022-07-05T02:41:13Z

pyodide-build/pyodide_build/install_xbuildenv.py

+
+def main(args: argparse.Namespace) -> None:
+    xbuildenv_path = Path(args.xbuild_env[0])
+    version = "2"


What is your future plan of versioning build envs?

One version per Pyodide release. After we merge this I am planning to add a CI job to deploy the cross build environment as part of deploy-release. It would also be nice to periodically deploy a "nightly" build at some point. Maybe once a month or once every other week or something.

ryanking13 · 2022-07-05T02:41:45Z

pyodide-build/pyodide_build/install_xbuildenv.py

+    rmtree(xbuildenv_path, ignore_errors=True)
+    with NamedTemporaryFile(suffix=".tar") as f:
+        urlretrieve(
+            f"http://pyodide-cache.s3-website-us-east-1.amazonaws.com/xbuildenv/{version}.tar",


Is this your personal S3 bucket?

Roman set it up. We could use it for other things too.

Yeah, as long as it remains a bit of dev oriented cache and we make no quality of service promises, I think it should be OK. Eventually, this could probably be also done with github releases though that's harder for nightly deployments.

Also if you can make it a .tar.gz to reduce the bandwidth that would be better.

Also is there anything platform/OS dependent in this tar? For instance, I see a,

xbuildenv/site-packages-extras/numpy/core/lib/libnpymath.a

is this intentional?

Should we add the linux-x86_64 suffix to indicate that it's linux specific? And also maybe name it something more explicit than "2.tar" otherwise it's not ideal when downloaded manually.

It's not linux specific, it's an Emscripten static library.

2.tar

I was thinking about naming them by version e.g., 0.21.0.tar.gz. But probably github releases is a good idea for the release versions.

hoodmane · 2022-07-05T05:07:13Z

So if I understand correctly, this PR is to make the following available?

Yup. See here for instance:
https://github.com/hoodmane/numpy/blob/12c656f228eb548a8f633876466345f15e39b095/.github/workflows/emscripten.yml#L49

ryanking13 · 2022-07-05T06:01:25Z

So if I understand correctly, this PR is to make the following available?

Yup. See here for instance: https://github.com/hoodmane/numpy/blob/12c656f228eb548a8f633876466345f15e39b095/.github/workflows/emscripten.yml#L49

Nice, it looks very cool :)

hoodmane · 2022-07-06T14:41:15Z

@rth would you mind reviewing this?

rth

Thanks for working on this! The results that you manage to use it in numpy CI is great!

A few comments below otherwise LGTM.

rth · 2022-07-06T14:53:45Z

pyodide-build/pyodide_build/install_xbuildenv.py

+    rmtree(xbuildenv_path, ignore_errors=True)
+    with NamedTemporaryFile(suffix=".tar") as f:
+        urlretrieve(
+            f"http://pyodide-cache.s3-website-us-east-1.amazonaws.com/xbuildenv/{version}.tar",


Yeah, as long as it remains a bit of dev oriented cache and we make no quality of service promises, I think it should be OK. Eventually, this could probably be also done with github releases though that's harder for nightly deployments.

Also if you can make it a .tar.gz to reduce the bandwidth that would be better.

rth · 2022-07-06T14:58:13Z

pyodide-build/pyodide_build/install_xbuildenv.py

+    rmtree(xbuildenv_path, ignore_errors=True)
+    with NamedTemporaryFile(suffix=".tar") as f:
+        urlretrieve(
+            f"http://pyodide-cache.s3-website-us-east-1.amazonaws.com/xbuildenv/{version}.tar",


Also is there anything platform/OS dependent in this tar? For instance, I see a,

xbuildenv/site-packages-extras/numpy/core/lib/libnpymath.a

is this intentional?

Should we add the linux-x86_64 suffix to indicate that it's linux specific? And also maybe name it something more explicit than "2.tar" otherwise it's not ideal when downloaded manually.

rth · 2022-07-06T15:05:10Z

pyodide-build/setup.py

    setuptools.setup(
        entry_points={
            "console_scripts": [
+                "pywasm = pyodide_build.out_of_tree.__main__:main",


That's a bit confusing with https://github.com/mohanson/pywasm

So I guess it would be a bit difficult to use subcommands here. Sill maybe something like pyodide-build-wasm even if it's a bit more verbose? pywasmbuild would also be better. Ideally, it would be to have pyodide in the name but maybe it will be too complex of a name if we include it.

Although if you are calling it as pywasm build (I missed the subcommand part), build in the name would indeed be a bit redundant.

Maybe we should just call this command pyodide and make this the new CLI API? So it would be pyodide build when called in CI of other projects. Or pyodide cross-build or any similar sub-command.

The main distinction in my mind is between in-tree and out-of-tree commands. E.g., mkpkg should be a public in-tree command, whereas it makes no sense out of tree. This is an out of tree command. People are likely to only need one CLI as appropriate to whether they are doing an in tree or out of tree build. So I think it makes sense to keep these as separate CLIs.

hoodmane · 2022-07-06T16:41:40Z

Is it okay if we fix the name of the CLI entrypoint and the .tar.gz extension in a subsequent PR? I think it would be easier to handle these things separately.

rth · 2022-07-06T16:46:07Z

Yes, sure thanks! Very nice progress on this!

In scikit-learn/scikit-learn#23727 I was saying that it's maybe a bit early to run in CI, but it looks like it's closer than I thought thanks to all the work you did! Though having a robust runner for large test suites probably still needs some work, particularly those that can potentially produce fatal errors due to scipy.

hoodmane added 22 commits July 1, 2022 12:19

Respect symbol visibility in export calculation

f3ab3d2

Add a test

480d7cc

Work on out of tree builds

f4eebdf

Set up download + install

f1c10bd

Try to get pywasmbuild entrypoint working

1566861

Chmod pywasmcross.py

0055269

Try to fix entry point

5c65533

Fix pywasmcross when accessed from entrypoint

2cb47b3

More work to get pywasmcross to work from entrypoint

e4c30a3

Cleanup pywasmcross prelude a bit

0f68b04

use exports=requested

4996fe5

Rearrange cli a bit

1d59947

Fix calculate_object_exports

66e37ce

Merge branch 'symbol-visibility' into out-of-tree

02cf02c

Set exports default to requested

109c89d

cleanup

e7ffd7e

Fix again

9811c7d

Merge branch 'symbol-visibility' into out-of-tree

d93690c

Add version argument to download_xbuild_env

9229d5b

Fix pywasmcross so that it works when pyodide_build is installed out …

3371a0d

…of tree

Tune up

9f1a6b7

Rename a few things

1c1bd37

hoodmane mentioned this pull request Jul 1, 2022

Fix pywasmcross when pyodide_build is installed out of tree #2824

Merged

hoodmane added 4 commits July 1, 2022 15:30

Merge branch 'out-of-tree-pywasmcross' into out-of-tree

abd658a

More rearrange

d4e7f88

Update logic again

8ae3acb

Rework yet again

5f4042c

hoodmane commented Jul 1, 2022

View reviewed changes

hoodmane added 2 commits July 1, 2022 16:00

Reduce diff

49d5d5f

Don't print exported symbols

4b4da1b

hoodmane added 15 commits July 1, 2022 16:01

Merge branch 'symbol-visibility' into out-of-tree

8fed837

Fall back to nm for bitcode

6e163a2

Merge branch 'symbol-visibility' into out-of-tree

cb8a022

Update test

9f32585

Merge branch 'symbol-visibility' into out-of-tree

d5328b9

Merge branch 'main' into out-of-tree

7a0c0b4

Merge branch 'main' into symbol-visibility

1fa76e9

Merge branch 'main' into symbol-visibility

c261ad5

Revert debug changes

84b37ce

Merge branch 'symbol-visibility' into out-of-tree

8d8ccd2

# Conflicts: # pyodide-build/pyodide_build/pywasmcross.py

Remove replace_libs argument

9f1084b

Add unisolated.txt to xbuildenv

6e4566b

New build environment version

b5f6b87

Merge branch 'main' into out-of-tree

fa0f5ea

Revert one change

1043040

ryanking13 reviewed Jul 5, 2022

View reviewed changes

rth reviewed Jul 6, 2022

View reviewed changes

hoodmane merged commit 03a05ab into pyodide:main Jul 6, 2022

hoodmane deleted the out-of-tree branch July 6, 2022 21:37

hoodmane mentioned this pull request Jul 10, 2022

Change out of tree build entrypoint name from pywasm to pyodide #2858

Merged

Uh oh!

Conversation

hoodmane commented Jul 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklists

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanking13 left a comment

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hoodmane commented Jul 5, 2022

Uh oh!

ryanking13 commented Jul 5, 2022

Uh oh!

hoodmane commented Jul 6, 2022

Uh oh!

rth left a comment

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!

Choose a reason for hiding this comment

Uh oh!

rth Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hoodmane commented Jul 6, 2022

Uh oh!

rth commented Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hoodmane commented Jul 1, 2022 •

edited

Loading

rth Jul 6, 2022 •

edited

Loading

rth commented Jul 6, 2022 •

edited

Loading