python / cpython

The Python programming language
https://www.python.org/
Other
60.9k stars 29.4k forks source link

Emscripten settings in configure.ac are not embedding friendly #120746

Open allsey87 opened 2 weeks ago

allsey87 commented 2 weeks ago

Bug report

Bug description:

I would like to be able to opt out of certain Emscripten options since I am embedded CPython in a larger application. For example, the forced inclusion of the JS file system is breaking my builds since I use WASMFS in my system which isn't compatible with -lidbfs.js, -lnodefs.js, -lproxyfs.js, -lworkerfs.js. Moreover, I would like to customise the TOTAL_MEMORY option.

Perhaps these options can be on by default but placed behind a flag that can turn them off? For example, --enable-wasm-emscripten-jsfs? Alternatively, perhaps all these settings can be disabled unless one of node, browser, node-debug, browser-debug is set via --with_emscripten_target?

Right now, it seems my only option is to apply a patch to turn these settings off which should be unnecessary in my opinion. @hoodmane @vstinner what do you think?

CPython versions tested on:

CPython main branch

Operating systems tested on:

Other

hoodmane commented 2 weeks ago

Well I think -sWASM_BIGINT should always be used. The other ones are pretty opinionated.

But these are all ldflags, are you using dynamic linking to load libpython3.xx.so? Pyodide builds libpython3.xx.a and then we control the linking entirely. If you are letting Python's makefile build libpython3.xx.so then are you passing the extra flags you need with EXTRA_LDFLAGS or some variable like that? Would be good to hear a bit more about what you are trying to do.

allsey87 commented 2 weeks ago

But these are all ldflags, are you using dynamic linking to load libpython3.xx.so? Pyodide builds libpython3.xx.a and then we control the linking entirely.

This is a good point. I am also building a staticlib so I am not sure why I am hitting this error now.

hoodmane commented 2 weeks ago

We are building the make target libpython3.xx.a specifically, which should prevent anything from getting linked.

allsey87 commented 2 weeks ago

I do the same, it might be this target that I run afterwards that is causing the linking:

make CROSS_COMPILE=yes inclinstall libinstall libainstall

libainstall installs libpython3.xx.a and (via patch) both libmpdec.a and libexpat.a.

hoodmane commented 2 weeks ago

We don't make libainstall so that's probably the one that is causing trouble. In trade, we have to do part of the install directory.

allsey87 commented 2 weeks ago

You are correct. The guilty dependency path is: libainstall -> all -> profile-opt which calls make on build_all -> python.js

In trade, we have to do part of the install directory.

I think I will just have to patch in a custom "libainstall" target since I am a bit more limited when working via configure_make.

allsey87 commented 2 weeks ago

Actually both libainstall and libinstall depend on all, so I suspect that python.js is built and linked in Pyodide and then just not used. It doesn't cause a problem there because you are not using WASMFS.

hoodmane commented 2 weeks ago

In my build folder, I don't find any evidence of a python.js I'm pretty sure we are not building it.

hoodmane commented 2 weeks ago

Looks to me like libinstall has no deps at all:

.PHONY: libinstall
libinstall:
    @for i in $(SCRIPTDIR) $(LIBDEST); \

OTOH libainstall does depend on all:

.PHONY: libainstall
libainstall: all scripts
    @for i in $(LIBDIR) $(LIBPL) $(LIBPC) $(BINDIR); \
allsey87 commented 2 weeks ago

https://github.com/python/cpython/blob/main/Makefile.pre.in#L2500

But hang on, I think I remember that you apply a patch that nukes these dependencies...

allsey87 commented 2 weeks ago

Well, not a patch, but a sed line: https://github.com/pyodide/pyodide/blob/main/cpython/Makefile#L43

hoodmane commented 2 weeks ago

I wonder if we should be doing that to libainstall and using that instead. What's the difference between libinstall and libainstall?

allsey87 commented 2 weeks ago

Good question, I was under the impression that libinstall was copying all of the .py files over to the install directory, but I'll have to check that.

I do the following in Makefile.pre.in to add the other staticlibs that we need to the libainstall target:

From 7557a0b12db8c894589a4329f4dc16e8a750c474 Mon Sep 17 00:00:00 2001
From: Michael Allwright <allsey87@gmail.com>
Date: Fri, 7 Jun 2024 15:55:16 +0200
Subject: [PATCH] Install vendored static libraries

---
 Makefile.pre.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makefile.pre.in b/Makefile.pre.in
index 68c55a356a..d43b5a1aa3 100644
--- a/Makefile.pre.in
+++ b/Makefile.pre.in
@@ -2193,6 +2193,8 @@ libainstall: all python-config
                    $(INSTALL_DATA) $(LDLIBRARY) $(DESTDIR)$(LIBPL) ; \
                else \
                    $(INSTALL_DATA) $(LIBRARY) $(DESTDIR)$(LIBPL)/$(LIBRARY) ; \
+                   $(INSTALL_DATA) -D $(LIBMPDEC_A) $(DESTDIR)$(LIBPL)/$(LIBMPDEC_A) ; \
+                   $(INSTALL_DATA) -D $(LIBEXPAT_A) $(DESTDIR)$(LIBPL)/$(LIBEXPAT_A) ; \
                fi; \
            else \
                echo Skip install of $(LIBRARY) - use make frameworkinstall; \
-- 
2.45.1
hoodmane commented 2 weeks ago

Right, I've had trouble with linking libmpdec, libexpat, and libhacl before. I'm not finding any special handling for them now in our Makefile, maybe @ryanking13 can remember what happened with that.

ryanking13 commented 2 weeks ago

Right, I've had trouble with linking libmpdec, libexpat, and libhacl before. I'm not finding any special handling for them now in our Makefile, maybe @ryanking13 can remember what happened with that.

We do handle them in Pyodide makefile. We patch cpython makefile to always link those libraries (code pointer)

LIBPYTHON_EXTRA_OBJECTS=$$(LIBMPDEC_OBJS) $$(LIBEXPAT_OBJS) $$(LIBHACL_SHA2_OBJS)

$(PYBUILD)/.patched_makefile:
    # Clear out libinstall deps (we build what we want explicitly first)
    cd $(PYBUILD) && sed -i -e 's/libinstall:.*/libinstall:/' Makefile;
    # Inject extra objects into libpython3.12.a so we don't have to link them
    # separately
    cd $(PYBUILD) && sed -i '/MODOBJS=/s/$$/ $(LIBPYTHON_EXTRA_OBJECTS)/' Makefile
    touch $(PYBUILD)/.patched_makefile