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.

Sign up for GitHub

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

Jump to bottom

Fixed usage issue with FindZLIB.cmake module #4655

Merged
merged 5 commits into from
Jul 16, 2024
Merged

Fixed usage issue with FindZLIB.cmake module #4655

merged 5 commits into from
Jul 16, 2024

Conversation

byrnHDF
Copy link
Contributor

@byrnHDF byrnHDF commented Jul 16, 2024

No description provided.

@byrnHDF byrnHDF added Merge - To 1.14 Priority - 0. Blocker ⛔ This MUST be merged for the release to happen Component - C Library Core C library issues (usually in the src directory) Component - Build CMake, Autotools Type - Bug / Bugfix Please report security issues to help@hdfgroup.org instead of creating an issue on GitHub labels Jul 16, 2024
@byrnHDF byrnHDF self-assigned this Jul 16, 2024
@mattjala
Copy link
Contributor

This doesn't fix the VOL CI. From a run of VOL connectors CI against this change:

 /usr/bin/ld: /usr/lib/x86_64-linux-gnu/libz.a(deflate.o): warning: relocation against `z_errmsg' in read-only section `.text'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libz.a(zutil.o): relocation R_X86_64_PC32 against symbol `z_errmsg' can not be used when making a shared object; recompile with -fPIC
/usr/bin/ld: final link failed: bad value
collect2: error: ld returned 1 exit status
gmake[2]: *** [src/CMakeFiles/hdf5-shared.dir/build.make:5668: bin/libhdf5.so.1000.0.0] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:2705: src/CMakeFiles/hdf5-shared.dir/all] Error 2
gmake: *** [Makefile:166: all] Error 2

mattjala
mattjala requested changes Jul 16, 2024
View reviewed changes
Copy link
Contributor

@mattjala mattjala left a comment

Choose a reason for hiding this comment

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

Fix VOL CI

@byrnHDF
Copy link
Contributor Author

byrnHDF commented Jul 16, 2024

Fix VOL CI

But that is a different problem as it complains about -fPic. Which means that the zlib it is finding was not built with -fPIC .
The other possibility is that it is using a newer CMake and needs to override the search for static compression libraries.

@mattjala
Copy link
Contributor

mattjala commented Jul 16, 2024 •
edited
Loading

But that is a different problem as it complains about -fPic. Which means that the zlib it is finding was not built with -fPIC . The other possibility is that it is using a newer CMake and needs to override the search for static compression libraries.

I'm not sure I understand exactly what has changed and why such that the VOL CI should change. I don't believe any of the failing VOL connectors retrieve Zlib in any special way. It looks like this set of changes removes the search for a dynamic zlib that would have been built with fPIC - is this somehow necessary in order to create the desired targets and define their output name property?

mattjala
mattjala approved these changes Jul 16, 2024
View reviewed changes
Copy link
Contributor

@mattjala mattjala left a comment

Choose a reason for hiding this comment

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

The release note should probably mention the new HDF5_USE_ZLIB_STATIC and HDF5_USE_LIBAEC_STATIC options, but overall looks good

@byrnHDF
Copy link
Contributor Author

byrnHDF commented Jul 16, 2024

The release note should probably mention the new HDF5_USE_ZLIB_STATIC and HDF5_USE_LIBAEC_STATIC options, but overall looks good

Actually only the ZLIB one is new but that reminds me that it needs to be added to the INSTALL_CMake.txt file.

@lrknox lrknox merged commit b3e1d6b into HDFGroup:develop Jul 16, 2024
59 checks passed
qkoziol pushed a commit to qkoziol/hdf5 that referenced this pull request Jul 18, 2024
@byrnHDF @qkoziol
Fixed usage issue with FindZLIB.cmake module (HDFGroup#4655)
e3dd94f 
* Add a comment on the FindZLIB.cmake module usage

* Allow choice of static/shared compression libs for Find Module

* Added new option to INSTALL_CMake file and changed option text
@byrnHDF byrnHDF deleted the develop-find-cmake-fix branch July 19, 2024 15:21
lrknox pushed a commit to lrknox/hdf5 that referenced this pull request Jul 19, 2024
@byrnHDF @lrknox
Fixed usage issue with FindZLIB.cmake module (HDFGroup#4655)
cd40e50 
* Add a comment on the FindZLIB.cmake module usage

* Allow choice of static/shared compression libs for Find Module

* Added new option to INSTALL_CMake file and changed option text
lrknox added a commit that referenced this pull request Jul 19, 2024
@lrknox
Sync develop changes July 2 - July 19 to hdf5_1_14 (#4665)
da7f0f7 
* Test fixes for log-based vol (#4618)

* fixes to address failures in the log-based VOL

* moved file cleanup to tests proper

* skipped index API test if not supported

* Add 'try' parameter to H5Z_find, and remove calls to H5E_clear_stack() (#4609)

* Bump the github-actions group with 4 updates (#4620)

Bumps the github-actions group with 4 updates: [actions/checkout](https://github.com/actions/checkout), [aws-actions/configure-aws-credentials](https://github.com/aws-actions/configure-aws-credentials), [softprops/action-gh-release](https://github.com/softprops/action-gh-release) and [github/codeql-action](https://github.com/github/codeql-action).

Updates `actions/checkout` from 4.1.1 to 4.1.7
- [Release notes](https://github.com/actions/checkout/releases)
- [Commits](actions/checkout@v4.1.1...v4.1.7)

Updates `aws-actions/configure-aws-credentials` from 1 to 4
- [Release notes](https://github.com/aws-actions/configure-aws-credentials/releases)
- [Changelog](https://github.com/aws-actions/configure-aws-credentials/blob/main/CHANGELOG.md)
- [Commits](aws-actions/configure-aws-credentials@v1...v4)

Updates `softprops/action-gh-release` from 2.0.5 to 2.0.6
- [Release notes](https://github.com/softprops/action-gh-release/releases)
- [Changelog](https://github.com/softprops/action-gh-release/blob/master/CHANGELOG.md)
- [Commits](softprops/action-gh-release@69320db...a74c6b7)

Updates `github/codeql-action` from 3.25.7 to 3.25.11
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@f079b84...b611370)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: github-actions
- dependency-name: aws-actions/configure-aws-credentials
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: github-actions
- dependency-name: softprops/action-gh-release
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: github-actions
- dependency-name: github/codeql-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: github-actions
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Fix a stack size warning in ros3 VFD code (#4625)

Just makes a stack array dynamic. Valgrind shows no memory leaks.

* Reworked cleaning up API test files (#4626)

* Reworked cleaning up test files, only removing test files if present to account for skipped tests

* changed to using H5Fis_accessible

* update to full use of remove_test_file

* corrected offset type in C wrapper (#4622)

* Remove some internal use of API calls and H5E_BEGIN_TRY/H5E_END_TRY (#4624)

* Remove auto NSIS check because of issues with CI (#4646)

* Add python testing for examples (#4628)

* Clean up Fortran __float128 configure-time checks (#4649)

* Always use DECIMAL_DIG instead of LDBL_DIG. This was controlled by
  an ifdef that is always true in C99 or greater

It's confusing to use float.h C constants as variable names in
configure.ac and the PAC_FC_LDBL_DIG macro.

* Directly compare MY_FLT128_DIG and MY_LDBL_DIG

* Make uniform across CMake and Autotools
* Don't export quadmath.h variables to H5pubconf.h

* Feature/awesome (#4604)

* Added Doxygen Awesome and fixed a few quirks.

* Fixed unterminated strings.

* Added Doxygen Awesome by copy.

* Add tools usage text as doxygen  for Tools UG (#4602)

* Add h5* compiler wrapper testing for CMake #4605 (#4613)

* Add show option

* remove non-static libs and correct names of static libs

* Fixup the pkg-config libs and comp builds

* Fix commands and add fortran pkg-config test scripts

* Add help usage option

* Add temporary fix for ARM64 Mac _Float16 build failure (#4639)

* Correct H5VL_t ref count on H5O_refresh_metadata failure (#4636)

* Fix bad H5VL_t rc on H5O_refresh_metadata fail

* Decrement nrefs before raising error

* Update doxygen Learn Basics / example refs. Add Reference sections (#4640)

* Fixed messed up table captions.  (#4653)

* Fixed messed up table captions. Browsers don't seem to respect relative values for width. Hardcoding 800px for now.

* Fixed FetchContent usage for new CMake reqs. (#4650)

CMake version 3.30 changed the behavior of the FetchContent module to deprecate
the use of FetchContent_Populate() in favor of FetchContent_MakeAvailable(). Therefore,
the copying of HDF specialized CMakeLists.txt files to the dependent project's source
was implemented in the FetchContent_Declare() call.

* Fixed usage issue with FindZLIB.cmake module (#4655)

* Add a comment on the FindZLIB.cmake module usage

* Allow choice of static/shared compression libs for Find Module

* Added new option to INSTALL_CMake file and changed option text

* Eliminate more H5E_BEGIN/END_TRY macros and H5E_clear_stack() calls (#4648)

* Correct name of zlib_ng option (#4658)

* Fix the examples for testing java with binaries (#4660)

* Update filename in RELEASE_PROCESS.md to current name
INSTALL_autotools.txt.

* Remove reference to V116 in tools/src/h5repack/h5repack.h.
@tbeu tbeu mentioned this pull request Jul 24, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattjala mattjala mattjala approved these changes

@lrknox lrknox lrknox approved these changes

@derobins derobins Awaiting requested review from derobins derobins is a code owner

@fortnern fortnern Awaiting requested review from fortnern fortnern is a code owner

@jhendersonHDF jhendersonHDF Awaiting requested review from jhendersonHDF jhendersonHDF is a code owner

@qkoziol qkoziol Awaiting requested review from qkoziol qkoziol is a code owner

@vchoi-hdfgroup vchoi-hdfgroup Awaiting requested review from vchoi-hdfgroup vchoi-hdfgroup is a code owner

@bmribler bmribler Awaiting requested review from bmribler bmribler is a code owner

@glennsong09 glennsong09 Awaiting requested review from glennsong09 glennsong09 is a code owner

@brtnfld brtnfld Awaiting requested review from brtnfld brtnfld is a code owner

Assignees

@byrnHDF byrnHDF

Labels
Component - Build CMake, Autotools Component - C Library Core C library issues (usually in the src directory) Priority - 0. Blocker ⛔ This MUST be merged for the release to happen Type - Bug / Bugfix Please report security issues to help@hdfgroup.org instead of creating an issue on GitHub
Projects
Downstream HDF5 Merges
Status: Needs Merged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@byrnHDF @mattjala @lrknox