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 hdf5 1.12.1 to 1.12.3 #4653

Closed
wants to merge 5 commits into from
Closed

Update hdf5 1.12.1 to 1.12.3 #4653

wants to merge 5 commits into from

Conversation

seanm
Copy link
Contributor

@seanm seanm commented May 8, 2024

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

seanm and others added 3 commits May 5, 2024 21:17
Code extracted from:

    https://github.com/HDFGroup/hdf5.git

at commit 106fb56936d1d785ca96147601a3608e433ddc3b (hdf5-1_12_3).
# By HDF5 Maintainers
* upstream-HDF5:
  HDF5 2023-11-06 (106fb569)

# Conflicts:
#	Modules/ThirdParty/HDF5/src/itkhdf5/CMakeFilters.cmake
#	Modules/ThirdParty/HDF5/src/itkhdf5/CMakeLists.txt
#	Modules/ThirdParty/HDF5/src/itkhdf5/config/cmake/ConfigureChecks.cmake
#	Modules/ThirdParty/HDF5/src/itkhdf5/config/cmake/HDFCompilerFlags.cmake
#	Modules/ThirdParty/HDF5/src/itkhdf5/config/cmake/HDFMacros.cmake
#	Modules/ThirdParty/HDF5/src/itkhdf5/config/cmake_ext_mod/ConfigureChecks.cmake
#	Modules/ThirdParty/HDF5/src/itkhdf5/config/cmake_ext_mod/HDFUseCXX.cmake
#	Modules/ThirdParty/HDF5/src/itkhdf5/hl/CMakeLists.txt
#	Modules/ThirdParty/HDF5/src/itkhdf5/src/CMakeLists.txt
Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

Looks good on a glance.

@dzenanz
Copy link
Member

dzenanz commented May 8, 2024

It looks like pleasing the CI will take some doing.

@dzenanz
Copy link
Member

dzenanz commented May 9, 2024

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
Copy link
Contributor Author

seanm commented May 9, 2024

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)

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

@dzenanz
Copy link
Member

dzenanz commented May 9, 2024

[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
Copy link
Member

thewtex commented May 20, 2024

@seanm please rebase on master

@thewtex
Copy link
Member

thewtex commented May 21, 2024

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

@seanm
Copy link
Contributor Author

seanm commented May 23, 2024

@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
Copy link
Member

thewtex commented May 29, 2024

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
Copy link
Contributor Author

seanm commented Jun 10, 2024

@thewtex I've tried creating another PR (#4716) but it looks the same. In fact my older ones do too (ex #4644).

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

@thewtex
Copy link
Member

thewtex commented Jun 10, 2024

@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
Copy link
Contributor Author

seanm commented Jun 10, 2024

./Utilities/SetupForDevelopment.sh

That worked I think, see this new PR: #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
Copy link
Member

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
Copy link
Contributor Author

seanm commented Jun 11, 2024

@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
Copy link
Member

thewtex commented Jun 13, 2024

@seanm yes, release-5.4 would be great.

@hjmjohnson
Copy link
Member

Superceeded by #4716

@hjmjohnson hjmjohnson closed this Jun 21, 2024
@jhlegarreta
Copy link
Member

Just a heads up: PR #4716 was still not created from a fork #4653 (comment).

@dzenanz dzenanz deleted the hdf5-1.12.3 branch June 21, 2024 13:17
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.

5 participants