[libvirt] [PATCH v2 0/3] wireshark: Further build system fixes

Changes from v1: * don't attempt to strip the wireshark prefix when we're building the plugindir ourselves * use exec_prefix instead of prefix everywhere (suggested by Viktor Mihajlovski) Andrea Bolognani (3): wireshark: Don't redefine ws_plugindir wireshark: Make fallback path construction more reliable wireshark: Use ${exec_prefix} instead of ${prefix} m4/virt-wireshark.m4 | 25 +++++++++++++------------ tools/Makefile.am | 1 - 2 files changed, 13 insertions(+), 13 deletions(-) -- 2.7.4

autoconf already defines the variable for us, and prints out a warning if we try to do it a second time. So let's not :) --- tools/Makefile.am | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/Makefile.am b/tools/Makefile.am index 319abb2..100e657 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -398,7 +398,6 @@ EXTRA_DIST += \ if WITH_WIRESHARK_DISSECTOR -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

We only need to strip $ws_prefix from $ws_plugindir if we've retrieved it from pkg-config: if we're building it ourselves from $libdir, we can just use it without further processing. --- m4/virt-wireshark.m4 | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4 index 556272a..f253c86 100644 --- a/m4/virt-wireshark.m4 +++ b/m4/virt-wireshark.m4 @@ -35,18 +35,18 @@ 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. ws_plugindir="$libdir/wireshark/plugins/$ws_modversion" - ws_prefix="$prefix" + else + 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 + 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 + ws_plugindir='$(prefix)'"${ws_plugindir#$ws_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 - 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 - 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 -- 2.7.4

${exec_prefix} and ${prefix} point to the same directory in most setups, but when that's not the case the former should be used for architecture-dependent data such as shared objects, which makes it the best fit for our Wireshark dissector. While at it, change all uses of $(var) to ${var}: they are absolutely identicaly as far as make's concerned, but autoconf itself seems to prefer the latter form so we might as well follow suit. --- m4/virt-wireshark.m4 | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4 index f253c86..d5d7404 100644 --- a/m4/virt-wireshark.m4 +++ b/m4/virt-wireshark.m4 @@ -29,23 +29,24 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[ if test "x$with_wireshark_dissector" != "xno" ; then if test "x$with_ws_plugindir" = "xcheck" ; then ws_plugindir="$($PKG_CONFIG --variable plugindir wireshark)" - ws_prefix="$($PKG_CONFIG --variable prefix wireshark)" + ws_exec_prefix="$($PKG_CONFIG --variable exec_prefix wireshark)" ws_modversion="$($PKG_CONFIG --modversion wireshark)" 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. ws_plugindir="$libdir/wireshark/plugins/$ws_modversion" else - 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" + if test "x$ws_exec_prefix" = "x" ; then + dnl If wireshark's exec_prefix cannot be retrieved from pkg-config, + dnl this is our best bet + ws_exec_prefix="/usr" fi - 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 - ws_plugindir='$(prefix)'"${ws_plugindir#$ws_prefix}" + dnl Replace wireshark's exec_prefix with our own. + dnl Note that ${exec_prefix} is kept verbatim at this point in time, + dnl and will only be expanded later, when make is called: this makes + dnl it possible to override such prefix at compilation or installation + dnl time + ws_plugindir='${exec_prefix}'"${ws_plugindir#$ws_exec_prefix}" 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]) -- 2.7.4

On 27.10.2016 22:29, Andrea Bolognani wrote:
Changes from v1:
* don't attempt to strip the wireshark prefix when we're building the plugindir ourselves
* use exec_prefix instead of prefix everywhere (suggested by Viktor Mihajlovski)
Andrea Bolognani (3): wireshark: Don't redefine ws_plugindir wireshark: Make fallback path construction more reliable wireshark: Use ${exec_prefix} instead of ${prefix}
m4/virt-wireshark.m4 | 25 +++++++++++++------------ tools/Makefile.am | 1 - 2 files changed, 13 insertions(+), 13 deletions(-)
ACK and safe for the release. Michal

On Mon, 2016-10-31 at 05:31 +0800, Michal Privoznik wrote:
Changes from v1: * don't attempt to strip the wireshark prefix when we're building the plugindir ourselves * use exec_prefix instead of prefix everywhere (suggested by Viktor Mihajlovski) Andrea Bolognani (3): wireshark: Don't redefine ws_plugindir wireshark: Make fallback path construction more reliable wireshark: Use ${exec_prefix} instead of ${prefix} m4/virt-wireshark.m4 | 25 +++++++++++++------------ tools/Makefile.am | 1 - 2 files changed, 13 insertions(+), 13 deletions(-) ACK and safe for the release.
Way too late to make it into 2.4.0, but it's pushed now. Thanks for the review :) -- Andrea Bolognani / Red Hat / Virtualization
participants (2)
-
Andrea Bolognani
-
Michal Privoznik