diff mbox series

[XEN,v3,07/25] tools/hotplug: cleanup Makefiles

Message ID 20220624160422.53457-8-anthony.perard@citrix.com (mailing list archive)
State Superseded
Headers show
Series Toolstack build system improvement, toward non-recursive makefiles | expand

Commit Message

Anthony PERARD June 24, 2022, 4:04 p.m. UTC 
Remove "build" targets.

Use simply expanded variables when recursively expanded variable
aren't needed. (Use ":=" instead of "=".)

Don't check if a directory already exist when installing, just create
it.

Fix $(HOTPLUGPATH), it shouldn't have any double-quote.

Some reindentation.

FreeBSD, "hotplugpath.sh" is already installed by common/.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/hotplug/FreeBSD/Makefile       | 11 +++--------
 tools/hotplug/Linux/Makefile         | 16 ++++++----------
 tools/hotplug/Linux/systemd/Makefile | 16 +++++++---------
 tools/hotplug/NetBSD/Makefile        |  9 +++------
 tools/hotplug/common/Makefile        | 16 ++++++----------
 5 files changed, 25 insertions(+), 43 deletions(-)

Comments

Luca Fancellu July 22, 2022, 11:02 a.m. UTC | #1 
> On 24 Jun 2022, at 17:04, Anthony PERARD <anthony.perard@citrix.com> wrote:
> 
> Remove "build" targets.
> 
> Use simply expanded variables when recursively expanded variable
> aren't needed. (Use ":=" instead of "=".)
> 
> Don't check if a directory already exist when installing, just create
> it.
> 
> Fix $(HOTPLUGPATH), it shouldn't have any double-quote.
> 
> Some reindentation.
> 
> FreeBSD, "hotplugpath.sh" is already installed by common/.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Hi Antony,

> ---
> tools/hotplug/FreeBSD/Makefile       | 11 +++--------
> tools/hotplug/Linux/Makefile         | 16 ++++++----------
> tools/hotplug/Linux/systemd/Makefile | 16 +++++++---------
> tools/hotplug/NetBSD/Makefile        |  9 +++------
> tools/hotplug/common/Makefile        | 16 ++++++----------
> 5 files changed, 25 insertions(+), 43 deletions(-)
> 
> diff --git a/tools/hotplug/FreeBSD/Makefile b/tools/hotplug/FreeBSD/Makefile
> index de9928cd86..a6552c9884 100644
> --- a/tools/hotplug/FreeBSD/Makefile
> +++ b/tools/hotplug/FreeBSD/Makefile
> @@ -2,18 +2,15 @@ XEN_ROOT = $(CURDIR)/../../..
> include $(XEN_ROOT)/tools/Rules.mk
> 
> # Xen script dir and scripts to go there.
> -XEN_SCRIPTS = vif-bridge block
> +XEN_SCRIPTS := vif-bridge block
> 
> -XEN_SCRIPT_DATA =
> +XEN_SCRIPT_DATA :=

Here (and also in NetBSD) the variable is empty, why are we keeping them
and the related code? Is it to keep a certain structure?

However the changes works and looks good to me, so your choice if it needs
to be addressed or not

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
diff mbox series

Patch

diff --git a/tools/hotplug/FreeBSD/Makefile b/tools/hotplug/FreeBSD/Makefile
index de9928cd86..a6552c9884 100644
--- a/tools/hotplug/FreeBSD/Makefile
+++ b/tools/hotplug/FreeBSD/Makefile
@@ -2,18 +2,15 @@  XEN_ROOT = $(CURDIR)/../../..
 include $(XEN_ROOT)/tools/Rules.mk
 
 # Xen script dir and scripts to go there.
-XEN_SCRIPTS = vif-bridge block
+XEN_SCRIPTS := vif-bridge block
 
-XEN_SCRIPT_DATA =
+XEN_SCRIPT_DATA :=
 
-XEN_RCD_PROG = rc.d/xencommons rc.d/xendriverdomain
+XEN_RCD_PROG := rc.d/xencommons rc.d/xendriverdomain
 
 .PHONY: all
 all:
 
-.PHONY: build
-build:
-
 .PHONY: install
 install: install-scripts install-rcd
 
@@ -44,12 +41,10 @@  install-rcd:
 	   do \
 	   $(INSTALL_PROG) $$i $(DESTDIR)$(INITD_DIR); \
 	done
-	$(INSTALL_DATA) ../common/hotplugpath.sh $(DESTDIR)$(XEN_SCRIPT_DIR)
 
 .PHONY: uninstall-rcd
 uninstall-rcd:
 	rm -f $(addprefix $(DESTDIR)$(INITD_DIR)/, $(XEN_RCD_PROG))
-	rm -f $(DESTDIR)$(XEN_SCRIPT_DIR)/hotplugpath.sh
 
 .PHONY: clean
 clean:
diff --git a/tools/hotplug/Linux/Makefile b/tools/hotplug/Linux/Makefile
index 0b1d111d7e..9a7b3a3515 100644
--- a/tools/hotplug/Linux/Makefile
+++ b/tools/hotplug/Linux/Makefile
@@ -2,7 +2,7 @@  XEN_ROOT = $(CURDIR)/../../..
 include $(XEN_ROOT)/tools/Rules.mk
 
 # Xen script dir and scripts to go there.
-XEN_SCRIPTS = vif-bridge
+XEN_SCRIPTS := vif-bridge
 XEN_SCRIPTS += vif-route
 XEN_SCRIPTS += vif-nat
 XEN_SCRIPTS += vif-openvswitch
@@ -22,16 +22,13 @@  XEN_SCRIPTS += launch-xenstore
 
 SUBDIRS-$(CONFIG_SYSTEMD) += systemd
 
-XEN_SCRIPT_DATA = xen-script-common.sh locking.sh logging.sh
+XEN_SCRIPT_DATA := xen-script-common.sh locking.sh logging.sh
 XEN_SCRIPT_DATA += xen-hotplug-common.sh xen-network-common.sh vif-common.sh
 XEN_SCRIPT_DATA += block-common.sh
 
 .PHONY: all
 all: subdirs-all
 
-.PHONY: build
-build:
-
 .PHONY: install
 install: install-initd install-scripts subdirs-install
 
@@ -41,9 +38,9 @@  uninstall: uninstall-initd uninstall-scripts subdirs-uninstall
 # See docs/misc/distro_mapping.txt for INITD_DIR location
 .PHONY: install-initd
 install-initd:
-	[ -d $(DESTDIR)$(INITD_DIR) ] || $(INSTALL_DIR) $(DESTDIR)$(INITD_DIR)
-	[ -d $(DESTDIR)$(SYSCONFIG_DIR) ] || $(INSTALL_DIR) $(DESTDIR)$(SYSCONFIG_DIR)
-	[ -d $(DESTDIR)$(LIBEXEC_BIN) ] || $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN)
+	$(INSTALL_DIR) $(DESTDIR)$(INITD_DIR)
+	$(INSTALL_DIR) $(DESTDIR)$(SYSCONFIG_DIR)
+	$(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN)
 	$(INSTALL_DATA) init.d/sysconfig.xendomains $(DESTDIR)$(SYSCONFIG_DIR)/xendomains
 	$(INSTALL_DATA) init.d/sysconfig.xencommons $(DESTDIR)$(SYSCONFIG_DIR)/xencommons
 	$(INSTALL_PROG) xendomains $(DESTDIR)$(LIBEXEC_BIN)
@@ -64,8 +61,7 @@  uninstall-initd:
 
 .PHONY: install-scripts
 install-scripts:
-	[ -d $(DESTDIR)$(XEN_SCRIPT_DIR) ] || \
-		$(INSTALL_DIR) $(DESTDIR)$(XEN_SCRIPT_DIR)
+	$(INSTALL_DIR) $(DESTDIR)$(XEN_SCRIPT_DIR)
 	set -e; for i in $(XEN_SCRIPTS); \
 	    do \
 	    $(INSTALL_PROG) $$i $(DESTDIR)$(XEN_SCRIPT_DIR); \
diff --git a/tools/hotplug/Linux/systemd/Makefile b/tools/hotplug/Linux/systemd/Makefile
index a5d41d86ef..26df2a43b1 100644
--- a/tools/hotplug/Linux/systemd/Makefile
+++ b/tools/hotplug/Linux/systemd/Makefile
@@ -1,12 +1,12 @@ 
 XEN_ROOT = $(CURDIR)/../../../..
 include $(XEN_ROOT)/tools/Rules.mk
 
-XEN_SYSTEMD_MODULES = xen.conf
+XEN_SYSTEMD_MODULES := xen.conf
 
-XEN_SYSTEMD_MOUNT =  proc-xen.mount
+XEN_SYSTEMD_MOUNT := proc-xen.mount
 XEN_SYSTEMD_MOUNT += var-lib-xenstored.mount
 
-XEN_SYSTEMD_SERVICE  = xenstored.service
+XEN_SYSTEMD_SERVICE := xenstored.service
 XEN_SYSTEMD_SERVICE += xenconsoled.service
 XEN_SYSTEMD_SERVICE += xen-qemu-dom0-disk-backend.service
 XEN_SYSTEMD_SERVICE += xendomains.service
@@ -14,7 +14,7 @@  XEN_SYSTEMD_SERVICE += xen-watchdog.service
 XEN_SYSTEMD_SERVICE += xen-init-dom0.service
 XEN_SYSTEMD_SERVICE += xendriverdomain.service
 
-ALL_XEN_SYSTEMD =	$(XEN_SYSTEMD_MODULES)  \
+ALL_XEN_SYSTEMD :=	$(XEN_SYSTEMD_MODULES)  \
 			$(XEN_SYSTEMD_MOUNT)	\
 			$(XEN_SYSTEMD_SERVICE)
 
@@ -30,10 +30,8 @@  distclean: clean
 
 .PHONY: install
 install: $(ALL_XEN_SYSTEMD)
-	[ -d $(DESTDIR)$(XEN_SYSTEMD_DIR) ] || \
-		$(INSTALL_DIR) $(DESTDIR)$(XEN_SYSTEMD_DIR)
-	[ -d $(DESTDIR)$(XEN_SYSTEMD_MODULES_LOAD) ] || \
-		$(INSTALL_DIR) $(DESTDIR)$(XEN_SYSTEMD_MODULES_LOAD)
+	$(INSTALL_DIR) $(DESTDIR)$(XEN_SYSTEMD_DIR)
+	$(INSTALL_DIR) $(DESTDIR)$(XEN_SYSTEMD_MODULES_LOAD)
 	$(INSTALL_DATA) *.service $(DESTDIR)$(XEN_SYSTEMD_DIR)
 	$(INSTALL_DATA) *.mount $(DESTDIR)$(XEN_SYSTEMD_DIR)
 	$(INSTALL_DATA) *.conf $(DESTDIR)$(XEN_SYSTEMD_MODULES_LOAD)
@@ -48,5 +46,5 @@  $(XEN_SYSTEMD_MODULES):
 	rm -f $@.tmp
 	for mod in $(LINUX_BACKEND_MODULES) ; do \
 		echo $$mod ; \
-		done > $@.tmp
+	done > $@.tmp
 	$(call move-if-changed,$@.tmp,$@)
diff --git a/tools/hotplug/NetBSD/Makefile b/tools/hotplug/NetBSD/Makefile
index f909ffa367..1cd3db2ccb 100644
--- a/tools/hotplug/NetBSD/Makefile
+++ b/tools/hotplug/NetBSD/Makefile
@@ -2,22 +2,19 @@  XEN_ROOT = $(CURDIR)/../../..
 include $(XEN_ROOT)/tools/Rules.mk
 
 # Xen script dir and scripts to go there.
-XEN_SCRIPTS =
+XEN_SCRIPTS :=
 XEN_SCRIPTS += locking.sh
 XEN_SCRIPTS += block
 XEN_SCRIPTS += vif-bridge
 XEN_SCRIPTS += vif-ip
 XEN_SCRIPTS += qemu-ifup
 
-XEN_SCRIPT_DATA =
-XEN_RCD_PROG = rc.d/xencommons rc.d/xendomains rc.d/xen-watchdog rc.d/xendriverdomain
+XEN_SCRIPT_DATA :=
+XEN_RCD_PROG := rc.d/xencommons rc.d/xendomains rc.d/xen-watchdog rc.d/xendriverdomain
 
 .PHONY: all
 all:
 
-.PHONY: build
-build:
-
 .PHONY: install
 install: install-scripts install-rcd
 
diff --git a/tools/hotplug/common/Makefile b/tools/hotplug/common/Makefile
index ef48bfacc9..e8a8dbea6c 100644
--- a/tools/hotplug/common/Makefile
+++ b/tools/hotplug/common/Makefile
@@ -1,22 +1,19 @@ 
 XEN_ROOT = $(CURDIR)/../../..
 include $(XEN_ROOT)/tools/Rules.mk
 
-HOTPLUGPATH="hotplugpath.sh"
+HOTPLUGPATH := hotplugpath.sh
 
 # OS-independent hotplug scripts go in this directory
 
 # Xen scripts to go there.
-XEN_SCRIPTS =
-XEN_SCRIPT_DATA = $(HOTPLUGPATH)
+XEN_SCRIPTS :=
+XEN_SCRIPT_DATA := $(HOTPLUGPATH)
 
 genpath-target = $(call buildmakevars2file,$(HOTPLUGPATH))
 $(eval $(genpath-target))
 
 .PHONY: all
-all: build
-
-.PHONY: build
-build: $(HOTPLUGPATH)
+all: $(HOTPLUGPATH)
 
 .PHONY: install
 install: install-scripts
@@ -25,9 +22,8 @@  install: install-scripts
 uninstall: uninstall-scripts
 
 .PHONY: install-scripts
-install-scripts: build
-	[ -d $(DESTDIR)$(XEN_SCRIPT_DIR) ] || \
-		$(INSTALL_DIR) $(DESTDIR)$(XEN_SCRIPT_DIR)
+install-scripts: all
+	$(INSTALL_DIR) $(DESTDIR)$(XEN_SCRIPT_DIR)
 	set -e; for i in $(XEN_SCRIPTS); \
 	   do \
 	   $(INSTALL_PROG) $$i $(DESTDIR)$(XEN_SCRIPT_DIR); \