Re: [libvirt] [PATCH] virt-wireshark.m4: Defer $(prefix) substitution

On Thu, 2016-10-20 at 15:54 +0800, Michal Privoznik wrote: > The problem with evaluating $(prefix) in configure phase is that > autoconf does a lot of magic with this variable. Firstly, if the > --prefix argument is not set for the configure script, the > variable will have value "NONE". Wait, wat? Secondly, even > autoconf scripts then have special cases for when variable is > "NONE". It's even worse - whenever they need to construct new > path based on prefix, they save its current value, substitute it > with proper value (like /usr/local/), do what they need and then > restore the original value. Le sigh. > This mad behaviour is, however, suppressed if there's --prefix on > the configure command line. > Now, the problem is that we tried to construct path for wireshark > plugin in the m4 file. Based on the knowledge described above, we > shouldn't do it and just construct the full path in Makefile > later where prefix has proper value. I don't think we need to go into so much detail, we can just point to the autoconf manual[1] which tells us *not* to use these variables outside of Makefiles so that users still have a chance to override $(prefix) at 'make' or 'make install' time. > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > m4/virt-wireshark.m4 | 2 +- > tools/Makefile.am | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4 > index f383e2b..e1e4a59 100644 > --- a/m4/virt-wireshark.m4 > +++ b/m4/virt-wireshark.m4 > @@ -38,7 +38,7 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[ > if test "x$ws_prefix" = "x" ; then > ws_prefix="/usr"; > fi > - plugindir="${prefix}${plugindir#$ws_prefix}" > + plugindir="${plugindir#$ws_prefix}" This got me wondering: is what we're doing even correct? If the plugindir for wireshark is eg. /usr/lib64/wireshark/plugins, should we still install under an arbitrary $(prefix)? Will wireshark pick up the plugin then?

> fi > elif test "x$with_ws_plugindir" = "xno" || test "x$with_ws_plugindir" = "xyes"; then > AC_MSG_ERROR([ws-plugindir must be used only with valid path]) > diff --git a/tools/Makefile.am b/tools/Makefile.am > index e7e42c3..08e1680 100644 > --- a/tools/Makefile.am > +++ b/tools/Makefile.am > @@ -398,7 +398,7 @@ EXTRA_DIST += \ > > if WITH_WIRESHARK_DISSECTOR > > -ws_plugindir = $(plugindir) > +ws_plugindir = $(prefix)$(plugindir) > ws_plugin_LTLIBRARIES = wireshark/src/libvirt.la > wireshark_src_libvirt_la_CPPFLAGS = \ > -I wireshark/src $(WIRESHARK_DISSECTOR_CFLAGS) ACK