| Message ID | 20211206170241.13165-26-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Toolstack build system improvement, toward non-recursive makefiles | expand |
On 06/12/2021 17:02, Anthony PERARD wrote: > diff --git a/tools/examples/Makefile b/tools/examples/Makefile > index 14e24f4cb3..48b520e133 100644 > --- a/tools/examples/Makefile > +++ b/tools/examples/Makefile > @@ -26,10 +22,8 @@ uninstall: uninstall-readmes uninstall-configs > > .PHONY: install-readmes > install-readmes: > - [ -d $(DESTDIR)$(XEN_CONFIG_DIR) ] || \ > - $(INSTALL_DIR) $(DESTDIR)$(XEN_CONFIG_DIR) > - set -e; for i in $(XEN_READMES); \ > - do [ -e $(DESTDIR)$(XEN_CONFIG_DIR)/$$i ] || \ > + $(INSTALL_DIR) $(DESTDIR)$(XEN_CONFIG_DIR) > + set -e; for i in $(XEN_READMES); do \ > $(INSTALL_DATA) $$i $(DESTDIR)$(XEN_CONFIG_DIR); \ Hmm. Do we need a shell loop here at all? Can't $(INSTALL_DATA) take multiple $i's at the same time? ~Andrew
On Thu, Dec 16, 2021 at 05:57:43PM +0000, Andrew Cooper wrote: > On 06/12/2021 17:02, Anthony PERARD wrote: > > diff --git a/tools/examples/Makefile b/tools/examples/Makefile > > index 14e24f4cb3..48b520e133 100644 > > --- a/tools/examples/Makefile > > +++ b/tools/examples/Makefile > > @@ -26,10 +22,8 @@ uninstall: uninstall-readmes uninstall-configs > > > > .PHONY: install-readmes > > install-readmes: > > - [ -d $(DESTDIR)$(XEN_CONFIG_DIR) ] || \ > > - $(INSTALL_DIR) $(DESTDIR)$(XEN_CONFIG_DIR) > > - set -e; for i in $(XEN_READMES); \ > > - do [ -e $(DESTDIR)$(XEN_CONFIG_DIR)/$$i ] || \ > > + $(INSTALL_DIR) $(DESTDIR)$(XEN_CONFIG_DIR) > > + set -e; for i in $(XEN_READMES); do \ > > $(INSTALL_DATA) $$i $(DESTDIR)$(XEN_CONFIG_DIR); \ > > Hmm. Do we need a shell loop here at all? Can't $(INSTALL_DATA) take > multiple $i's at the same time? Probably, even if the only time they are multiple filed install by INSTALL_DATA in our build system is when shell globing is involve. So, it's probably fine to remove the loop. Thanks,
diff --git a/tools/examples/Makefile b/tools/examples/Makefile index 14e24f4cb3..48b520e133 100644 --- a/tools/examples/Makefile +++ b/tools/examples/Makefile @@ -2,6 +2,7 @@ XEN_ROOT = $(CURDIR)/../.. include $(XEN_ROOT)/tools/Rules.mk # Xen configuration dir and configs to go there. + XEN_READMES = README XEN_CONFIGS += xlexample.hvm @@ -10,14 +11,9 @@ XEN_CONFIGS += xlexample.pvhlinux XEN_CONFIGS += xl.conf XEN_CONFIGS += cpupool -XEN_CONFIGS += $(XEN_CONFIGS-y) - .PHONY: all all: -.PHONY: build -build: - .PHONY: install install: all install-readmes install-configs @@ -26,10 +22,8 @@ uninstall: uninstall-readmes uninstall-configs .PHONY: install-readmes install-readmes: - [ -d $(DESTDIR)$(XEN_CONFIG_DIR) ] || \ - $(INSTALL_DIR) $(DESTDIR)$(XEN_CONFIG_DIR) - set -e; for i in $(XEN_READMES); \ - do [ -e $(DESTDIR)$(XEN_CONFIG_DIR)/$$i ] || \ + $(INSTALL_DIR) $(DESTDIR)$(XEN_CONFIG_DIR) + set -e; for i in $(XEN_READMES); do \ $(INSTALL_DATA) $$i $(DESTDIR)$(XEN_CONFIG_DIR); \ done @@ -39,12 +33,9 @@ uninstall-readmes: .PHONY: install-configs install-configs: $(XEN_CONFIGS) - [ -d $(DESTDIR)$(XEN_CONFIG_DIR) ] || \ - $(INSTALL_DIR) $(DESTDIR)$(XEN_CONFIG_DIR) - [ -d $(DESTDIR)$(XEN_CONFIG_DIR)/auto ] || \ - $(INSTALL_DIR) $(DESTDIR)$(XEN_CONFIG_DIR)/auto - set -e; for i in $(XEN_CONFIGS); \ - do [ -e $(DESTDIR)$(XEN_CONFIG_DIR)/$$i ] || \ + $(INSTALL_DIR) $(DESTDIR)$(XEN_CONFIG_DIR) + $(INSTALL_DIR) $(DESTDIR)$(XEN_CONFIG_DIR)/auto + set -e; for i in $(XEN_CONFIGS); do \ $(INSTALL_DATA) $$i $(DESTDIR)$(XEN_CONFIG_DIR); \ done
Don't check if a target exist before installing it. For directory, install doesn't complain, and for file it would prevent from updating them. Remove XEN_CONFIGS-y which isn't used. Remove "build" target. Add an empty line after the first comment. The comment isn't about $(XEN_READMES), it is about the makefile as a whole. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- tools/examples/Makefile | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-)