netcdf-c icon indicating copy to clipboard operation
netcdf-c copied to clipboard

Why not use H5_DEFAULT_PLUGINDIR to set the plugin directory?

Open opoplawski opened this issue 2 years ago • 8 comments

hdf5 defines H5_DEFAULT_PLUGINDIR in H5pubconf.h, why not use it to set the plugin path by default?

opoplawski avatar Jun 25 '22 13:06 opoplawski

Because you are the first to suggest it! ;-)

I did not even know it existed. Are you suggesting we take whatever value of H5_DEFAULT_PLUGINDIR is in h5pubconf.h and install plugins there?

edwardhartnett avatar Jun 25 '22 14:06 edwardhartnett

That would be my suggestion for a default value. That is where hdf5 will look in the absence of HDF5_PLUGIN_PATH.

opoplawski avatar Jun 25 '22 15:06 opoplawski

I looked at the code. The flag H5_DEFAULT_PLUGINDIR is intended as an HDF5 CMake build option of the form:

-DH5_DEFAULT_PLUGINDIR:PATH=location

It does not to be ever used as an environment variable.

DennisHeimbigner avatar Jun 25 '22 21:06 DennisHeimbigner

I never said it was an environment variable.

opoplawski avatar Jun 26 '22 03:06 opoplawski

The H5_DEFAULT_PLUGINDIR value in /usr/include/hdf5/serial/H5pubconf.h is also exposed through pkg-config:

$ grep H5_DEFAULT_PLUGINDIR /usr/include/hdf5/serial/H5pubconf.h 
#define H5_DEFAULT_PLUGINDIR "/usr/lib/x86_64-linux-gnu/hdf5/serial/plugins"

$ pkg-config --variable=PluginDir hdf5
/usr/lib/x86_64-linux-gnu/hdf5/serial/plugins

Although this is a customization in the hdf5 Debian package.

Extracting the value from the include file is most reliable, e.g.:

$ cat hdf5-plugin-dir.c
#include <stdio.h>
#include "H5pubconf.h"

int main () {
    printf("%s\n", H5_DEFAULT_PLUGINDIR);
}
$ gcc -I /usr/include/hdf5/serial hdf5-plugin-dir.c && ./a.out 
/usr/lib/x86_64-linux-gnu/hdf5/serial/plugins

sebastic avatar Jun 26 '22 04:06 sebastic

I do not understand your point. Even if exported as an environment variable, it is never used in the code at run-time. It is only used when hdf5 is built.

DennisHeimbigner avatar Jun 26 '22 18:06 DennisHeimbigner

As mentioned in #2429:

The default path (/usr/local/hdf5/lib/plugin) is also incorrect when using -DCMAKE_INSTALL_PREFIX=/usr and -DCMAKE_INSTALL_LIBDIR=lib/x86_64-linux-gnu. A better default would be ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}/hdf5/plugin.

/usr/local/hdf5/lib/plugin is a bad default value:

IF(ENABLE_PLUGIN_INSTALL AND PLUGIN_INSTALL_DIR STREQUAL "YES")
    # Default to last dir (lowest search priority) in HDF5_PLUGIN_PATH
    IF(DEFINED ENV{HDF5_PLUGIN_PATH})
      SET(PLUGIN_PATH "$ENV{HDF5_PLUGIN_PATH}")
    ELSE()
        IF(ISMSVC OR ISMINGW)
          SET(PLUGIN_PATH "$ENV{ALLUSERSPROFILE}\\hdf5\\lib\\plugin")
        ELSE()
          SET(PLUGIN_PATH "/usr/local/hdf5/lib/plugin")
        ENDIF()
    ENDIF()
ENDIF()
# If user wants, then install selected standard filters
AC_MSG_CHECKING([whether and where we should install plugins])
AC_ARG_WITH([plugin-dir], [AS_HELP_STRING([--with-plugin-dir=<absolute directory>|no|--without-plugin-dir],
              [Install selected standard filters in specified or default directory])],
              [],[with_plugin_dir=no])
AC_MSG_RESULT([$with_plugin_dir])
if test "x$with_plugin_dir" = xno ; then # option missing|disabled
    with_plugin_dir=no
    with_plugin_dir_setting="N.A."
    enable_plugin_dir=no
elif test "x$with_plugin_dir" = xyes ; then # --with-plugin-dir, no argument
    # Default to last dir (lowest search priority) in HDF5_PLUGIN_PATH
    PLUGIN_PATH="$HDF5_PLUGIN_PATH"
    if test "x${PLUGIN_PATH}" = x ; then
        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
    fi
    # Use the lowest priority dir in the path
    if test "x$ISMSVC" = xyes || test "x$ISMINGW" = xyes; then
      PLUGIN_DIR=`echo "$PLUGIN_PATH" | tr ';' ' '`
    else
      PLUGIN_DIR=`echo "$PLUGIN_PATH" | tr ':' ' '`
    fi
    for pp in ${PLUGIN_DIR} ; do last="$pp"; done
    PLUGIN_DIR="$last"
    with_plugin_dir_setting="$PLUGIN_DIR"
    # canonical form is all forward slashes
    with_plugin_dir=`echo "$PLUGIN_DIR" | tr '\\\\' '/'`
    enable_plugin_dir=yes
    AC_MSG_NOTICE([Defaulting to --with-plugin-dir=$with_plugin_dir])
else # --with-plugin-dir=<dir|path>
    with_plugin_dir_setting="$with_plugin_dir"
    enable_plugin_dir=yes
fi

These parts of the CMake and Autotools build systems should be updated to use the value of H5_DEFAULT_PLUGINDIR and only if that is not set/available fall back to hardcode /usr/local/hdf5/lib/plugin.

Using the value of H5_DEFAULT_PLUGINDIR will make it work for most cases, instead of requiring HDF5_PLUGIN_PATH to be set because the default value is bad.

sebastic avatar Jun 26 '22 18:06 sebastic

I see my confusion. When you said:

That would be my suggestion for a default value. That is where hdf5 will look in the absence of HDF5_PLUGIN_PATH.

I interpreted that to mean that it was another environment variable. I now see that you you are saying that instead of hard-coding e.g. /usr/local/hdf5/lib/plugin, we should get the value from the HDF5 build.

I note the following:

  1. It looks like this was added in the 1.10.x version of HDF5.
  2. that path (and the windows equivalent) are hard-wired into the hdf5 documentation.
  3. If netcdf-c is built without HDF5 but with NCZarr, then we do not have access to that HDF5 value, so we should do the same thing for the NCZarr code, but of course it would need to be coordinated with the HDF5 value. Alternatively, we could add an option for this (like HDF5 does). This does not seem an unreasonable idea since it only matters if the builder was already doing it for HDF5.

DennisHeimbigner avatar Jun 26 '22 19:06 DennisHeimbigner