Skip to content

Update CMake to match hdf5 #816

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
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Update CMake to match hdf5 #816

wants to merge 2 commits into from

Conversation

byrnHDF
Copy link
Collaborator

@byrnHDF byrnHDF commented May 8, 2025

No description provided.

@byrnHDF byrnHDF self-assigned this May 8, 2025
bmribler
bmribler reviewed May 9, 2025
View reviewed changes
.github/workflows/netcdf-cmake.yml Outdated
@@ -65,23 +65,23 @@ jobs:
sudo cmake --install . --config Release --prefix="/usr/local"
working-directory: ${{ runner.workspace }}/build

- name: Checkout HDF5
- name: Checkout HDF4
uses: actions/[email protected]
with:
repository: HDFGroup/hdf5
Copy link
Collaborator

Choose a reason for hiding this comment

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

hdf4

.github/workflows/netcdf-cmake.yml Outdated
@@ -65,23 +65,23 @@ jobs:
sudo cmake --install . --config Release --prefix="/usr/local"
working-directory: ${{ runner.workspace }}/build

- name: Checkout HDF5
- name: Checkout HDF4
uses: actions/[email protected]
with:
repository: HDFGroup/hdf5
path: hdf5
Copy link
Collaborator

Choose a reason for hiding this comment

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

hdf4, few more below

.github/workflows/netcdf-cmake.yml Outdated
@@ -65,23 +65,23 @@ jobs:
sudo cmake --install . --config Release --prefix="/usr/local"
working-directory: ${{ runner.workspace }}/build

- name: Checkout HDF5
- name: Checkout HDF4
uses: actions/[email protected]
with:
repository: HDFGroup/hdf5
path: hdf5
ref: hdf5_1.14.5
Copy link
Collaborator

Choose a reason for hiding this comment

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

hdf?

CMakePresets.json
@@ -25,7 +25,7 @@
"ZLIB_TGZ_NAME": {"type": "STRING", "value": "zlib-1.3.1.tar.gz"},
"ZLIBNG_PACKAGE_NAME": {"type": "STRING", "value": "zlib-ng"},
"ZLIBNG_TGZ_ORIGPATH": {"type": "STRING", "value": "https://github.com/zlib-ng/zlib-ng/archive/refs/tags"},
Copy link
Collaborator

Choose a reason for hiding this comment

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

https://github.com/zlib-ng/zlib-ng/archive/refs/tags doesn't work

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not by itself but combined with filename it should

CMakePresets.json
@@ -25,7 +25,7 @@
"ZLIB_TGZ_NAME": {"type": "STRING", "value": "zlib-1.3.1.tar.gz"},
"ZLIBNG_PACKAGE_NAME": {"type": "STRING", "value": "zlib-ng"},
"ZLIBNG_TGZ_ORIGPATH": {"type": "STRING", "value": "https://github.com/zlib-ng/zlib-ng/archive/refs/tags"},
"ZLIBNG_TGZ_NAME": {"type": "STRING", "value": "2.2.2.tar.gz"},
"ZLIBNG_TGZ_NAME": {"type": "STRING", "value": "2.2.4.tar.gz"},
"LIBAEC_PACKAGE_NAME": {"type": "STRING", "value": "libaec"},
"LIBAEC_TGZ_ORIGPATH": {"type": "STRING", "value": "https://github.com/MathisRosenhauer/libaec/releases/download/v1.1.3"},
Copy link
Collaborator

Choose a reason for hiding this comment

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

https://github.com/MathisRosenhauer/libaec/releases/download/v1.1.3 doesn't work; maybe: https://github.com/MathisRosenhauer/libaec/releases/tag/v1.1.3?

HDF4Examples/config/cmake/jrunTest.cmake
set (ENV{LD_LIBRARY_PATH} "$ENV{LD_LIBRARY_PATH}:${TEST_LIBRARY_DIRECTORY}")
if (TEST_LIBRARY_DIRECTORY) # Directory to add to PATH
if (WIN32)
set (ENV{PATH} "$ENV{PATH};${TEST_LIBRARY_DIRECTORY}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Kinda inconsistent to have some with _DIR and some with _DIRECTORY, isn't it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TEST_LIBRARY_DIRECTORY has been around for a long time in these macros

config/cmake/libh4cc.in
echo " -showconfig Show the HDF4 library configuration summary"
echo " "
echo " <pkg-config line> - the pkg-config compile line options for the compiler"
echo " that was used to compile HDF4."
Copy link
Collaborator

Choose a reason for hiding this comment

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

"to compile HDF4."? What does that mean? HDF4 library? HDF4 application?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The compiler used to build HDF4.

config/cmake/libh4cc.in
export PKG_CONFIG_PATH=$dir/lib/pkgconfig
usage() {
# "How-to use" message.
echo "usage: $prg [OPTIONS] <pkg-config line>"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it $prg or $pkg? Is it short for program?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

$prg is the script; h4cc, etc

doxygen/aliases
@@ -99,7 +99,6 @@ ALIASES += fgdta_loc_obj_id{1}="\loc_obj_id{\1}. The identifier may be that of a
# Others
################################################################################

ALIASES += cpp_c_api_note="\attention \Bold{C++ Developers using HDF5 C-API functions beware:}\n Several functions in this C-API take function pointers or callbacks as arguments. Examples include H5Pset_elink_cb(), H5Pset_type_conv_cb(), H5Tconvert(), and H5Ewalk2(). Application code must ensure that those callback functions return normally such to allow the HDF5 to manage its resources and maintain a consistent state. For instance, those functions must not use the C \c setjmp / \c longjmp mechanism to leave those callback functions. Within the context of C++, any exceptions thrown within the callback function must be caught, such as with a \TText{catch(…)} statement. Any exception state can be placed within the provided user data function call arguments, and may be thrown again once the calling function has returned. Exceptions raised and not handled inside the callback are not supported as it might leave the HDF5 library in an inconsistent state. Similarly, using C++20 coroutines cannot be used as callbacks, since they do not support plain return statements. If a callback function yields execution to another C++20 coroutine calling HDF5 functions as well, this may lead to undefined behavior."
ALIASES += par_compr_note="\attention If you are planning to use compression with parallel HDF5, ensure that calls to H5Dwrite() occur in collective mode. In other words, all MPI ranks (in the relevant communicator) call H5Dwrite() and pass a dataset transfer property list with the MPI-IO collective option property set to #H5FD_MPIO_COLLECTIVE_IO.\n Note that data transformations are currently \Bold{not} supported when writing to datasets in parallel and with compression enabled."
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are a bunch more hdf5 in this file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Doxygen support was never finished, pending further development.

doxygen/dox/userguide/chap5.dox Outdated
@@ -1,4 +1,4 @@
/** @page CH5_UG Vgroups (V API)
/** @page CH4_UG Vgroups (V API)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be CH5_UG

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Runaway search and replace

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bmribler bmribler bmribler left review comments

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

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

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

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

@vchoi-hdfgroup vchoi-hdfgroup Awaiting requested review from vchoi-hdfgroup vchoi-hdfgroup 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

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

At least 1 approving review is required to merge this pull request.

Assignees

@byrnHDF byrnHDF

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@byrnHDF @bmribler