hdf5 version in build_usd.py doesn't build with newer clang versions

jonlm commented 1 month ago

Description of Issue

Building on macOS Sonoma with XCode 15.3 and Alembic + HDF5 enabled fails with this error:

src/hdf5-1.10.0-patch1/src/H5Dint.c:2193:6: error: incompatible pointer to integer conversion assigning to 'herr_t' (aka 'int') from 'void *' [-Wint-conversion]
            HGOTO_ERROR(H5E_DATASET, H5E_CANTRESET, NULL, "unable to reset layout info")
            ^                                       ~~~~

I found some discussion of this same problem here:

"The -Wint-conversion warning diagnostic for implicit int <-> pointer conversions now defaults to an error in all C language modes. It may be downgraded to a warning with -Wno-error=int-conversion, or disabled entirely with -Wno-int-conversion." https://releases.llvm.org/15.0.0/tools/clang/docs/ReleaseNotes.html

The hdf5 version (1.10.0 patch 1) is from 2016, so this isn't too surprising. I tried replacing it with the latest version of 1.10, but in order for it to build I had to remove this chunk of code from build_usd.py since neither patch cleanly applies anymore, nor seems to be necessary:

    with CurrentWorkingDirectory(DownloadURL(HDF5_URL, context, force)):
        if MacOS():
            PatchFile("config/cmake_ext_mod/ConfigureChecks.cmake",
                    [("if (ARCH_LENGTH GREATER 1)", "if (FALSE)")])
            if context.targetUniversal:
                PatchFile("config/cmake/H5pubconf.h.in",
                        [(" #define H5_SIZEOF_LONG_LONG 8",
                        " #define H5_SIZEOF_LONG_LONG 8\n" +
                        " #define H5_SIZEOF_LONG_DOUBLE 16")])

At that point it did build fine, however I didn't test any hdf5 alembic files so I can't speak to the correctness of the build.

System Information (OS, Hardware)

macOS 14.5, XCode 15.3, clang "15.0.0 (clang-1500.3.9.4)"

Package Versions

USD 24.05

Build Flags

build_usd.py --build-variant release --openimageio --opencolorio --alembic --hdf5 --draco --materialx --ptex

sunyab commented 1 month ago

@jonlm Out of curiosity, did you need HDF5 support for a particular reason or were you just testing things out? I was under the impression that HDF5 wasn't really used anymore in favor of Ogawa.

jonlm commented 1 month ago

Don't specifically need it, was building it with everything turned on just for completeness. I agree that HDF5 Alembic files are very unlikely to be encountered by anyone who isn't pulling archival data. That said, the 2016 HDF5 version is probably not a great thing to be offering up as an option given that CVEs have been addressed, etc..

sunyab commented 1 month ago

Agreed -- given its lack of use I think I'd prefer to address this by dropping HDF5 support rather than updating the dependency, but I'll discuss this with folks. Thanks for the report!

dgovil commented 1 month ago

My 2¢ is that the folks who need legacy alembic support, are also the people most likely able to build Alembic themselves outside of the build_usd.py with it.

For timeline of use, Alembic was 2010 release. Ogawa was 2013 release. Adding in how long it takes people to adopt new tech, I think it's likely that very few people actually have significant HDF5 Alembics today.

I think it's worth removing HDF5 support at this point.

jesschimein commented 1 month ago

Filed as internal issue #USD-9834

PixarAnimationStudios / OpenUSD