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.

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

Further simplify the plugin installation process. #2440

Closed
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions nc_test4/findplugin.in
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,8 @@ findpluginext() {
TDY=`find ${TOPBUILDDIR}/plugins -name '*misc.dylib'`
TCYG=`find ${TOPBUILDDIR}/plugins -name 'cyg*misc.dll'`
TDLL=`find ${TOPBUILDDIR}/plugins -name '*misc.dll'`
if test "x$TSO" != x ; then
FP_PLUGIN_EXT="so"
FP_PLUGIN_PRE="lib__nc"
elif test "x$TDY" != x ; then
# Test order may be important esp for dylib vs so
if test "x$TDY" != x ; then
FP_PLUGIN_EXT="dylib"
FP_PLUGIN_PRE="lib__nc"
elif test "x$TCYG" != x ; then
Expand All @@ -49,6 +47,9 @@ findpluginext() {
elif test "x$TDLL" != x ; then
FP_PLUGIN_EXT="dll"
FP_PLUGIN_PRE="__nc"
elif test "x$TSO" != x ; then
FP_PLUGIN_EXT="so"
FP_PLUGIN_PRE="lib__nc"
else # unknown
unset FP_PLUGIN_EXT
unset FP_PLUGIN_PRE
Expand Down
2 changes: 0 additions & 2 deletions nc_test4/tst_unknown.sh
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ UNKNOWNDIR="${HDF5_PLUGIN_DIR}"
UNKNOWNLIB="${HDF5_PLUGIN_LIB}"
UNKNOWNFILTER="${HDF5_PLUGIN_DIR}/${UNKNOWNLIB}"

# Getting the name is especially tricky for dylib, which puts the version before the .dylib

# Verify
if ! test -f ${UNKNOWNFILTER} ; then echo "Unable to locate ${UNKNOWNFILTER}"; exit 1; fi

Expand Down
8 changes: 4 additions & 4 deletions nc_test4/tst_vars3.c
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ main(int argc, char **argv)
if (nc_close(ncid)) ERR;
}
SUMMARIZE_ERR;
#ifdef HAVE_SZ
#ifdef HAVE_H5Z_SZIP
printf("**** testing simple szip filter setup...");
{
int ncid;
Expand Down Expand Up @@ -748,9 +748,9 @@ main(int argc, char **argv)
params[1] = NC_SZIP_EC_BPP_IN; /* pixels_per_block */
if (nc_def_var_chunking(ncid, varid, NC_CHUNKED, NULL)) ERR;
{ int stat;
if ((stat = nc_def_var_filter(ncid, varid, H5_FILTER_SZIP, NUM_PARAMS_IN,
params)) != NC_ENOFILTER)
ERR;
stat = nc_def_var_filter(ncid, varid, H5_FILTER_SZIP, NUM_PARAMS_IN, params);
if(stat != NC_ENOFILTER)
ERR;
}
if (nc_def_var_szip(ncid, varid, NC_SZIP_NN,
NC_SZIP_EC_BPP_IN) != NC_ENOFILTER) ERR;
Expand Down
76 changes: 35 additions & 41 deletions plugins/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,28 @@
# Put Together AM_CPPFLAGS and AM_LDFLAGS
include $(top_srcdir)/lib_flags.am

AM_LDFLAGS += -module -avoid-version -shared -export-dynamic \
-rpath ${abs_builddir} ${NOUNDEFINED}

# Create an alternate directory if not installing or for noinst installs.
ALTPLUGINDIR = ${abs_top_builddir}/plugins/plugindir

# This is where the plugins are to be installed
if ENABLE_PLUGIN_DIR
#if ENABLE_PLUGIN_DIR
plugindir = @PLUGIN_INSTALL_DIR@
else
plugindir = ${ALTPLUGINDIR}
endif
#else
plugindir = ${prefix}
Copy link
Contributor

@DWesl DWesl Jun 30, 2022

Choose a reason for hiding this comment

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

I haven't seen anything tying --enable-plugins to --with-plugin-dir; I think just specifying --enable-plugins without --with-plugin-dir will put the (non-test) plugins into ${prefix} (test run with the old ALTPLUGINDIR value here). Would ${libexecdir}/netcdf-c or similar make more sense for the fallback location, would a better fallback be not installing those plugins at all (as implied by the old value), or is there some reasoning for this choice I'm not aware of?

Copy link
Contributor

Choose a reason for hiding this comment

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

New version is basically what I ended up with.

#endif

plugin_LTLIBRARIES =
AM_LDFLAGS += -module -avoid-version -shared -export-dynamic ${NOUNDEFINED}

# There seems no guaranteed way to turn of the version in the name
#plugin_version_info = -no-version-info 0:0:0

# Apparently one cannot have plugin_LTLIBRARIES and also noinst_LTLIBRARIES.
# So create a tmp location for "noinst" shared libraries.
tmpdir = ${ALTPLUGINDIR}
# Use this only with installable libraries
# Not currently needed for the platforms we support.
#INSTALLFLAGS = -rpath ${plugindir} ${plugin_version_info}

tmp_LTLIBRARIES =
# Use this only with test libraries
# Note: it appears necessary to set rpath for the check libraries
# so that they get generated
TESTFLAGS = -rpath ${plugindir}

# This linker flag specifies libtool version info.
# See http://www.gnu.org/software/libtool/manual/libtool.html#Libtool-versioning
# for information regarding incrementing `-version-info`.
plugin_version_info = -version-info 0:0:0
plugin_LTLIBRARIES =
check_LTLIBRARIES =

if ISMINGW
LDADD = ${top_builddir}/liblib/libnetcdf.la
Expand All @@ -52,18 +49,19 @@ if ENABLE_FILTER_TESTING
if ENABLE_NCZARR_FILTERS
plugin_LTLIBRARIES += lib__nch5fletcher32.la lib__nch5shuffle.la lib__nch5deflate.la
lib__nch5shuffle_la_SOURCES = H5Zshuffle.c
lib__nch5shuffle_la_LDFLAGS = ${INSTALLFLAGS}
lib__nch5fletcher32_la_SOURCES = H5Zfletcher32.c H5checksum.c
lib__nch5fletcher32_la_LDFLAGS = ${INSTALLFLAGS}
lib__nch5deflate_la_SOURCES = H5Zdeflate.c
lib__nch5deflate_la_LDFLAGS = ${INSTALLFLAGS}

lib__nch5shuffle_la_LDFLAGS = ${plugin_version_info}
lib__nch5deflate_la_LDFLAGS = ${plugin_version_info}
lib__nch5fletcher32_la_LDFLAGS = ${plugin_version_info}

# Need our version of szip if libsz available and we are not using HDF5
# Need our version of szip if libsz available and we are using nczarr
if HAVE_SZ
if ENABLE_NCZARR_FILTERS
plugin_LTLIBRARIES += lib__nch5szip.la
lib__nch5szip_la_SOURCES = H5Zszip.c H5Zszip.h
lib__nch5szip_la_LDFLAGS = ${plugin_version_info}
lib__nch5szip_la_LDFLAGS = ${INSTALLFLAGS}
endif
endif

endif # ENABLE_NCZARR_FILTERS
Expand All @@ -72,20 +70,21 @@ if ENABLE_PLUGINS

# The NCZarr codec libraries
lib__nczstdfilters_la_SOURCES = NCZstdfilters.c
lib__nczstdfilters_la_LDFLAGS = ${INSTALLFLAGS}
lib__nczhdf5filters_la_SOURCES = NCZhdf5filters.c

lib__nczhdf5filters_la_LDFLAGS = ${INSTALLFLAGS}
Copy link
Contributor

Choose a reason for hiding this comment

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

You seem to have commented out the INSTALLFLAGS assignment on line 20; should the _LDFLAGS lines with just that be deleted (allowing the default AM_LDFLAGS to serve those plugins, or the INSTALLFLAGS assignment uncommented?

Copy link
Contributor

Choose a reason for hiding this comment

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

Uncommented the INSTALLFLAGS definition and started it with AM_LDFLAGS, that'll help Cygwin builds (and probably Windows).

Doesn't look like there's a "resolve" button like I've seen on some of these.

plugin_LTLIBRARIES += lib__nczhdf5filters.la
plugin_LTLIBRARIES += lib__nczstdfilters.la

if HAVE_BLOSC
lib__nch5blosc_la_SOURCES = H5Zblosc.c H5Zblosc.h
lib__nch5blosc_la_LDFLAGS = ${plugin_version_info}
lib__nch5blosc_la_LDFLAGS = ${INSTALLFLAGS}
plugin_LTLIBRARIES += lib__nch5blosc.la
endif

if HAVE_ZSTD
lib__nch5zstd_la_SOURCES = H5Zzstd.c H5Zzstd.h
lib__nch5zstd_la_LDFLAGS = ${plugin_version_info}
lib__nch5zstd_la_LDFLAGS = ${INSTALLFLAGS}
plugin_LTLIBRARIES += lib__nch5zstd.la
endif

Expand All @@ -95,20 +94,20 @@ endif #ENABLE_PLUGINS
# Need two distinct instances
lib__nch5noop_la_SOURCES = H5Znoop.c H5Zutil.c h5noop.h
lib__nch5noop1_la_SOURCES = H5Znoop1.c H5Zutil.c h5noop.h
lib__nch5noop_la_LDFLAGS = ${plugin_version_info}
lib__nch5noop1_la_LDFLAGS = ${plugin_version_info}
lib__nch5noop_la_LDFLAGS = ${TESTFLAGS}
lib__nch5noop1_la_LDFLAGS = ${TESTFLAGS}

# The misc filter is to allow testing of filter arguments
lib__nch5misc_la_SOURCES = H5Zmisc.c H5Zutil.c h5misc.h
lib__nch5misc_la_LDFLAGS = ${plugin_version_info}
lib__nch5misc_la_LDFLAGS = ${TESTFLAGS}
lib__nczmisc_la_SOURCES = NCZmisc.c
lib__nczmisc_la_LDFLAGS = ${plugin_version_info}
lib__nczmisc_la_LDFLAGS = ${TESTFLAGS}

# Provide a filter to test missing filter
lib__nch5unknown_la_SOURCES = H5Zunknown.c
lib__nch5unknown_la_LDFLAGS = ${plugin_version_info}
lib__nch5unknown_la_LDFLAGS = ${TESTFLAGS}

tmp_LTLIBRARIES += lib__nch5noop.la lib__nch5noop1.la lib__nch5misc.la lib__nczmisc.la lib__nch5unknown.la
check_LTLIBRARIES += lib__nch5noop.la lib__nch5noop1.la lib__nch5misc.la lib__nczmisc.la lib__nch5unknown.la

# Bzip2 is used to test more complex filters
lib__nch5bzip2_la_SOURCES = H5Zbzip2.c h5bzip2.h
Expand All @@ -117,7 +116,7 @@ EXTRA_DIST += ${BZIP2SRC} BZIP2_LICENSE
if HAVE_LOCAL_BZ2
lib__nch5bzip2_la_SOURCES += ${BZIP2SRC}
endif
lib__nch5bzip2_la_LDFLAGS = ${plugin_version_info}
lib__nch5bzip2_la_LDFLAGS = ${INSTALLFLAGS}
plugin_LTLIBRARIES += lib__nch5bzip2.la

endif #ENABLE_FILTER_TESTING
Expand All @@ -138,8 +137,3 @@ bzip2::
tar -zxf ${BZIP2DIR}.tar.gz
cd ${BZIP2DIR}; cp ${BZIP2SRC} ..; cp LICENSE ../BZIP2_LICENSE ; cd ..
rm -fr ./${BZIP2DIR}

# Custom clean
clean-local:
rm -fr ${ALTPLUGINDIR}

7 changes: 7 additions & 0 deletions test_common.in
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@

# See netcdf-c/COPYRIGHT file for more info.

# Call only once per shell script
if test "x$TEST_COMMON_INITIALIZE" != xyes ; then
TEST_COMMON_INITIALIZE=yes

# Define location of execution
TOPSRCDIR='@abs_top_srcdir@'
TOPBUILDDIR='@abs_top_builddir@'
Expand Down Expand Up @@ -172,3 +176,6 @@ if test yes = `${execdir}/../ncdump/ncfilteravail $1` ; then return 0 ; else ech

# Make sure we are in builddir (not execdir)
cd $builddir

fi # TTEST_COMMON_INITIALIZE