[libvirt] [PATCH 0/3] Yet some install fixes

After my patches fixing install & uninstall [1] I've found some other flaws. The last patch is essential as the problem I'm seeing there I'm seeing in libvirt-php too. I've came up with an ugly fix in libvirt-php. But then found a better way of dealing with it, so if this gets in, I'll mimic it there too. 1: https://www.redhat.com/archives/libvir-list/2016-April/msg01192.html Michal Privoznik (3): Makefile: Enable distuninstallcheck again tools: Introduce install-nss targets wireshark: Fix distcheck Makefile.am | 3 --- m4/virt-wireshark.m4 | 8 +++++++- tools/Makefile.am | 11 +++++++---- 3 files changed, 14 insertions(+), 8 deletions(-) -- 2.7.3

This target has been disabled historically for whatever reason. Now that we do uninstall properly enable the test again. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- Makefile.am | 3 --- 1 file changed, 3 deletions(-) diff --git a/Makefile.am b/Makefile.am index d6f09ba..c52a4cb 100644 --- a/Makefile.am +++ b/Makefile.am @@ -83,9 +83,6 @@ clean-cov: MAINTAINERCLEANFILES = .git-module-status -# disable this check -distuninstallcheck: - dist-hook: gen-ChangeLog gen-AUTHORS # Generate the ChangeLog file (with all entries since the switch to git) -- 2.7.3

On 04/25/2016 03:11 AM, Michal Privoznik wrote:
This target has been disabled historically for whatever reason. Now that we do uninstall properly enable the test again.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- Makefile.am | 3 --- 1 file changed, 3 deletions(-)
diff --git a/Makefile.am b/Makefile.am index d6f09ba..c52a4cb 100644 --- a/Makefile.am +++ b/Makefile.am @@ -83,9 +83,6 @@ clean-cov:
MAINTAINERCLEANFILES = .git-module-status
-# disable this check -distuninstallcheck: - dist-hook: gen-ChangeLog gen-AUTHORS
# Generate the ChangeLog file (with all entries since the switch to git)
ACK, I was wondering why distcheck wasn't catching those uninstall issues - Cole

We do have something similar for installing init system files. Basically I'm trying to avoid the following warning produced by automake: tools/Makefile.am:429: warning: uninstall-local was already defined in condition TRUE, which includes condition WITH_BSD_NSS ... tools/Makefile.am:292: ... 'uninstall-local' previously defined here Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/Makefile.am | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tools/Makefile.am b/tools/Makefile.am index c5a6a0d..20c6711 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -287,9 +287,9 @@ endif WITH_WIN_ICON && if grep 'POD ERROR' $(srcdir)/$@ ; then \ rm $(srcdir)/$@; exit 1; fi -install-data-local: install-init install-systemd +install-data-local: install-init install-systemd install-nss -uninstall-local: uninstall-init uninstall-systemd +uninstall-local: uninstall-init uninstall-systemd uninstall-nss install-sysconfig: $(MKDIR_P) $(DESTDIR)$(sysconfdir)/sysconfig @@ -422,17 +422,20 @@ LIBVIRT_NSS_SYMBOL_FILE = \ $(srcdir)/nss/libvirt_nss_bsd.syms NSS_SO_VER = 1 -install-exec-hook: +install-nss: ( cd $(DESTDIR)$(libdir) && \ rm -f nss_libvirt.so.$(NSS_SO_VER) && \ $(LN_S) libnss_libvirt.so.$(NSS_SO_VER) nss_libvirt.so.$(NSS_SO_VER) ) -uninstall-local: +uninstall-nss: -rm -f $(DESTDIR)$(libdir)/nss_libvirt.so.$(NSS_SO_VER) else ! WITH_BSD_NSS LIBVIRT_NSS_SYMBOL_FILE = \ $(srcdir)/nss/libvirt_nss.syms NSS_SO_VER = 2 + +install-nss: +uninstall-nss: endif ! WITH_BSD_NSS LIBVIRT_NSS_SOURCES = \ -- 2.7.3

On 04/25/2016 03:11 AM, Michal Privoznik wrote:
We do have something similar for installing init system files. Basically I'm trying to avoid the following warning produced by automake:
tools/Makefile.am:429: warning: uninstall-local was already defined in condition TRUE, which includes condition WITH_BSD_NSS ... tools/Makefile.am:292: ... 'uninstall-local' previously defined here
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/Makefile.am | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
ACK - Cole

Our distcheck is broken. Well, it works but only by pure chance. When wireshark plugin is enabled, we try to query which path should the plugin be installed into. Firstly, we try to ask pkg-config as some releases of wireshark already sets corresponding variable in their pkg-config files. However, if we obtained no value from there we try to construct the path on our own. Based on our observations it usually is: $libdir/wireshark/plugins/$version/. Now, the problem is in the way we are deciding whether we have obtained the plugin directory from pkg-config or not. Simply said, we are checking wrong variable. The variable we are checking has never been set, thus in our test is empty and therefore we will always construct the plugin dir path on our own, regardless of its presence in the pkg-config file. To make things worse, after fixing this problem, VPATH build was broken as it now tried to install plugin into correct directory. Yes, this is problem, because --prefix was not honoured and everything but the plugin was installed into given prefix. I've managed to resolve this issue by replacing plugin dir prefix with our own. So when doing regular installation (our prefix == wireshark prefix), nothing changes. When doing VPATH build & installation plugin is installed into correctly prefixed dir. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- m4/virt-wireshark.m4 | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4 index ac2e44c..d8cb7c8 100644 --- a/m4/virt-wireshark.m4 +++ b/m4/virt-wireshark.m4 @@ -29,10 +29,16 @@ 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)" - if test "x$ws_plugindir" = "x" ; then + 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)" + else + ws_prefix="$($PKG_CONFIG --variable prefix wireshark)" + if test "x$ws_prefix" = "x" ; then + ws_prefix="/usr"; + fi + plugindir="${prefix}${plugindir#ws_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.3

On 04/25/2016 03:11 AM, Michal Privoznik wrote:
Our distcheck is broken. Well, it works but only by pure chance. When wireshark plugin is enabled, we try to query which path should the plugin be installed into. Firstly, we try to ask pkg-config as some releases of wireshark already sets corresponding variable in their pkg-config files. However, if we obtained no value from there we try to construct the path on our own. Based on our observations it usually is: $libdir/wireshark/plugins/$version/. Now, the problem is in the way we are deciding whether we have obtained the plugin directory from pkg-config or not. Simply said, we are checking wrong variable. The variable we are checking has never been set, thus in our test is empty and therefore we will always construct the plugin dir path on our own, regardless of its presence in the pkg-config file. To make things worse, after fixing this problem, VPATH build was broken as it now tried to install plugin into correct directory. Yes, this is problem, because --prefix was not honoured and everything but the plugin was installed into given prefix. I've managed to resolve this issue by replacing plugin dir prefix with our own. So when doing regular installation (our prefix == wireshark prefix), nothing changes. When doing VPATH build & installation plugin is installed into correctly prefixed dir.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- m4/virt-wireshark.m4 | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4 index ac2e44c..d8cb7c8 100644 --- a/m4/virt-wireshark.m4 +++ b/m4/virt-wireshark.m4 @@ -29,10 +29,16 @@ 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)" - if test "x$ws_plugindir" = "x" ; then + 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)" + else + ws_prefix="$($PKG_CONFIG --variable prefix wireshark)" + if test "x$ws_prefix" = "x" ; then + ws_prefix="/usr"; + fi + plugindir="${prefix}${plugindir#ws_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])
Sounds reasonable to me, ACK - Cole
participants (2)
-
Cole Robinson
-
Michal Privoznik