Further simplify the plugin installation process.#2440
Further simplify the plugin installation process.#2440DennisHeimbigner wants to merge 6 commits intoUnidata:mainfrom
Conversation
re: Unidata#2431 Hat tip to [DWesl](https://github.com/DWesl) and [opoplawski](https://github.com/opoplawski) for their help. With some help, I found out how to get rid of the tmp_LTLIBARIES hack and replace it with check_LTLIBRARIES. Using check_LTLIBRARIES means also that the test libraries are not built until "make check" is executed. This PR replaces the one above.
DWesl
left a comment
DWesl
left a comment
There was a problem hiding this comment.
Would it be worth adding a step checking make DESTDIR=/tmp/pretend-root install doesn't install any of the test plugins to one of the CI jobs?
if ! find /tmp/pretend-root -name lib__nctestplugin.la; exit 1
might work as the condition.
| plugindir = ${ALTPLUGINDIR} | ||
| endif | ||
| #else | ||
| plugindir = ${prefix} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
New version is basically what I ended up with.
| lib__nczstdfilters_la_LDFLAGS = ${INSTALLFLAGS} | ||
| lib__nczhdf5filters_la_SOURCES = NCZhdf5filters.c | ||
|
|
||
| lib__nczhdf5filters_la_LDFLAGS = ${INSTALLFLAGS} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
* If --disable-plugin is set, then force --without-plugin-dir * Propagate changes to CMakeLists.txt * Fix a couple of memory leaks in nc4internal.c and dhttp.c I was unable to figure out a way to prevent any attempt to install the shared libraries when --without-plugin-dir is used. It turns out you can conditionally set the install library, but it seems to ignore the conditional so in that case plugindir needs to be set to some internal location in the build tree. In any case, you still need a location for use with -rpath in order to force creation of the .so files.
|
This seems to work for me. Thanks. |
|
@DennisHeimbigner some conflicts have crept in, and Github is (again) not letting me resolve them. I'll try to sort that out, but are you able to make changes and/or have a moment to address them? |
|
Ok, let me see what I can do. |
|
This PR was apparently made moot by other, later PRs. |
4 participants
re: #2431
Hat tip to DWesl and opoplawski for their help.
With some help, I found out how to get rid of the tmp_LTLIBARIES
hack and replace it with check_LTLIBRARIES.
Using check_LTLIBRARIES means also that the test libraries are not
built until "make check" is executed.
This PR replaces the one above.