Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to emscripten 3.1.6 #2672

Merged
merged 20 commits into from
Jun 10, 2022
Merged

Update to emscripten 3.1.6 #2672

merged 20 commits into from
Jun 10, 2022

Conversation

hoodmane
Copy link
Member

@hoodmane hoodmane commented Jun 8, 2022

Issues:

  • test_console_html fails
  • _Py_DumpTraceback went missing
  • pathlib test failure
  • signal test failure
  • libgsl build fails
  • h5py import fails

Frequently when updating Python or Emscripten, a handlful of packages
stop working. Because of the way that the CI works, these prevent
other packages from being built or tested. However, packages with
many dependents are currently annoying to disable. This adds a key
to meta.yaml "disable: true" that turns off a package.

I also fixed the "!package" and no-numpy-dependents to be transitive.

We use graphlib to sort the packages topologically, then we traverse
the packages once in build order to locate all the packages which
transitively depend on disabled packages. Then we traverse in reverse
build order to locate all packages that are dependencies of non-disabled
requested packages.
@hoodmane
Copy link
Member Author

hoodmane commented Jun 8, 2022

@bmaranville I'm getting the following errors from HDF5 at import time. Any idea what it means?

Details
HDF5-DIAG: Error detected in HDF5 (1.13.1) thread 0:
  #000: /src/packages/libhdf5/build/libhdf5-1.13.1/src/H5.c line 1109 in H5open(): library initialization failed
    major: Function entry/exit
    minor: Unable to initialize object
  #001: /src/packages/libhdf5/build/libhdf5-1.13.1/src/H5.c line 286 in H5_init_library(): unable to initialize property list interface
    major: Function entry/exit
    minor: Unable to initialize object
  #002: /src/packages/libhdf5/build/libhdf5-1.13.1/src/H5Pint.c line 481 in H5P_init_phase1(): can't register properties
    major: Property lists
    minor: Unable to register new ID
  #003: /src/packages/libhdf5/build/libhdf5-1.13.1/src/H5Pocpl.c line 163 in H5P__ocrt_reg_prop(): can't insert property into class
    major: Property lists
    minor: Unable to insert object
  #004: /src/packages/libhdf5/build/libhdf5-1.13.1/src/H5Pint.c line 2236 in H5P__register_real(): Can't insert property into class
    major: Property lists
    minor: Unable to insert object
  #005: /src/packages/libhdf5/build/libhdf5-1.13.1/src/H5Pint.c line 1349 in H5P__add_prop(): can't insert property into skip list
    major: Property lists
    minor: Unable to insert object
  #006: /src/packages/libhdf5/build/libhdf5-1.13.1/src/H5SL.c line 1037 in H5SL_insert(): can't create new skip list node
    major: Skip Lists
    minor: Unable to insert object
  #007: /src/packages/libhdf5/build/libhdf5-1.13.1/src/H5SL.c line 744 in H5SL__insert_common(): can't create new skip list node
    major: Skip Lists
    minor: No space available for allocation
  #008: /src/packages/libhdf5/build/libhdf5-1.13.1/src/H5SL.c line 644 in H5SL__new_node(): memory allocation failed
    major: Skip Lists
    minor: No space available for allocation
  #009: /src/packages/libhdf5/build/libhdf5-1.13.1/src/H5FL.c line 2219 in H5FL_fac_malloc(): memory allocation failed
    major: Resource unavailable
    minor: No space available for allocation
  #010: /src/packages/libhdf5/build/libhdf5-1.13.1/src/H5FL.c line 245 in H5FL__malloc(): memory allocation failed for chunk
    major: Resource unavailable
    minor: No space available for allocation

@bmaranville
Copy link
Contributor

I've been told that the 1.13.x branches of libhdf5 are experimental and probably shouldn't be used for production builds. Try building the 1.12.1 version instead - the only reason we didn't use that version originally was (ironically) because it didn't properly compile with the old emscripten version being used before (2.0.17 or something). It compiles just fine with the newer emscripten versions.

diff --git a/packages/h5py/meta.yaml b/packages/h5py/meta.yaml
index c113b082..d761b4d7 100644
--- a/packages/h5py/meta.yaml
+++ b/packages/h5py/meta.yaml
@@ -23,4 +23,4 @@ build:
   script: |
     export HDF5_MPI=OFF
     export H5PY_SETUP_REQUIRES="0"
-    export HDF5_VERSION=1.13.1
+    export HDF5_VERSION=1.12.1
diff --git a/packages/h5py/patches/configure.patch b/packages/h5py/patches/configure.patch
index 7afbadc0..fce2e142 100644
--- a/packages/h5py/patches/configure.patch
+++ b/packages/h5py/patches/configure.patch
@@ -32,7 +32,7 @@ index 16c355b..85a4f90 100644
 -            raise
  
 -        return int(major.value), int(minor.value), int(release.value)
-+        return (1, 13, 1)
++        return (1, 12, 1)
  
      def load_function(self, func_name):
          try:
diff --git a/packages/libhdf5/meta.yaml b/packages/libhdf5/meta.yaml
index 2eab0bf2..c21be265 100644
--- a/packages/libhdf5/meta.yaml
+++ b/packages/libhdf5/meta.yaml
@@ -1,10 +1,10 @@
 package:
   name: libhdf5
-  version: 1.13.1
+  version: 1.12.1
 
 source:
-  sha256: 92552458f35c7e58128ce1bfc2831abf901cc142ea0fdd2b056311e4452db7bf
-  url: https://github.com/HDFGroup/hdf5/archive/refs/tags/hdf5-1_13_1.tar.gz
+  sha256: e6dde173c2d243551922d23a0387a79961205b018502e6a742acb30b61bc2d5f
+  url: https://github.com/HDFGroup/hdf5/archive/refs/tags/hdf5-1_12_1.tar.gz

"I've been told that the 1.13.x branches of libhdf5 are experimental and probably
shouldn't be used for production builds. Try building the 1.12.1 version instead
- the only reason we didn't use that version originally was (ironically) because
it didn't properly compile with the old emscripten version being used before
(2.0.17 or something). It compiles just fine with the newer emscripten versions."
@hoodmane
Copy link
Member Author

hoodmane commented Jun 8, 2022

Thanks!

@hoodmane
Copy link
Member Author

hoodmane commented Jun 8, 2022

Well v1.12.1 currently fails on import with a stack overflow.

@bmaranville
Copy link
Contributor

That's not good - I tried to build locally to see what was going on but something that changed in pyodide-build is making my local build environment not work anymore...

FileNotFoundError: [Errno 2] No such file or directory: '/home/brian/dev/pyodide-pr2672/cpython/installs/python-3.10.2/sysconfigdata/_sysconfigdata__emscripten_wasm32-emscripten.py'

@hoodmane
Copy link
Member Author

hoodmane commented Jun 8, 2022

I think you need to make -C cpython clean; make -C cpython.

@hoodmane
Copy link
Member Author

hoodmane commented Jun 8, 2022

I don't have symbols for h5py but the stack trace is:

RangeError: Maximum call stack size exceeded
    at __memcpy (emscripten_memcpy.c:29)
    at __fwritex (fwrite.c:23)
    at out (vfprintf.c:179)
    at printf_core (vfprintf.c:513)
    at __vfprintf_internal (vfprintf.c:739)
    at vfprintf (vfprintf.c:756)
    at vsnprintf (vsnprintf.c:54)
    at vasprintf (vasprintf.c:10)
    at 00cc9e02:0xc362a
    at 00cc9e02:0x84c68

Most likely H5open was trying to bail and it messed up and tried to format an infinite message.

@bmaranville
Copy link
Contributor

bmaranville commented Jun 8, 2022

I can't build with the version in this PR and a virtualenv in Ubuntu...
I rand make clean in both the cpython and emsdk folders, and then make -C cpython still gives this error:

make[2]: Leaving directory '/home/brian/dev/pyodide-pr2672/cpython/build/Python-3.10.2'
# Generate sysconfigdata. It outputs into a subfolder of build/, and
# the subfolder is written to pybuilddir.txt.
_PYTHON_SYSCONFIGDATA_NAME=_sysconfigdata__emscripten_wasm32-emscripten _PYTHON_PROJECT_BASE=/home/brian/dev/pyodide-pr2672/cpython/build/Python-3.10.2 /home/brian/.virtualenvs/pyodide/bin/python3.10 -m sysconfig --generate-posix-vars
Traceback (most recent call last):
  File "/home/brian/.local/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/home/brian/.local/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/home/brian/.local/lib/python3.10/sysconfig.py", line 803, in <module>
    _main()
  File "/home/brian/.local/lib/python3.10/sysconfig.py", line 791, in _main
    _generate_posix_vars()
  File "/home/brian/.local/lib/python3.10/sysconfig.py", line 415, in _generate_posix_vars
    makefile = get_makefile_filename()
  File "/home/brian/.local/lib/python3.10/sysconfig.py", line 399, in get_makefile_filename
    return os.path.join(get_path('stdlib'), config_dir_name, 'Makefile')
  File "/home/brian/.local/lib/python3.10/sysconfig.py", line 572, in get_path
    return get_paths(scheme, vars, expand)[name]
  File "/home/brian/.local/lib/python3.10/sysconfig.py", line 562, in get_paths
    return _expand_vars(scheme, vars)
  File "/home/brian/.local/lib/python3.10/sysconfig.py", line 218, in _expand_vars
    _extend_dict(vars, get_config_vars())
  File "/home/brian/.local/lib/python3.10/sysconfig.py", line 615, in get_config_vars
    _init_posix(_CONFIG_VARS)
  File "/home/brian/.local/lib/python3.10/sysconfig.py", line 477, in _init_posix
    _temp = __import__(name, globals(), locals(), ['build_time_vars'], 0)
ModuleNotFoundError: No module named '_sysconfigdata__emscripten_wasm32-emscripten'
make[1]: *** [Makefile:38: /home/brian/dev/pyodide-pr2672/cpython/installs/python-3.10.2/lib/libpython3.10.a] Error 1
make[1]: Leaving directory '/home/brian/dev/pyodide-pr2672/cpython'
make: *** [Makefile:226: /home/brian/dev/pyodide-pr2672/cpython/installs/python-3.10.2/lib/python3.10] Error 2

EDIT: in a clean miniconda environment, I seem to be able to build it. I will use that.

@hoodmane
Copy link
Member Author

hoodmane commented Jun 8, 2022

That's a weird error message. For some reason sysconfig is trying to import the file it is in the process of generating.

@bmaranville
Copy link
Contributor

So I can build in miniconda, but when I load up dist/console.html in my browser to test, it just spins forever. loadPyodide never seems to return. How are you loading pyodide for manual testing?

@hoodmane
Copy link
Member Author

hoodmane commented Jun 8, 2022

Sounds like you are doing the right thing and your build is still screwed up for some reason. Have you tried building in the docker image?

@bmaranville
Copy link
Contributor

The result of ./run_docker --pre-built and then make:

Digest: sha256:9db018c3487500055ebd337489fe3d064329631118f5f4aeb8b039f787fd359e
Status: Downloaded newer image for pyodide/pyodide:0.20.0
Waiting for initialization to finish...
brian@0d71c50ec554:/src$ make
./tools/dependency-check.sh
emcc -o src/core/docstring.o -c src/core/docstring.c -O2 -g0 -fPIC  -Wall -Wno-warn-absolute-paths -Werror=unused-variable -Werror=sometimes-uninitialized -Werror=int-conversion -Werror=incompatible-pointer-types -Werror=unused-result -I/src/cpython/installs/python-3.10.2/include/python3.10 -s EXCEPTION_CATCHING_ALLOWED=['we only want to allow exception handling in side modules'] -Isrc/core/
ccache: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version `GLIBCXX_3.4.26' not found (required by ccache)
make: *** [Makefile:183: src/core/docstring.o] Error 1
brian@0d71c50ec554:/src$ 

@hoodmane
Copy link
Member Author

hoodmane commented Jun 8, 2022

You need make -C emsdk clean; make -C emsdk. The problem is that emcc ccache is linked against a newer version of glibcxx than is present in the docker image.

@hoodmane hoodmane mentioned this pull request Jun 8, 2022
@hoodmane hoodmane mentioned this pull request Jun 9, 2022
@hoodmane
Copy link
Member Author

hoodmane commented Jun 9, 2022

@rth @ryanking13 I think this is ready for review. We can reenable h5py in a subsequent pr after merging this and #2679.

@hoodmane
Copy link
Member Author

hoodmane commented Jun 9, 2022

Well merging #2378 made this way harder. We may have to revert #2378 and wait on better Emscripten/Rust integration.

@rth
Copy link
Member

rth commented Jun 10, 2022

Yeah, I would say having the latest emscripten is more impactful for other PRs than having the rust build.

So +1 to revert #2378 for now and merge your emscripten update PRs first.

@hoodmane
Copy link
Member Author

Will do that once this is approved.

@hoodmane
Copy link
Member Author

hoodmane commented Jun 10, 2022

Actually rather than reverting I'll just disable cryptography here. I think this will lead to a more helpful git history. The rust-crypto pr was already a bit fat. And I think I know how to fix it in a follow up pr.

Copy link
Member

@rth rth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you can make the CI pass without reverting that works.

Code wise LGTM. Thanks again for doing this maintanance work!

@hoodmane hoodmane merged commit 52f27f0 into pyodide:main Jun 10, 2022
@hoodmane hoodmane deleted the emsdk-3.1.6-2 branch June 10, 2022 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants