| Message ID | 20211206170241.13165-15-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Toolstack build system improvement, toward non-recursive makefiles | expand |
On 12/6/21 12:01 PM, Anthony PERARD wrote: > They are no *.opic or *.so in this subdir, so no need to clean them. > > The TEST* variables doesn't seems to be used anywhere, and they weren't > used by xen.git when introduced. > Both CLIENTS_* variables aren't used. > Both target "print-dir" and "print-end" only exist in this directory > and are probably not used anywhere. > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
On 06/12/2021 17:01, Anthony PERARD wrote: > @@ -35,21 +29,13 @@ flask-set-bool: set-bool.o > > .PHONY: clean > clean: > - rm -f *.o *.opic *.so > + rm -f *.o > rm -f $(CLIENTS) > $(RM) $(DEPS_RM) Can we collapse this to $(RM) *.o $(CLIENTS) $(DEPS_RM) ? Here and in plenty of subsequent patches, there's manipulation of raw `rm -f`'s which ought to be cleaned up to $(RM) I can fix this on commit if you're happy. ~Andrew
On Thu, Dec 16, 2021 at 12:36:41PM +0000, Andrew Cooper wrote: > On 06/12/2021 17:01, Anthony PERARD wrote: > > @@ -35,21 +29,13 @@ flask-set-bool: set-bool.o > > > > .PHONY: clean > > clean: > > - rm -f *.o *.opic *.so > > + rm -f *.o > > rm -f $(CLIENTS) > > $(RM) $(DEPS_RM) > > Here and in plenty of subsequent patches, there's manipulation of raw > `rm -f`'s which ought to be cleaned up to $(RM) About using $(RM) or `rm -f`, I don't think we need to care if one or the other is used in Makefiles, they needs to be equivalent or things are going to fails. GNU make manual says that `rm` should exist, especially when things like autoconf are used. For example our ./configure scrips is using `rm -f` and I don't think we can configure that. So having RM configurable doesn't really serve any purpose. Beyond that, using $(RM) is good in order to be consistent, and changing "rm -f" to "$(RM)" is fine, I might not doing it myself or ask for it. > I can fix this on commit if you're happy. For the avoidance of doubt, yes, I'm happy with the patch that have been committed. Thanks,
diff --git a/tools/flask/utils/Makefile b/tools/flask/utils/Makefile index ae87102144..5449f05b13 100644 --- a/tools/flask/utils/Makefile +++ b/tools/flask/utils/Makefile @@ -4,13 +4,7 @@ include $(XEN_ROOT)/tools/Rules.mk CFLAGS += -Werror CFLAGS += $(CFLAGS_libxenctrl) -TESTDIR = testsuite/tmp -TESTFLAGS= -DTESTING -TESTENV = XENSTORED_ROOTDIR=$(TESTDIR) XENSTORED_RUNDIR=$(TESTDIR) - CLIENTS := flask-loadpolicy flask-setenforce flask-getenforce flask-label-pci flask-get-bool flask-set-bool -CLIENTS_SRCS := $(patsubst flask-%,%.c,$(CLIENTS)) -CLIENTS_OBJS := $(patsubst flask-%,%.o,$(CLIENTS)) .PHONY: all all: $(CLIENTS) @@ -35,21 +29,13 @@ flask-set-bool: set-bool.o .PHONY: clean clean: - rm -f *.o *.opic *.so + rm -f *.o rm -f $(CLIENTS) $(RM) $(DEPS_RM) .PHONY: distclean distclean: clean -.PHONY: print-dir -print-dir: - @echo -n tools/flask/utils: - -.PHONY: print-end -print-end: - @echo - .PHONY: install install: all $(INSTALL_DIR) $(DESTDIR)$(sbindir)
They are no *.opic or *.so in this subdir, so no need to clean them. The TEST* variables doesn't seems to be used anywhere, and they weren't used by xen.git when introduced. Both CLIENTS_* variables aren't used. Both target "print-dir" and "print-end" only exist in this directory and are probably not used anywhere. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- tools/flask/utils/Makefile | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-)