[libvirt] [PATCH v2 0/3] Various cleanups to the build system
Changes from [v1]: * patches 1-4 from v1 have been pushed; * patches 1 and 2 are new; * patch 3 (which was patch 5 in v1) no longer drops the various WITH_LIBVIRTD conditionals. [v1] https://www.redhat.com/archives/libvir-list/2019-January/msg00212.html Andrea Bolognani (3): src: Define initdir src: Only install SysV init scripts when libvirtd is built src: Simplify installing/uninstalling data src/Makefile.am | 66 ++++++++++++++++++-------------------- src/remote/Makefile.inc.am | 30 +++++++---------- 2 files changed, 44 insertions(+), 52 deletions(-) -- 2.20.1
Avoid building the same path several times. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/Makefile.am | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index d3e8a1b572..0a8f54120d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -811,16 +811,18 @@ uninstall-logrotate: endif ! WITH_LIBVIRTD if LIBVIRT_INIT_SCRIPT_RED_HAT +initdir = $(sysconfdir)/rc.d/init.d + install-init:: $(SYSVINIT_FILES) install-sysconfig - $(MKDIR_P) $(DESTDIR)$(sysconfdir)/rc.d/init.d + $(MKDIR_P) $(DESTDIR)$(initdir) for f in $(SYSVINIT_FILES:%.init=%) ; \ do \ - $(INSTALL_SCRIPT) $$f.init $(DESTDIR)$(sysconfdir)/rc.d/init.d/$$f; \ + $(INSTALL_SCRIPT) $$f.init $(DESTDIR)$(initdir)/$$f; \ done uninstall-init:: uninstall-sysconfig - rm -f $(SYSVINIT_FILES:%.init=$(DESTDIR)$(sysconfdir)/rc.d/init.d/%) - rmdir $(DESTDIR)$(sysconfdir)/rc.d/init.d || : + rm -f $(SYSVINIT_FILES:%.init=$(DESTDIR)$(initdir)/%) + rmdir $(DESTDIR)$(initdir) || : BUILT_SOURCES += $(SYSVINIT_FILES) DISTCLEANFILES += $(SYSVINIT_FILES) -- 2.20.1
This is consistent with the way we already handle configuration for other init systems such as upstart and systemd. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/Makefile.am | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Makefile.am b/src/Makefile.am index 0a8f54120d..82a96adb99 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -810,6 +810,7 @@ install-logrotate: uninstall-logrotate: endif ! WITH_LIBVIRTD +if WITH_LIBVIRTD if LIBVIRT_INIT_SCRIPT_RED_HAT initdir = $(sysconfdir)/rc.d/init.d @@ -830,6 +831,10 @@ else ! LIBVIRT_INIT_SCRIPT_RED_HAT install-init:: uninstall-init:: endif ! LIBVIRT_INIT_SCRIPT_RED_HAT +else ! WITH_LIBVIRTD +install-init:: +uninstall-init:: +endif ! WITH_LIBVIRTD %.8: %.8.in $(top_srcdir)/configure.ac -- 2.20.1
On Thu, Jan 10, 2019 at 03:11:45PM +0100, Andrea Bolognani wrote:
This is consistent with the way we already handle configuration for other init systems such as upstart and systemd.
Apart from consistency, I wonder whether we still need that extra condition or it's a historical artefact we can get rid of. Erik
On Thu, 2019-01-10 at 16:32 +0100, Erik Skultety wrote:
On Thu, Jan 10, 2019 at 03:11:45PM +0100, Andrea Bolognani wrote:
This is consistent with the way we already handle configuration for other init systems such as upstart and systemd.
Apart from consistency, I wonder whether we still need that extra condition or it's a historical artefact we can get rid of.
Without those conditions, one could conceivably run ./configure --with-libvirtd=no --with-init-script=systemd and end up with a bunch of .service files, pointing to binaries that don't exist, installed on the system. Whether or not we should even prepare for such a scenario is of course up for debate, but the conditions are not, strictly speaking, useless. -- Andrea Bolognani / Red Hat / Virtualization
Instead of defining targets conditionally and depending on them unconditionally, define a couple of variables and conditionally add targets to them. In addition to removing a bunch of useless code, this has the nice effect of no longer requiring the main Makefile.am to have any knowledge about the contents of the various snippets it includes. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/Makefile.am | 59 ++++++++++++++++---------------------- src/remote/Makefile.inc.am | 30 ++++++++----------- 2 files changed, 37 insertions(+), 52 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 82a96adb99..cd386297ed 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -72,6 +72,8 @@ STATEFUL_DRIVER_SOURCE_FILES = noinst_LTLIBRARIES = mod_LTLIBRARIES = INSTALL_DATA_DIRS = +INSTALL_DATA_LOCAL = +UNINSTALL_LOCAL = libvirt_la_BUILT_LIBADD = SYM_FILES = USED_SYM_FILES = @@ -805,10 +807,10 @@ install-logrotate: $(LOGROTATE_FILES) uninstall-logrotate: rm -f $(LOGROTATE_FILES:%.logrotate=$(DESTDIR)$(sysconfdir)/logrotate.d/%) rmdir $(DESTDIR)$(sysconfdir)/logrotate.d || : -else ! WITH_LIBVIRTD -install-logrotate: -uninstall-logrotate: -endif ! WITH_LIBVIRTD + +INSTALL_DATA_LOCAL += install-logrotate +UNINSTALL_LOCAL += uninstall-logrotate +endif WITH_LIBVIRTD if WITH_LIBVIRTD if LIBVIRT_INIT_SCRIPT_RED_HAT @@ -827,14 +829,11 @@ uninstall-init:: uninstall-sysconfig BUILT_SOURCES += $(SYSVINIT_FILES) DISTCLEANFILES += $(SYSVINIT_FILES) -else ! LIBVIRT_INIT_SCRIPT_RED_HAT -install-init:: -uninstall-init:: -endif ! LIBVIRT_INIT_SCRIPT_RED_HAT -else ! WITH_LIBVIRTD -install-init:: -uninstall-init:: -endif ! WITH_LIBVIRTD + +INSTALL_DATA_LOCAL += install-init +UNINSTALL_LOCAL += uninstall-init +endif LIBVIRT_INIT_SCRIPT_RED_HAT +endif WITH_LIBVIRTD %.8: %.8.in $(top_srcdir)/configure.ac @@ -873,14 +872,11 @@ install-systemd: $(SYSTEMD_UNIT_FILES) install-sysconfig uninstall-systemd: uninstall-sysconfig rm -f $(SYSTEMD_UNIT_FILES:%=$(DESTDIR)$(SYSTEMD_UNIT_DIR)/%) rmdir $(DESTDIR)$(SYSTEMD_UNIT_DIR) || : -else ! LIBVIRT_INIT_SCRIPT_SYSTEMD -install-systemd: -uninstall-systemd: -endif ! LIBVIRT_INIT_SCRIPT_SYSTEMD -else ! WITH_LIBVIRTD -install-systemd: -uninstall-systemd: -endif ! WITH_LIBVIRTD + +INSTALL_DATA_LOCAL += install-systemd +UNINSTALL_LOCAL += uninstall-systemd +endif LIBVIRT_INIT_SCRIPT_SYSTEMD +endif WITH_LIBVIRTD EXTRA_DIST += $(UPSTART_FILES) @@ -904,14 +900,11 @@ uninstall-upstart: uninstall-sysconfig rm -f $(DESTDIR)$(sysconfdir)/event.d/$$tgt ; \ done rmdir $(DESTDIR)$(sysconfdir)/event.d || : -else ! LIBVIRT_INIT_SCRIPT_UPSTART -install-upstart: -uninstall-upstart: -endif ! LIBVIRT_INIT_SCRIPT_UPSTART -else ! WITH_LIBVIRTD -install-upstart: -uninstall-upstart: -endif ! WITH_LIBVIRTD + +INSTALL_DATA_LOCAL += install-upstart +UNINSTALL_LOCAL += uninstall-upstart +endif LIBVIRT_INIT_SCRIPT_UPSTART +endif WITH_LIBVIRTD EXTRA_DIST += dtrace2systemtap.pl @@ -1010,17 +1003,15 @@ libvirt_nss_la_LIBADD = \ endif WITH_NSS -install-data-local: install-init install-systemd install-upstart \ - install-sysctl install-polkit install-sasl \ - install-logrotate $(INSTALL_DATA_DIRS:%=install-data-%) +install-data-local: $(INSTALL_DATA_LOCAL) \ + $(INSTALL_DATA_DIRS:%=install-data-%) $(MKDIR_P) "$(DESTDIR)$(localstatedir)/cache/libvirt" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/images" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/filesystems" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/boot" -uninstall-local:: uninstall-init uninstall-systemd uninstall-upstart \ - uninstall-sysctl uninstall-polkit uninstall-sasl \ - uninstall-logrotate $(INSTALL_DATA_DIRS:%=uninstall-data-%) +uninstall-local:: $(UNINSTALL_LOCAL) \ + $(INSTALL_DATA_DIRS:%=uninstall-data-%) rmdir "$(DESTDIR)$(localstatedir)/cache/libvirt" ||: rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/images" ||: rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/filesystems" ||: diff --git a/src/remote/Makefile.inc.am b/src/remote/Makefile.inc.am index 3355dc68e5..0988435eed 100644 --- a/src/remote/Makefile.inc.am +++ b/src/remote/Makefile.inc.am @@ -203,10 +203,10 @@ install-sysctl: uninstall-sysctl: rm -f $(DESTDIR)$(sysctldir)/60-libvirtd.conf rmdir $(DESTDIR)$(sysctldir) || : -else ! WITH_SYSCTL -install-sysctl: -uninstall-sysctl: -endif ! WITH_SYSCTL + +INSTALL_DATA_LOCAL += install-sysctl +UNINSTALL_LOCAL += uninstall-sysctl +endif WITH_SYSCTL if WITH_POLKIT polkitdir = $(datadir)/polkit-1 @@ -227,17 +227,11 @@ uninstall-polkit:: rm -f $(DESTDIR)$(polkitrulesdir)/50-libvirt.rules rmdir $(DESTDIR)$(polkitrulesdir) || : -else ! WITH_POLKIT -install-polkit:: -uninstall-polkit:: -endif ! WITH_POLKIT +INSTALL_DATA_LOCAL += install-polkit +UNINSTALL_LOCAL += uninstall-polkit +endif WITH_POLKIT -else ! WITH_LIBVIRTD -install-polkit:: -uninstall-polkit:: -install-sysctl:: -uninstall-sysctl:: -endif ! WITH_LIBVIRTD +endif WITH_LIBVIRTD .PHONY: \ install-data-remote \ @@ -258,10 +252,10 @@ install-sasl: uninstall-sasl: rm -f $(DESTDIR)$(sasldir)/libvirt.conf rmdir $(DESTDIR)$(sasldir) || : -else ! WITH_SASL -install-sasl: -uninstall-sasl: -endif ! WITH_SASL + +INSTALL_DATA_LOCAL += install-sasl +UNINSTALL_LOCAL += uninstall-sasl +endif WITH_SASL libvirtd.init: remote/libvirtd.init.in $(top_builddir)/config.status $(AM_V_GEN)sed \ -- 2.20.1
On Thu, Jan 10, 2019 at 03:11:43PM +0100, Andrea Bolognani wrote:
Changes from [v1]:
* patches 1-4 from v1 have been pushed;
* patches 1 and 2 are new;
* patch 3 (which was patch 5 in v1) no longer drops the various WITH_LIBVIRTD conditionals.
[v1] https://www.redhat.com/archives/libvir-list/2019-January/msg00212.html
Andrea Bolognani (3): src: Define initdir src: Only install SysV init scripts when libvirtd is built src: Simplify installing/uninstalling data
src/Makefile.am | 66 ++++++++++++++++++-------------------- src/remote/Makefile.inc.am | 30 +++++++---------- 2 files changed, 44 insertions(+), 52 deletions(-)
Reviewed-by: Erik Skultety <eskultet@redhat.com>
participants (2)
-
Andrea Bolognani -
Erik Skultety