Skip to content

Add HDF5 1.12.3#487

Closed
dipterix wants to merge 0 commit intor-wasm:mainfrom
dipterix:main
Closed

Add HDF5 1.12.3#487
dipterix wants to merge 0 commit intor-wasm:mainfrom
dipterix:main

Conversation

@dipterix
Copy link
Copy Markdown
Contributor

No description provided.

@dipterix
Copy link
Copy Markdown
Contributor Author

Hi I would like to get hdf5r compiled on webr repo. Is there any way I can test if this PR has all the components needed to install that package?

  • The HDF5 version is 1.12.3 because this is a version that both hdf5r and rhdf5 support.
  • In the hdf5/targets.mk, I don't know values to assign to OPTIONAL_HDF5_BINS. I put h5cc because it's the executable program filename.

@dipterix dipterix marked this pull request as ready for review September 23, 2024 04:06
Copy link
Copy Markdown
Member

@georgestagg georgestagg left a comment

Choose a reason for hiding this comment

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

Hi, thanks for taking the time to write this PR!

It looks like the hdf5 sources are not being built yet, because the make variables need to be named OPTIONAL_WASM_LIBS and OPTIONAL_WASM_BINS to be included in the make all command. Unfortunately, that means the tests have passed but not actually tested your changes.

Once I made that change, some issues have arisen. You should be able to see this yourself by renaming the make variables as suggested, then running:

cd libs; make all

I have made a first pass at correcting the issues, and included the required changes to get building up and running in this review.

I find that the build currently fails with the following:

LD_LIBRARY_PATH="$LD_LIBRARY_PATH`echo -L/Users/georgestagg/work/webr/wasm/lib -s USE_BZIP2=1 -s USE_ZLIB=1 -Oz -fwasm-exceptions -s SUPPORT_LONGJMP=wasm |                  \
                sed -e 's/-L/:/g' -e 's/ //g'`"                               \
         ./H5make_libsettings  H5lib_settings.c  ||                               \
            (test $HDF5_Make_Ignore && echo "*** Error ignored") ||          \
            (rm -f H5lib_settings.c ; exit 1)
/bin/sh: ./H5make_libsettings: Permission denied
make[2]: *** [H5lib_settings.c] Error 1
make[1]: *** [install-recursive] Error 1
emmake: error: 'make install' failed (returned 2)
make: *** [/Users/georgestagg/work/webr/wasm/lib/libhdf5.a] Error 1

Once the changes below have been applied, I will come back later and take another pass at getting hdf5 to compile for Wasm. Possibly H5make_libsettings is an intermediate binary and we just need to patch configure to run it through node, or something similar.

Once the resulting library is compiling and available, it should be that the R package "just works", but we will see.

Comment thread libs/recipes/hdf5/targets.mk Outdated
Comment thread libs/recipes/hdf5/rules.mk Outdated
Comment thread libs/recipes/hdf5/rules.mk Outdated
@georgestagg
Copy link
Copy Markdown
Member

georgestagg commented Oct 1, 2024

Is there any way we can use a newer version of HDF5? I have tested and am able to build v1.14.5, because there have been come changes in the HDF5 source that much improve the process of cross-compilation.

@dipterix
Copy link
Copy Markdown
Contributor Author

dipterix commented Oct 2, 2024

Please let me check if package hdf5r and rhdf5 work with the new versions. The latest header version for hdf5r is 1.12.0 and for rhdf5 is 1.10.0. I'm not sure whether newer versions will work.

Also when compiling the package, I encountered this error:

wasm-ld: error: unable to find library -lsz

Maybe I should make a PR to add libaec as well...

@georgestagg
Copy link
Copy Markdown
Member

Whoops, I messed that up!

Let's try again at #540.

@georgestagg
Copy link
Copy Markdown
Member

With #540 the hdf5r package now seems to work in the latest builds of webR at https://webr.r-wasm.org/latest.

Screenshot 2025-05-28 at 17 31 27

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.

2 participants