InsightSoftwareConsortium / ITK

Insight Toolkit (ITK) -- Official Repository. ITK builds on a proven, spatially-oriented architecture for processing, segmentation, and registration of scientific images in two, three, or more dimensions.
https://itk.org
Apache License 2.0
1.37k stars 660 forks source link

Update hdf5 1.12.1 to 1.12.3 #4653

Closed seanm closed 1 week ago

seanm commented 1 month ago

This smaller update seems easier to deal with than going to 1.14...

dzenanz commented 1 month ago

It looks like pleasing the CI will take some doing.

dzenanz commented 1 month ago
CMake Warning at Modules/ThirdParty/HDF5/src/itkhdf5/CMakeFilters.cmake:132 (message):
   ZLib support in HDF5 was enabled but not found
Call Stack (most recent call first):
  Modules/ThirdParty/HDF5/src/itkhdf5/CMakeLists.txt:886 (include)
seanm commented 1 month ago
CMake Warning at Modules/ThirdParty/HDF5/src/itkhdf5/CMakeFilters.cmake:132 (message):
   ZLib support in HDF5 was enabled but not found
Call Stack (most recent call first):
  Modules/ThirdParty/HDF5/src/itkhdf5/CMakeLists.txt:886 (include)

30127566fae48fbdc854d0095f32c9ef901a95d6 should fix that. It was OFF in ITK 5.3, I must have accidentally turned it on.

dzenanz commented 1 month ago
[4087/5894] Building C object Modules\ThirdParty\HDF5\src\itkhdf5\src\CMakeFiles\hdf5-shared.dir\H5Ztrans.c.obj
[4088/5894] Building C object Modules\ThirdParty\HDF5\src\itkhdf5\hl\src\CMakeFiles\hdf5_hl-shared.dir\H5DO.c.obj
[4089/5894] Building C object Modules\ThirdParty\HDF5\src\itkhdf5\hl\src\CMakeFiles\hdf5_hl-shared.dir\H5DS.c.obj
[4090/5894] Building C object Modules\ThirdParty\HDF5\src\itkhdf5\hl\src\CMakeFiles\hdf5_hl-shared.dir\H5IM.c.obj
[4091/5894] Linking C shared library bin\itkhdf5-shared-5.4.dll
FAILED: bin/itkhdf5-shared-5.4.dll lib/itkhdf5-shared-5.4.lib 
C:\Windows\system32\cmd.exe /C "cd . && "C:\Program Files\CMake\bin\cmake.exe" -E vs_link_dll --intdir=Modules\ThirdParty\HDF5\src\itkhdf5\src\CMakeFiles\hdf5-shared.dir --rc=C:\PROGRA~2\WI3CF2~1\10\bin\100226~1.0\x64\rc.exe --mt=C:\PROGRA~2\WI3CF2~1\10\bin\100226~1.0\x64\mt.exe --manifests  -- C:\PROGRA~2\MICROS~2\2019\ENTERP~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\link.exe /nologo @CMakeFiles\hdf5-shared.rsp  /out:bin\itkhdf5-shared-5.4.dll /implib:lib\itkhdf5-shared-5.4.lib /pdb:bin\itkhdf5-shared-5.4.pdb /dll /version:1.0 /machine:x64  /INCREMENTAL:NO && cd ."
LINK: command "C:\PROGRA~2\MICROS~2\2019\ENTERP~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\link.exe /nologo @CMakeFiles\hdf5-shared.rsp /out:bin\itkhdf5-shared-5.4.dll /implib:lib\itkhdf5-shared-5.4.lib /pdb:bin\itkhdf5-shared-5.4.pdb /dll /version:1.0 /machine:x64 /INCREMENTAL:NO /MANIFEST:EMBED,ID=2" failed (exit code 1120) with the following output:
   Creating library lib\itkhdf5-shared-5.4.lib and object lib\itkhdf5-shared-5.4.exp
H5Zdeflate.c.obj : error LNK2019: unresolved external symbol itkzlib_inflate referenced in function H5Z__filter_deflate
H5Zdeflate.c.obj : error LNK2019: unresolved external symbol itkzlib_inflateEnd referenced in function H5Z__filter_deflate
H5Zdeflate.c.obj : error LNK2019: unresolved external symbol itkzlib_compress2 referenced in function H5Z__filter_deflate

H5Zdeflate.c.obj : error LNK2019: unresolved external symbol itkzlib_inflateInit_ referenced in function H5Z__filter_deflate

bin\itkhdf5-shared-5.4.dll : fatal error LNK1120: 4 unresolved externals

[4092/5894] Building C object Modules\ThirdParty\HDF5\src\itkhdf5\hl\src\CMakeFiles\hdf5_hl-shared.dir\H5LT.c.obj
[4093/5894] Building C object Modules\ThirdParty\HDF5\src\itkhdf5\hl\src\CMakeFiles\hdf5_hl-shared.dir\H5LTanalyze.c.obj
ninja: build stopped: subcommand failed.
Command exited with the value: 1
MakeCommand:"C:\Program Files\CMake\bin\cmake.exe" --build . --config "MinSizeRel"
   5 Compiler errors
   1 Compiler warnings
thewtex commented 1 month ago

@seanm please rebase on master

thewtex commented 1 month ago

@seanm please also create PR's from forks -- we do not want to clutter the main repo with topic branches.

seanm commented 1 month ago

@seanm please also create PR's from forks -- we do not want to clutter the main repo with topic branches.

Mmmm, not sure what you mean here? I'm pretty sure I created this PR the same way I always do...

thewtex commented 1 month ago

The branch was pushed to the main InsightSoftwareConsortium/ITK repository:

image

We want to create the pull request from a forked repository, e.g.:

image

seanm commented 3 weeks ago

@thewtex I've tried creating another PR (https://github.com/InsightSoftwareConsortium/ITK/pull/4716) but it looks the same. In fact my older ones do too (ex https://github.com/InsightSoftwareConsortium/ITK/pull/4644).

I'm not sure what git sorcery is responsible nor how to fix it... thoughts?

thewtex commented 3 weeks ago

@seanm On thing to check: run

cd ITK ./Utilities/SetupForDevelopment.sh

And check that the

origin

Remote is set to seanm/ITK, not InsightSoftwareConsortium/ITK.

seanm commented 3 weeks ago

./Utilities/SetupForDevelopment.sh

That worked I think, see this new PR: https://github.com/InsightSoftwareConsortium/ITK/pull/4717

I for sure ran SetupForDevelopment.sh years ago. I guess something in it changed, or whatever it did was somehow lost... no idea... but thanks for noticing!

Now do I have to create this PR for a third time, or is there git magic to fix it up?

jhlegarreta commented 3 weeks ago

Now do I have to create this PR for a third time, or is there git magic to fix it up?

Open a new branch from your origin and cherry-pick the commits?

git checkout master
git fetch upstream
git merge upstream/master
git push origin master   # updating your local `master` so far
git checkout -b hdf5-1.12.3 origin/master
git cherry pick 4e15f8b
git cherry pick 3f96eed
git cherry pick b021190
git cherry pick 3012756
git cherry pick f6a87ec
git push origin hdf5-1.12.3 
# Open PR from your fork/branch

Assuming origin is set to https://github.com/seanm/ITK.git, and upstream is https://github.com/InsightSoftwareConsortium/ITK.git as set by SetupForDevelopment.sh (check with git remote -v).

seanm commented 3 weeks ago

@jhlegarreta thanks for that, will give it a try.

@thewtex actually, would this be appropriate for the 5.4 branch? On FreeBSD, there are build errors from HDF5 that I'm hoping this will fix.

thewtex commented 3 weeks ago

@seanm yes, release-5.4 would be great.

hjmjohnson commented 1 week ago

Superceeded by #4716

jhlegarreta commented 1 week ago

Just a heads up: PR #4716 was still not created from a fork https://github.com/InsightSoftwareConsortium/ITK/pull/4653#issuecomment-2122504033.