[libvirt] [PATCH 0/5] wireshark: Fix $(prefix) handling

Well, almost :) There are still some cases that are not handled correctly, but at least this will unbreak 'make rpm' while I work on the rest. Tested by running 'make rpm' successfully on Fedora 23, Fedora 24 and Fedora rawhide. Andrea Bolognani (5): wireshark: Introduce $ws_modversion wireshark: Hoist $ws_prefix declaration wireshark: Strip prefix correctly wireshark: Inject $(prefix) at the right time wireshark: Rename plugindir to ws_plugindir m4/virt-wireshark.m4 | 29 ++++++++++++++++++----------- tools/Makefile.am | 2 +- 2 files changed, 19 insertions(+), 12 deletions(-) -- 2.7.4

Use a separate variable instead of setting it inline for slightly cleaner code. --- m4/virt-wireshark.m4 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4 index e1e4a59..eb6c8a6 100644 --- a/m4/virt-wireshark.m4 +++ b/m4/virt-wireshark.m4 @@ -29,10 +29,11 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[ if test "x$with_wireshark_dissector" != "xno" ; then if test "x$with_ws_plugindir" = "xcheck" ; then plugindir="$($PKG_CONFIG --variable plugindir wireshark)" + ws_modversion="$($PKG_CONFIG --modversion wireshark)" if test "x$plugindir" = "x" ; then dnl On some systems the plugindir variable may not be stored within pkg config. dnl Fall back to older style of constructing the plugin dir path. - plugindir="$libdir/wireshark/plugins/$($PKG_CONFIG --modversion wireshark)" + plugindir="$libdir/wireshark/plugins/$ws_modversion" else ws_prefix="$($PKG_CONFIG --variable prefix wireshark)" if test "x$ws_prefix" = "x" ; then -- 2.7.4

Keep all variable declarations close together. --- m4/virt-wireshark.m4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4 index eb6c8a6..64acca9 100644 --- a/m4/virt-wireshark.m4 +++ b/m4/virt-wireshark.m4 @@ -29,13 +29,13 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[ if test "x$with_wireshark_dissector" != "xno" ; then if test "x$with_ws_plugindir" = "xcheck" ; then plugindir="$($PKG_CONFIG --variable plugindir wireshark)" + ws_prefix="$($PKG_CONFIG --variable prefix wireshark)" ws_modversion="$($PKG_CONFIG --modversion wireshark)" if test "x$plugindir" = "x" ; then dnl On some systems the plugindir variable may not be stored within pkg config. dnl Fall back to older style of constructing the plugin dir path. plugindir="$libdir/wireshark/plugins/$ws_modversion" else - ws_prefix="$($PKG_CONFIG --variable prefix wireshark)" if test "x$ws_prefix" = "x" ; then ws_prefix="/usr"; fi -- 2.7.4

Even when we're building $plugindir ourselves because we can't retrieve it using pkg-config, we still want to strip the prefix, except in that case it would be the same prefix we're using for building libvirt. The fact that $plugindir is missing also doesn't tell us anything about $ws_prefix, so we have to handle the two variables separately. --- m4/virt-wireshark.m4 | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4 index 64acca9..90d731e 100644 --- a/m4/virt-wireshark.m4 +++ b/m4/virt-wireshark.m4 @@ -35,12 +35,14 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[ dnl On some systems the plugindir variable may not be stored within pkg config. dnl Fall back to older style of constructing the plugin dir path. plugindir="$libdir/wireshark/plugins/$ws_modversion" - else - if test "x$ws_prefix" = "x" ; then - ws_prefix="/usr"; - fi - plugindir="${plugindir#$ws_prefix}" + ws_prefix="$prefix" fi + if test "x$ws_prefix" = "x" ; then + dnl If the wireshark prefix cannot be retrieved from pkg-config, + dnl /usr is our best bet + ws_prefix="/usr" + fi + plugindir="${plugindir#$ws_prefix}" 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]) else -- 2.7.4

Adding $(prefix) in Makefile.am, as we were doing, means that it would be prepended even when using --with-ws-plugindir, which is something we don't want to happen. Instead, we add it beforehand but take care that it doesn't get expanded until make is called. --- m4/virt-wireshark.m4 | 6 +++++- tools/Makefile.am | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4 index 90d731e..c949f8a 100644 --- a/m4/virt-wireshark.m4 +++ b/m4/virt-wireshark.m4 @@ -42,7 +42,11 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[ dnl /usr is our best bet ws_prefix="/usr" fi - plugindir="${plugindir#$ws_prefix}" + dnl Replace the wireshark prefix with our own. + dnl Note that $(prefix) is kept verbatim at this point in time, and will + dnl only be expanded later, when make is called: this makes it possible + dnl to override the prefix at compilation or installation time + plugindir='$(prefix)'"${plugindir#$ws_prefix}" 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]) else diff --git a/tools/Makefile.am b/tools/Makefile.am index 08e1680..981be31 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -398,7 +398,7 @@ EXTRA_DIST += \ if WITH_WIRESHARK_DISSECTOR -ws_plugindir = $(prefix)$(plugindir) +ws_plugindir = @plugindir@ ws_plugin_LTLIBRARIES = wireshark/src/libvirt.la wireshark_src_libvirt_la_CPPFLAGS = \ -I wireshark/src $(WIRESHARK_DISSECTOR_CFLAGS) -- 2.7.4

Since we're using autoconf to substitute the right value in Makefile.am now, we can use a less generic name without running into circular dependencies. --- m4/virt-wireshark.m4 | 12 ++++++------ tools/Makefile.am | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4 index c949f8a..556272a 100644 --- a/m4/virt-wireshark.m4 +++ b/m4/virt-wireshark.m4 @@ -28,13 +28,13 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[ dnl Check for system location of wireshark plugins if test "x$with_wireshark_dissector" != "xno" ; then if test "x$with_ws_plugindir" = "xcheck" ; then - plugindir="$($PKG_CONFIG --variable plugindir wireshark)" + ws_plugindir="$($PKG_CONFIG --variable plugindir wireshark)" ws_prefix="$($PKG_CONFIG --variable prefix wireshark)" ws_modversion="$($PKG_CONFIG --modversion wireshark)" - if test "x$plugindir" = "x" ; then + if test "x$ws_plugindir" = "x" ; then dnl On some systems the plugindir variable may not be stored within pkg config. dnl Fall back to older style of constructing the plugin dir path. - plugindir="$libdir/wireshark/plugins/$ws_modversion" + ws_plugindir="$libdir/wireshark/plugins/$ws_modversion" ws_prefix="$prefix" fi if test "x$ws_prefix" = "x" ; then @@ -46,15 +46,15 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[ dnl Note that $(prefix) is kept verbatim at this point in time, and will dnl only be expanded later, when make is called: this makes it possible dnl to override the prefix at compilation or installation time - plugindir='$(prefix)'"${plugindir#$ws_prefix}" + ws_plugindir='$(prefix)'"${ws_plugindir#$ws_prefix}" 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]) else - plugindir=$with_ws_plugindir + ws_plugindir=$with_ws_plugindir fi fi - AC_SUBST([plugindir]) + AC_SUBST([ws_plugindir]) ]) AC_DEFUN([LIBVIRT_RESULT_WIRESHARK],[ diff --git a/tools/Makefile.am b/tools/Makefile.am index 981be31..319abb2 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -398,7 +398,7 @@ EXTRA_DIST += \ if WITH_WIRESHARK_DISSECTOR -ws_plugindir = @plugindir@ +ws_plugindir = @ws_plugindir@ ws_plugin_LTLIBRARIES = wireshark/src/libvirt.la wireshark_src_libvirt_la_CPPFLAGS = \ -I wireshark/src $(WIRESHARK_DISSECTOR_CFLAGS) -- 2.7.4

On 26.10.2016 15:27, Andrea Bolognani wrote:
Well, almost :) There are still some cases that are not handled correctly, but at least this will unbreak 'make rpm' while I work on the rest.
Tested by running 'make rpm' successfully on Fedora 23, Fedora 24 and Fedora rawhide.
Andrea Bolognani (5): wireshark: Introduce $ws_modversion wireshark: Hoist $ws_prefix declaration wireshark: Strip prefix correctly wireshark: Inject $(prefix) at the right time wireshark: Rename plugindir to ws_plugindir
m4/virt-wireshark.m4 | 29 ++++++++++++++++++----------- tools/Makefile.am | 2 +- 2 files changed, 19 insertions(+), 12 deletions(-)
ACK series. Thanks for cleaning up my mess. Michal

On Wed, 2016-10-26 at 16:54 +0200, Michal Privoznik wrote:
On 26.10.2016 15:27, Andrea Bolognani wrote:
Well, almost :) There are still some cases that are not handled correctly, but at least this will unbreak 'make rpm' while I work on the rest. Tested by running 'make rpm' successfully on Fedora 23, Fedora 24 and Fedora rawhide. Andrea Bolognani (5): wireshark: Introduce $ws_modversion wireshark: Hoist $ws_prefix declaration wireshark: Strip prefix correctly wireshark: Inject $(prefix) at the right time wireshark: Rename plugindir to ws_plugindir m4/virt-wireshark.m4 | 29 ++++++++++++++++++----------- tools/Makefile.am | 2 +- 2 files changed, 19 insertions(+), 12 deletions(-) ACK series. Thanks for cleaning up my mess.
Don't mention it, it was fun :) On the other hand, if you really want to thank me, I guess taking a look at the follow-up series https://www.redhat.com/archives/libvir-list/2016-October/msg01187.html would be as good a way as any :P -- Andrea Bolognani / Red Hat / Virtualization

On 26.10.2016 15:27, Andrea Bolognani wrote:
Well, almost :) There are still some cases that are not handled correctly, but at least this will unbreak 'make rpm' while I work on the rest.
Tested by running 'make rpm' successfully on Fedora 23, Fedora 24 and Fedora rawhide.
Andrea Bolognani (5): wireshark: Introduce $ws_modversion wireshark: Hoist $ws_prefix declaration wireshark: Strip prefix correctly wireshark: Inject $(prefix) at the right time wireshark: Rename plugindir to ws_plugindir
m4/virt-wireshark.m4 | 29 ++++++++++++++++++----------- tools/Makefile.am | 2 +- 2 files changed, 19 insertions(+), 12 deletions(-)
Thanks for the fixes. As it happens, I have tried to fix our local builds in parallel, and thus have some minor suggestions, patch attached. -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
participants (3)
-
Andrea Bolognani
-
Michal Privoznik
-
Viktor Mihajlovski