Conversation
We also update it to handle .so files that are not in the MEMFS. (In this case we have to copy the file into memory before loading it.)
Instead of trying to convince setup.py to do what we want, the wheel package empowers us to unpack and directly manipulate the wheel. In particular this is important because `--skip-build` really does not work. First we build a native wheel. If skip_host is True, then this "native" wheel has defective empty files as binaries, if it's False, we build a real native wheel. If skip_host is False, then we use pip to install our native wheel into host directory. Then we unpack the wheel, replace the platform tag with wasm32_emscripten, remove the non-metadata files, and copy in the new files with the cross compiled binaries. Of course, this is all only necessary for binary extensions. Hopefully this will help get us towards using alternative build backends. The one concern is that this approach depends on implementation details of where the build backend puts the files into the directory tree before copying them into the wheel. If the build backend moved the linked binaries after building them, we would be in trouble. Or worse if the build backend deleted the files after packing them into the wheel, we'd be in trouble. Basically, it seems clear that we can only support alternative build backends on a case-by-case basis.
This reverts commit 53900b0.
Rather than recording the first pass and then replaying it. Build systems are allowed to implement arbitrary logic
There was a problem hiding this comment.
Thanks for your effort in working on this @hoodmane! This change will definitely make understanding our build system much easier.
The overall changes LGTM, please update docs on our build system, and the changelog update that briefly describes changes in our build system would be great.
|
Thanks for the review @ryanking13! |
rth
left a comment
There was a problem hiding this comment.
Thanks for working on this @hoodmane, it looks very neat! Also thanks for the reviews @ryanking13 !
If there are no two stage builds anymore, wouldn't that mean that https://pyodide.org/en/latest/development/new-packages.html#partial-rebuilds need updating?
Also I don't really understand how building packages for host happens with this PR (e.g. numpy installed on host). We do have to run the build once for each arch don't we?
Maybe we could use this opportunity to document how cross-compilation works in broad terms in Pyodide, possibly by adding a page on "Build system" with a "Cross-compilation" subsection nested under https://pyodide.org/en/latest/development/new-packages.html (same as "The meta.yaml specification") or otherwise in the top level of "Development"?
| [ | ||
| "-Werror=implicit-function-declaration", | ||
| "-Werror=mismatched-parameter-types", | ||
| "-Werror=return-type", |
There was a problem hiding this comment.
Can we document this somewhere, under package building? Otherwise it's easy to forget that these are injected..
Our package build process currently has a significant flaw: we first run
setup.py, recording all compilation commands, then we rewrite these compilation commands to invokeemccand replay them, and then we pray that the cross compiled executables ended up in the right place to go into the wheel. This is not a good strategy because the build script is allowed to implement arbitrary logic, and if it moves, renames, etc any of the output files then we lose track of them. This has repeatedly caused difficulty for us.However, we also make no particularly significant use of the two pass approach. We can just do the simpler thing: capture the compiler commands as they occur, modify them as needed, and then run the fixed command.
I also added a patch to fix the numpy feature detection for wasm so that we don't have to include
_npyconfig.handconfig.h, numpy can generate them in the way it would for a native build. I opened a numpy PR that would fix the detection for us upstream:numpy/numpy#21154
This clears the way for us to switch to using pypa/build (as @henryiii has suggested) by removing our dependence on specific setuptools behavior.
This is on top of #2238.