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

Set H5 specific vars immediately if legacy find #4512

Merged
merged 5 commits into from
May 22, 2024
Merged

Set H5 specific vars immediately if legacy find #4512

merged 5 commits into from
May 22, 2024

Conversation

byrnHDF
Copy link
Contributor

@byrnHDF byrnHDF commented May 21, 2024

No description provided.

@byrnHDF byrnHDF added Merge - To 1.14 Priority - 0. Blocker ⛔ This MUST be merged for the release to happen Component - Build CMake, Autotools Type - Bug / Bugfix Please report security issues to help@hdfgroup.org instead of creating an issue on GitHub labels May 21, 2024
@byrnHDF byrnHDF self-assigned this May 21, 2024
jhendersonHDF
jhendersonHDF reviewed May 21, 2024
View reviewed changes
CMakeFilters.cmake Outdated
@@ -75,6 +75,8 @@ if (HDF5_ENABLE_Z_LIB_SUPPORT)
find_package (ZLIB NAMES ${ZLIB_PACKAGE_NAME}${HDF_PACKAGE_EXT} COMPONENTS static shared)
if (NOT ZLIB_FOUND)
find_package (ZLIB) # Legacy find
set (H5_ZLIB_INCLUDE_DIR ${ZLIB_INCLUDE_DIRS})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't these also need to be set if the first find_package call succeeds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am confusing two different processes! No wonder it doesn't work

Copy link
Collaborator

Choose a reason for hiding this comment

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

Based on this refactoring, it even seems like the H5_ZLIB_FOUND and H5_SZIP_FOUND aren't really needed at a first glance, though there could be other places they're used that I'm not seeing in the limited diff context.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe - but let's make sure everything works first

@byrnHDF byrnHDF mentioned this pull request May 21, 2024
Closed
jhendersonHDF
jhendersonHDF reviewed May 21, 2024
View reviewed changes
CMakeFilters.cmake
@@ -10,8 +10,8 @@
# help@hdfgroup.org.
#
option (USE_LIBAEC_STATIC "Use static AEC library" OFF)
option (ZLIB_USE_EXTERNAL "Use External Library Building for ZLIB" OFF)
option (SZIP_USE_EXTERNAL "Use External Library Building for SZIP" OFF)
option (ZLIB_USE_EXTERNAL "Use External Library Building for ZLIB else search" OFF)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like "else search" doesn't give much information and would just make this even more confusing for users, but I don't have any particular evidence to back that up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

User sstill complained it was unclear and that is what happens search using find_package

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right, I'm saying maybe that needs to be even more explicit, such as "else search system with find_package", even though I still think that isn't necessarily helpful for most people that just want to build their HDF5 with zlib support and don't know what find_package is.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm confused - is that evidence? If it's OFF, what does that mean about "else search"? If it's ON, does it mean "Build and use designated External Library or use find_package to search system"?

@lrknox lrknox merged commit 4fa004e into HDFGroup:develop May 22, 2024
58 checks passed
lrknox pushed a commit to lrknox/hdf5 that referenced this pull request May 23, 2024
@byrnHDF @lrknox
Set H5 specific vars immediately if legacy find (HDFGroup#4512)
205a607 
* Set H5 specific vars immediately if legacy find

* Correct find process vars (vs in-line build)

* Correct SZIP find

* Everything is libaec 1.0.6 or newer

* Correct option help text
lrknox added a commit that referenced this pull request May 23, 2024
@lrknox
Sync develop changes May 6 - May 20 to hdf5_1_14 (#4503)
86f603e 
* win32defs: Fix Wundef warning (#4467)

* Refactor error handling code to eliminate internal ID calls (#4453)

All calls to the H5I routines are now made in API routines (sometimes in
FUNC_ENTER/LEAVE_* macros), except for some calls to H5E_clear_stack() within
the library, but I'm planning to remove those over time.

Also, made all the library internal error messages into static const variables,
instead of malloc'ing them, which means that they can just be referenced
and not copied.

Several new and updated auto-generated header files were necessary to enable
this.

* CMake: Fix mingw/fortran build (#4466)

* Update for blosc2 in plugins and prefix hdf5 cmake varnames (#4468)

* Fix an issue where compound datatype member IDs can be leaked during conversion (#4459)

Also fixes issues with handling of partially initialized datatypes during library shutdown

* H5Group: Fix operator= (#4473)

Closes #4472

* Fix github issue #2523: doxygen -- fix grammatically incorrect sentence alias (#4474)

* Remove env step not used by CI in testing (#4476)

* Add H5fortkit dependecy for H5Rff.F90 (#4482)

* Properly clean up cache when failing to load an object header (#4477)

* Properly clean up cache when failing to load an object header

* Don't check message type a second time in H5G__open_oid if the first attempt returns error

* Add more asserts to H5O__assert() to avoid segfaults

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

* Add a missing image from the original document (#4490)

* Disable EOF checks for SWMR readers in more cases. (#4489)

Fixes a race condition where the reader opens the file and sets its EOF from the
file's size (from the stat() call in the driver open callback).  Then, before
the reader can read the file's superblock, a SWMR writer races in, extends the
file, and closes the file, writing an updated superblock with the 'writer' and
'SWMR writer' flags in the superblock off (appropriately).  Then the reader
proceeds to read the superblock, and flags the EOF as wrong.  Taking out the
check for the 'writer' and 'SWMR writer' flags will cause SWMR readers to avoid
flagging the file as incorrect.

* Remove unnecessary fortran install (#4498)

* Only one version of binaries is produced for platforms (#4496)

* Fix for github issue #2220. (#4497)

Document the limitation in the Passthrough Conncector section of the VOL Connector Author Guide.
The limitation is posted by Neil in the github issue on Dec 22, 2022.

* Release asset tarballs with no version filenames (#4494)

* Improve spec. reading superblock into cache (a little) by using v2 size (#4491)

* Improve spec. reading superblock into cache (a little) by using v2 size

Instead of reading the absolute minimal possible, use the likely value of
a v2+ superblock w/8-byte addresses & lengths.

* Fix for github Issue #1388 can't delete renamed dense attribute with corder tracking enabled (#4462)

* Fix for github issue #1388: can't delete renamed dense attribute with corder tracking enabled

The problem occurs in step 3(b) below which will delete the attribute with corder x
from the creation order index v2 B-tree.

The rename sequence in H5A__dense_rename() occurs in the following order:
1) The old attribute with corder x was removed from the creation order index v2 B-tree
2) The new renamed attribute was inserted via H5A__dense_insert():
(a) insert the attribute with new name j into the name index v2 B-tree
(b) insert the attribute with corder x into the creation order index v2 B-tree
3) The old attribute was removed via H5A__dense_remove():
(a) remove the attribute with old name k from the name index v2 B-tree
(b) remove the attribute with coorder x from the creation order index v2 B-tree

Fix: deactivate the "corder_bt2_addr" field so that H5A__dense_remove()
won't delete the attribute with corder x from the creation order index v2 B-tree.

* Fix/revert a libtool sed hack (#4501)

* Revert "Remove Autotools sed hack (#3848)"

This reverts commit 8b3ffde.

* Fix libtool sed cleanup on MacOS

Convert sed -i line to sed > libtool.bak && mv libtool.bak libtool
to avoid non-portable -i option.

* Update src/H5public.h

* Set H5 specific vars immediately if legacy find (#4512)

* Correct find process vars (vs in-line build)

* Correct SZIP find

* Everything is libaec 1.0.6 or newer

* Correct option help text
lrknox pushed a commit to lrknox/hdf5 that referenced this pull request Jun 7, 2024
@byrnHDF @lrknox
Set H5 specific vars immediately if legacy find (HDFGroup#4512)
5a60e4c 
* Set H5 specific vars immediately if legacy find

* Correct find process vars (vs in-line build)

* Correct SZIP find

* Everything is libaec 1.0.6 or newer

* Correct option help text
derobins added a commit that referenced this pull request Jun 8, 2024
@lrknox @byrnHDF
@hyoklee @qkoziol @jhendersonHDF @mattjala @derobins @PeterC-DLS @dependabot @bmribler @brtnfld
Sync develop changes May 20 - June 6 to hdf5_1_14 (#4553)
ad50867 
* Fix daily-build CI and correct use of *_FOUND settings for filters (#4504)

* Correct examples tests to just run under dynamic analysis (#4505)

* Remove trailing extra whitespace in hyperlink (#4509)

* Set H5 specific vars immediately if legacy find (#4512)

* Set H5 specific vars immediately if legacy find

* Correct find process vars (vs in-line build)

* Correct SZIP find

* Everything is libaec 1.0.6 or newer

* Correct option help text

* Don't update 'pos' and 'op' fields when using pread/pwrite (#4492)

Instead of reading the absolute minimal possible, use the likely value of
a v2+ superblock w/8-byte addresses & lengths.

* Fix spelling (#4522)

* Fix typo in DAPL callback documentation (#4523)

* Move/rename libhdf5.settings input files (#4525)

Move without other changes:

src/libhdf5.settings.in -> src/libhdf5.settings.autotools.in
config/cmake/libhdf5.settings.cmake.in -> src/libhdf5.settings.cmake.in

* Disable UNITY_BUILD for now - globally (#4515)

* Fix function name in USAGE for H5Pencode2() (#4519)

* Allow HDF5_LIB_INFIX to work with DLL (#4500)

* Allow HDF5_LIB_INFIX to work with DLL

* Separate individual library name into parts and add suffix option

* Java cannot use alternative names and removed extra setting

* Incorporate the underscore into the CORE name

* Fix typos in property callback documentation (#4532)

* Fix wrong int type as some systems have int as 64-bit wide (#4534)

* H5FDquery return value (#4530)

* Switch H5FDquery() return values to use library's FAIL / SUCCEED macros

* Update return value also

* Refactor to reduce code duplication (#4531)

* Update error output w/new routine name

* Fix a few function names in USAGE comments that don't match the actual (#4533)

* Fix a few function names in USAGE comments that don't match the actual
function names.

* Remove typo '['

* Switch to working url for api-compatibility-macros.html.

* Remove julia CI actions (#4540)

These have been failing for a week or two for unclear reasons, both
in the Autotools and CMake. No obvious library changes triggered
this.

See GitHub issue #4539 for more info/discussion

The Julia tests will be disabled until the root cause is found.

* Bump the github-actions group with 3 updates (#4538)

Bumps the github-actions group with 3 updates: [softprops/action-gh-release](https://github.com/softprops/action-gh-release), [ossf/scorecard-action](https://github.com/ossf/scorecard-action) and [github/codeql-action](https://github.com/github/codeql-action).


Updates `softprops/action-gh-release` from 2.0.4 to 2.0.5
- [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@9d7c94c...69320db)

Updates `ossf/scorecard-action` from 2.3.1 to 2.3.3
- [Release notes](https://github.com/ossf/scorecard-action/releases)
- [Changelog](https://github.com/ossf/scorecard-action/blob/main/RELEASE.md)
- [Commits](ossf/scorecard-action@0864cf1...dc50aa9)

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

---
updated-dependencies:
- dependency-name: softprops/action-gh-release
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: github-actions
- dependency-name: ossf/scorecard-action
  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 various mistakes in doxygen docs (#4541)

* Fix a dead link and example file names

* Add the missing content of a section

* Export HDF5 parallel status for CMake FetchContent'ed VOL connectors (#4542)

* Remove an unnecessary check for parallel and thread-safety from examples (#4543)

* Add option to use zlib-ng as zlib library (#4487)

* Export HDF5 version for CMake FetchContent'ed VOL connectors (#4548)

* Adjust h5repack userblock option to allow reserve size (#4544)

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Allen Byrne <50328838+byrnHDF@users.noreply.github.com>
Co-authored-by: H. Joe Lee <hyoklee@hdfgroup.org>
Co-authored-by: Quincey Koziol <quincey@koziol.cc>
Co-authored-by: jhendersonHDF <jhenderson@hdfgroup.org>
Co-authored-by: mattjala <124107509+mattjala@users.noreply.github.com>
Co-authored-by: Dana Robinson <43805+derobins@users.noreply.github.com>
Co-authored-by: Peter Chang <peter.chang@diamond.ac.uk>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: bmribler <39579120+bmribler@users.noreply.github.com>
Co-authored-by: Scot Breitenfeld <brtnfld@hdfgroup.org>
@byrnHDF byrnHDF deleted the develop-fix-legacy branch June 9, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lrknox lrknox lrknox approved these changes

@jhendersonHDF jhendersonHDF jhendersonHDF 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

@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

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

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

Assignees

@byrnHDF byrnHDF

Labels
Component - Build CMake, Autotools 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 @lrknox @jhendersonHDF