From a7077c6e6ec3ce397c79111af5b82ec32cb5e94f Mon Sep 17 00:00:00 2001 From: Ward Fisher Date: Thu, 19 Sep 2024 14:56:35 -0500 Subject: [PATCH 1/8] Add configure option to enable potentially-problematic legacy macro _FillValue --- configure.ac | 11 +++++++++++ include/netcdf.h | 10 +++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 33c85470eb..cc84304889 100644 --- a/configure.ac +++ b/configure.ac @@ -462,6 +462,17 @@ else enable_set_log_level=no fi +# Do we want to enable unsafe macros, e.g. _FillValue in addition to NC_FillValue. +# See https://github.com/Unidata/netcdf-c/issues/3029 +AC_MSG_CHECKING([whether to allow unsafe macros]) +AC_ARG_ENABLE([unsafe-macros], + [AS_HELP_STRING([--enable-unsafe-macros], + [enable unsafe macros for backwards compatibility purposes. Use with caution.\ + This can lead to unexpected consequences/behavior.])]) +if test "x$enable_unsafe_macros" = xyes; then + AC_DEFINE([NETCDF_ENABLE_LEGACY_MACROS], 1, [If true, enable unsafe macros in netcdf.h]) +fi + # Does the user want to allow reading of remote data via range headers? AC_MSG_CHECKING([whether byte range support is enabled]) AC_ARG_ENABLE([byterange], diff --git a/include/netcdf.h b/include/netcdf.h index ceaeed2b03..1718ccdfab 100644 --- a/include/netcdf.h +++ b/include/netcdf.h @@ -109,7 +109,15 @@ extern "C" { * different value than the above defaults, create an attribute with * the same type as the variable and this reserved name. The value you * give the attribute will be used as the fill value for that - * variable. */ + * variable. + * Refactored to NC_FillValue in support of + * https://github.com/Unidata/netcdf-c/issues/2858, and parameterized + * behind an unsafe macros option as part of + * https://github.com/Unidata/netcdf-c/issues/3029 + */ +#ifdef NETCDF_ENABLE_LEGACY_MACROS +#define _FillValue "_FillValue" +#endif #define NC_FillValue "_FillValue" #define NC_FILL 0 /**< Argument to nc_set_fill() to clear NC_NOFILL */ #define NC_NOFILL 0x100 /**< Argument to nc_set_fill() to turn off filling of data. */ From de7992c0e01ea35095cf0ce48056260a9290cf43 Mon Sep 17 00:00:00 2001 From: Ward Fisher Date: Wed, 25 Sep 2024 11:34:10 -0500 Subject: [PATCH 2/8] Add option to enable legacy macros. --- CMakeLists.txt | 5 +++++ config.h.cmake.in | 3 +++ configure.ac | 12 ++++++------ 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 96852ed60a..3ff12b6957 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -547,6 +547,11 @@ else() set(NETCDF_ENABLE_HDF4 OFF) endif() +# Option legacy macros +# Do we want to enable unsafe macros, e.g. _FillValue in addition to NC_FillValue. +# See https://github.com/Unidata/netcdf-c/issues/3029 +option(NETCDF_ENABLE_LEGACY_MACROS "Enable legacy macros for backwards compatibility. Use with Caution." OFF) + # Option Logging, only valid for netcdf4 dispatchers. option(NETCDF_ENABLE_LOGGING "Enable Logging." OFF) if(NOT NETCDF_ENABLE_NETCDF4) diff --git a/config.h.cmake.in b/config.h.cmake.in index 3e0bff607e..b1b0507a2b 100644 --- a/config.h.cmake.in +++ b/config.h.cmake.in @@ -496,6 +496,9 @@ with zip */ /* Idspatch table version */ #cmakedefine NC_DISPATCH_VERSION ${NC_DISPATCH_VERSION} +/* Enable Legacy, potential-conflict Macro _FillValue */ +#cmakedefine NETCDF_ENABLE_LEGACY_MACROS + /* no IEEE float on this platform */ #cmakedefine NO_IEEE_FLOAT 1 diff --git a/configure.ac b/configure.ac index cc84304889..fba3fe5364 100644 --- a/configure.ac +++ b/configure.ac @@ -464,13 +464,13 @@ fi # Do we want to enable unsafe macros, e.g. _FillValue in addition to NC_FillValue. # See https://github.com/Unidata/netcdf-c/issues/3029 -AC_MSG_CHECKING([whether to allow unsafe macros]) -AC_ARG_ENABLE([unsafe-macros], - [AS_HELP_STRING([--enable-unsafe-macros], - [enable unsafe macros for backwards compatibility purposes. Use with caution.\ +AC_MSG_CHECKING([whether to allow legacy macros]) +AC_ARG_ENABLE([legacy-macros], + [AS_HELP_STRING([--enable-legacy-macros], + [enable legacy macros for backwards compatibility purposes. Use with caution.\ This can lead to unexpected consequences/behavior.])]) -if test "x$enable_unsafe_macros" = xyes; then - AC_DEFINE([NETCDF_ENABLE_LEGACY_MACROS], 1, [If true, enable unsafe macros in netcdf.h]) +if test "x$enable_legacy_macros" = xyes; then + AC_DEFINE([NETCDF_ENABLE_LEGACY_MACROS], 1, [If true, enable legacy macros in netcdf.h]) fi # Does the user want to allow reading of remote data via range headers? From a222922d7bd5b0a7452d0075d911f646b82571dd Mon Sep 17 00:00:00 2001 From: Ward Fisher Date: Wed, 25 Sep 2024 12:15:55 -0500 Subject: [PATCH 3/8] Add legacy macro option to cmake build system. Changed netcdf_plugin_install_dir to use /usr/local/hdf5/lib/plugin by default unless a 'prefix' is specified by the user, in which case it becomes prefix/hdf5/lib/plugin. This can still be overridden with the build flags appropriate to the build system. --- CMakeLists.txt | 19 ++++++++++++------- configure.ac | 11 +++++++++-- libnetcdf.settings.in | 1 + 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3ff12b6957..d2d99b501c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -667,14 +667,18 @@ if(ENABLE_PLUGIN_INSTALL) set(NETCDF_PLUGIN_INSTALL_DIR "$ENV{HDF5_PLUGIN_PATH}") else() if(ISMSVC OR ISMINGW) - set(NETCDF_PLUGIN_INSTALL_DIR "$ENV{ALLUSERSPROFILE}\\hdf5\\lib\\plugin") - else() - set(NETCDF_PLUGIN_INSTALL_DIR "/usr/local/hdf5/lib/plugin") - endif() - endif() - message(STATUS "Defaulting to -DPLUGIN_INSTALL_DIR=${NETCDF_PLUGIN_INSTALL_DIR}") + set(NETCDF_PLUGIN_INSTALL_DIR "$ENV{ALLUSERSPROFILE}\\hdf5\\lib\\plugin") + else() + if(NOT DEFINED CMAKE_INSTALL_PREFIX) + set(NETCDF_PLUGIN_INSTALL_DIR "/usr/local/hdf5/lib/plugin") + else() + set(NETCDF_PLUGIN_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/hdf5/lib/plugin") + endif(NOT DEFINED CMAKE_INSTALL_PREFIX) + endif(ISMSVC OR ISMINGW) + endif(DEFINED ENV{HDF5_PLUGIN_PATH}) + message(STATUS "Defaulting to -DPLUGIN_INSTALL_DIR=${NETCDF_PLUGIN_INSTALL_DIR}") endif() -endif() +endif(ENABLE_PLUGIN_INSTALL) if(ENABLE_PLUGIN_INSTALL) # Use the lowest priority dir in the path @@ -1699,6 +1703,7 @@ is_enabled(NETCDF_ENABLE_PARALLEL4 HAS_PARALLEL4) is_enabled(NETCDF_ENABLE_DAP HAS_DAP) is_enabled(NETCDF_ENABLE_DAP2 HAS_DAP2) is_enabled(NETCDF_ENABLE_DAP4 HAS_DAP4) +is_enabled(NETCDF_ENABLE_LEGACY_MACROS HAS_LEGACY_MACROS) is_enabled(NETCDF_ENABLE_BYTERANGE HAS_BYTERANGE) is_enabled(NETCDF_ENABLE_DISKLESS HAS_DISKLESS) is_enabled(USE_MMAP HAS_MMAP) diff --git a/configure.ac b/configure.ac index fba3fe5364..4894430924 100644 --- a/configure.ac +++ b/configure.ac @@ -1943,6 +1943,7 @@ AM_CONDITIONAL(USE_DAP, [test "x$enable_dap" = xyes]) # Alias # Provide protocol specific flags AM_CONDITIONAL(NETCDF_ENABLE_DAP, [test "x$enable_dap" = xyes]) AM_CONDITIONAL(NETCDF_ENABLE_DAP4, [test "x$enable_dap4" = xyes]) +AM_CONDITIONAL(NETCDF_ENABLE_LEGACY_MACROS, [test "x$netcdf_enable_legacy_macros" = xyes]) AM_CONDITIONAL(USE_STRICT_NULL_BYTE_HEADER_PADDING, [test x$enable_strict_null_byte_header_padding = xyes]) AM_CONDITIONAL(NETCDF_ENABLE_CDF5, [test "x$enable_cdf5" = xyes]) AM_CONDITIONAL(NETCDF_ENABLE_DAP_REMOTE_TESTS, [test "x$enable_dap_remote_tests" = xyes]) @@ -2061,6 +2062,7 @@ AC_SUBST(NC_LIBS,[$NC_LIBS]) AC_SUBST(HAS_DAP,[$enable_dap]) AC_SUBST(HAS_DAP2,[$enable_dap]) AC_SUBST(HAS_DAP4,[$enable_dap4]) +AC_SUBST(HAS_LEGACY_MACROS,[$enable_legacy_macros]) AC_SUBST(HAS_NC2,[$nc_build_v2]) AC_SUBST(HAS_CDF5,[$enable_cdf5]) AC_SUBST(HAS_HDF4,[$enable_hdf4]) @@ -2167,8 +2169,13 @@ elif test "x$with_plugin_dir" = xyes ; then # --with-plugin-dir, no argument if test "x$ISMSVC" = xyes || test "x$ISMINGW" = xyes; then PLUGIN_PATH="${ALLUSERSPROFILE}\\hdfd5\\lib\\plugin" else - PLUGIN_PATH="/usr/local/hdf5/lib/plugin" - fi + if test "x${prefix}" = x ; then + PLUGIN_PATH="/usr/local/hdf5/lib/plugin" + else + PLUGIN_PATH="${prefix}/hdf5/lib/plugin" + fi + + fi fi # Use the lowest priority dir in the path if test "x$ISMSVC" = xyes || test "x$ISMINGW" = xyes; then diff --git a/libnetcdf.settings.in b/libnetcdf.settings.in index 4cf899a47b..4aac31c1c9 100644 --- a/libnetcdf.settings.in +++ b/libnetcdf.settings.in @@ -24,6 +24,7 @@ Shared Library: @enable_shared@ Static Library: @enable_static@ Extra libraries: @LIBS@ XML Parser: @XMLPARSER@ +Legacy Macros: @HAS_LEGACY_MACROS@ # Features -------- From 5f09bbf492e6fd2be39768812f1fc66a3b24e2f9 Mon Sep 17 00:00:00 2001 From: Ward Fisher Date: Thu, 26 Sep 2024 17:38:23 -0500 Subject: [PATCH 4/8] Ensure absolute paths in rpath for plugins. --- plugins/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/Makefile.am b/plugins/Makefile.am index b9fc2d177c..b92e3095b5 100644 --- a/plugins/Makefile.am +++ b/plugins/Makefile.am @@ -14,7 +14,7 @@ AM_LDFLAGS += -module -avoid-version -shared -export-dynamic $(NOUNDEFINED) # Create an alternate directory if not installing. ALTPLUGINDIR = ${abs_top_builddir}/plugins/plugindir -RPATH = -rpath $(abs_builddir)/.libs +RPATH = -rpath $(abs_top_builddir)/.libs # This is where the plugins are to be installed if ENABLE_PLUGIN_DIR From 0f92d4c0f6b2ee8b12707f155ca313c1950fba24 Mon Sep 17 00:00:00 2001 From: Ward Fisher Date: Thu, 26 Sep 2024 16:52:05 -0600 Subject: [PATCH 5/8] Clean up a couple of logic errors resulting in github action failures. --- configure.ac | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 4894430924..2972e3e0d3 100644 --- a/configure.ac +++ b/configure.ac @@ -469,6 +469,7 @@ AC_ARG_ENABLE([legacy-macros], [AS_HELP_STRING([--enable-legacy-macros], [enable legacy macros for backwards compatibility purposes. Use with caution.\ This can lead to unexpected consequences/behavior.])]) +test "x$enable_legacy_macros" = xyes || enable_legacy_macros=no if test "x$enable_legacy_macros" = xyes; then AC_DEFINE([NETCDF_ENABLE_LEGACY_MACROS], 1, [If true, enable legacy macros in netcdf.h]) fi @@ -2169,7 +2170,7 @@ elif test "x$with_plugin_dir" = xyes ; then # --with-plugin-dir, no argument if test "x$ISMSVC" = xyes || test "x$ISMINGW" = xyes; then PLUGIN_PATH="${ALLUSERSPROFILE}\\hdfd5\\lib\\plugin" else - if test "x${prefix}" = x ; then + if test "x${prefix}" = xNONE ; then PLUGIN_PATH="/usr/local/hdf5/lib/plugin" else PLUGIN_PATH="${prefix}/hdf5/lib/plugin" From dc0634c6e039b1eca3f4e0c481871d8e0d49e661 Mon Sep 17 00:00:00 2001 From: Ward Fisher Date: Fri, 27 Sep 2024 14:22:59 -0600 Subject: [PATCH 6/8] Toggle legacy macros to on, for now. --- CMakeLists.txt | 2 +- configure.ac | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d2d99b501c..8c3e9eeaee 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -550,7 +550,7 @@ endif() # Option legacy macros # Do we want to enable unsafe macros, e.g. _FillValue in addition to NC_FillValue. # See https://github.com/Unidata/netcdf-c/issues/3029 -option(NETCDF_ENABLE_LEGACY_MACROS "Enable legacy macros for backwards compatibility. Use with Caution." OFF) +option(NETCDF_ENABLE_LEGACY_MACROS "Enable legacy macros for backwards compatibility. Use with Caution." ON) # Option Logging, only valid for netcdf4 dispatchers. option(NETCDF_ENABLE_LOGGING "Enable Logging." OFF) diff --git a/configure.ac b/configure.ac index 2972e3e0d3..fb6642281d 100644 --- a/configure.ac +++ b/configure.ac @@ -466,10 +466,11 @@ fi # See https://github.com/Unidata/netcdf-c/issues/3029 AC_MSG_CHECKING([whether to allow legacy macros]) AC_ARG_ENABLE([legacy-macros], - [AS_HELP_STRING([--enable-legacy-macros], + [AS_HELP_STRING([--disable-legacy-macros], [enable legacy macros for backwards compatibility purposes. Use with caution.\ This can lead to unexpected consequences/behavior.])]) -test "x$enable_legacy_macros" = xyes || enable_legacy_macros=no +test "x$enable_legacy_macros" = xno || enable_legacy_macros=yes +AC_MSG_RESULT($enable_legacy_macros) if test "x$enable_legacy_macros" = xyes; then AC_DEFINE([NETCDF_ENABLE_LEGACY_MACROS], 1, [If true, enable legacy macros in netcdf.h]) fi From 28ff401f9f5c68c29442b940d0afca4856890452 Mon Sep 17 00:00:00 2001 From: Ward Fisher Date: Fri, 27 Sep 2024 15:51:44 -0600 Subject: [PATCH 7/8] Updated release notes. --- RELEASE_NOTES.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index cfcaa3149c..5ec1be7443 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -9,12 +9,16 @@ This file contains a high-level description of this package's evolution. Release ## Known Issue -* Parallel operation using `mpich 4.2.0` (the default on `Ubuntu 24.04`) results in 'unexpected results' when running `nc_test4/run_par_test.sh`. This can be fixed by removing `mpich` and associated libraries and development packages and installing `mpich 4.2.2` by hand, or by using `openmpi` provided via `apt`. +> Parallel operation using `mpich 4.2.0` (the default on `Ubuntu 24.04`) results in 'unexpected results' when running `nc_test4/run_par_test.sh`. This can be fixed by removing `mpich` and associated libraries and development packages and installing `mpich 4.2.2` by hand, or by using `openmpi` provided via `apt`. ## Release Notes ### Release Candidate 2 - TBD +> Note: To avoid a conflict between `_FillValue` and `libc++18`, we have introduced a new option, `--enable-legacy-macros` for autotools and `NETCDF_ENABLE_LEGACY_MACROS` for cmake. These are turned on by default currently but will be turned off eventually. Developers are encouraged to move away from the `FillValue` macro and replace it with the new `NC_FillValue` macro. See [Github #2858](https://github.com/Unidata/netcdf-c/issues/2858) for more information. + + + * Provide better documentation for the .rc file mechanism and API. See [Github #2956](https://github.com/Unidata/netcdf-c/pull/2956) for more information. * Convert NCZarr V2 to store all netcdf-4 specific info as attributes. This improves interoperability with other Zarr implementations by no longer using non-standard keys. The price to be paid is that lazy attribute reading cannot be supported. See [Github #2836](https://github.com/Unidata/netcdf-c/pull/2936) for more information. * Cleanup the option code for NETCDF_ENABLE_SET_LOG_LEVEL\[_FUNC\] See [Github #2931](https://github.com/Unidata/netcdf-c/pull/2931) for more information. From b03d1373b94ce89cdbe39215effe8dd0a90b8e7e Mon Sep 17 00:00:00 2001 From: Ward Fisher Date: Fri, 27 Sep 2024 15:56:05 -0600 Subject: [PATCH 8/8] Amend annoying whitespace. --- RELEASE_NOTES.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 5ec1be7443..dce1f1291b 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -17,8 +17,6 @@ This file contains a high-level description of this package's evolution. Release > Note: To avoid a conflict between `_FillValue` and `libc++18`, we have introduced a new option, `--enable-legacy-macros` for autotools and `NETCDF_ENABLE_LEGACY_MACROS` for cmake. These are turned on by default currently but will be turned off eventually. Developers are encouraged to move away from the `FillValue` macro and replace it with the new `NC_FillValue` macro. See [Github #2858](https://github.com/Unidata/netcdf-c/issues/2858) for more information. - - * Provide better documentation for the .rc file mechanism and API. See [Github #2956](https://github.com/Unidata/netcdf-c/pull/2956) for more information. * Convert NCZarr V2 to store all netcdf-4 specific info as attributes. This improves interoperability with other Zarr implementations by no longer using non-standard keys. The price to be paid is that lazy attribute reading cannot be supported. See [Github #2836](https://github.com/Unidata/netcdf-c/pull/2936) for more information. * Cleanup the option code for NETCDF_ENABLE_SET_LOG_LEVEL\[_FUNC\] See [Github #2931](https://github.com/Unidata/netcdf-c/pull/2931) for more information.