[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] Force build failure if modules build fails



Mark McLoughlin writes ("[Xen-devel] [PATCH] Force build failure if modules 
build fails"):
>       if grep "^CONFIG_MODULES=" $(LINUX_DIR)/.config ; then \
> -         $(MAKE) -C $(LINUX_DIR) ARCH=$(LINUX_ARCH) modules ; \
> +         $(MAKE) -C $(LINUX_DIR) ARCH=$(LINUX_ARCH) modules || exit 1 ; \

Usually a better way to do this is to add `set -e' to the start of the
multi-statement shell command, like this:

  -     if grep "^CONFIG_MODULES=" $(LINUX_DIR)/.config ; then \
  +     set -e; if grep "^CONFIG_MODULES=" $(LINUX_DIR)/.config ; then \

Doing it that way means you don't have to keep track of exactly which
parts need `|| exit 1'.  Best practice is to start any nontrivial
command in a makefile with `set -e'.

I've done some searching through the code and come up with the patch
below which fixes a number of similar mistakes.  I've only intended to
change places where the code was definitely wrong, rather than the
IMO much larger set of places where set -e would have been good style
but is technically unnecessary.

I've also checked that at least `make all' still works.

Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

Ian.

diff -r e2fa79c659e2 buildconfigs/mk.linux-2.6-common
--- a/buildconfigs/mk.linux-2.6-common  Thu Dec 20 10:44:06 2007 +0000
+++ b/buildconfigs/mk.linux-2.6-common  Thu Dec 20 11:13:14 2007 +0000
@@ -103,7 +103,7 @@ ifneq ($(EXTRAVERSION),)
        echo "$(EXTRAVERSION)" >$(LINUX_DIR)/localversion-xen
 endif
        $(MAKE) -C $(LINUX_SRCDIR) ARCH=$(LINUX_ARCH) oldconfig 
O=$$(/bin/pwd)/$(LINUX_DIR)
-       @if [ ! -f $(LINUX_DIR)/Makefile ] ; then \
+       @set -e ; if [ ! -f $(LINUX_DIR)/Makefile ] ; then \
            echo "***********************************"; \
            echo "oldconfig did not create a Makefile"; \
            echo "Generating $(LINUX_DIR)/Makefile   "; \
diff -r e2fa79c659e2 buildconfigs/src.hg-clone
--- a/buildconfigs/src.hg-clone Thu Dec 20 10:44:06 2007 +0000
+++ b/buildconfigs/src.hg-clone Thu Dec 20 10:53:36 2007 +0000
@@ -25,7 +25,7 @@ XEN_LINUX_HGREV  ?= tip
            echo "Pulling changes from $${__parent} into $(LINUX_SRCDIR)." ; \
            $(HG) -R $(LINUX_SRCDIR) pull $${__parent} ; \
        fi
-       if [ -n "$(XEN_LINUX_HGREV)" ] ; then \
+       set -e ; if [ -n "$(XEN_LINUX_HGREV)" ] ; then \
            echo "Updating $(LINUX_SRCDIR) to revision $(XEN_LINUX_HGREV)." ; \
            ( cd $(LINUX_SRCDIR) && $(HG) update $(XEN_LINUX_HGREV) ); \
        fi
diff -r e2fa79c659e2 docs/Makefile
--- a/docs/Makefile     Thu Dec 20 10:44:06 2007 +0000
+++ b/docs/Makefile     Thu Dec 20 10:56:08 2007 +0000
@@ -44,7 +44,7 @@ html:
 .PHONY: python-dev-docs
 python-dev-docs:
        @mkdir -v -p api/tools/python
-       @if which $(DOXYGEN) 1>/dev/null 2>/dev/null; then         \
+       @set -e ; if which $(DOXYGEN) 1>/dev/null 2>/dev/null; then \
         echo "Running doxygen to generate Python tools APIs ... "; \
        $(DOXYGEN) Doxyfile;                                       \
        $(MAKE) -C api/tools/python/latex ; else                   \
diff -r e2fa79c659e2 tools/examples/Makefile
--- a/tools/examples/Makefile   Thu Dec 20 10:44:06 2007 +0000
+++ b/tools/examples/Makefile   Thu Dec 20 11:04:23 2007 +0000
@@ -76,7 +76,7 @@ install-configs: $(XEN_CONFIGS)
                $(INSTALL_DIR) $(DESTDIR)$(XEN_CONFIG_DIR)
        [ -d $(DESTDIR)$(XEN_CONFIG_DIR)/auto ] || \
                $(INSTALL_DIR) $(DESTDIR)$(XEN_CONFIG_DIR)/auto
-       for i in $(XEN_CONFIGS); \
+       set -e; for i in $(XEN_CONFIGS); \
            do [ -e $(DESTDIR)$(XEN_CONFIG_DIR)/$$i ] || \
            $(INSTALL_DATA) $$i $(DESTDIR)$(XEN_CONFIG_DIR); \
        done
@@ -85,11 +85,11 @@ install-scripts:
 install-scripts:
        [ -d $(DESTDIR)$(XEN_SCRIPT_DIR) ] || \
                $(INSTALL_DIR) $(DESTDIR)$(XEN_SCRIPT_DIR)
-       for i in $(XEN_SCRIPTS); \
+       set -e; for i in $(XEN_SCRIPTS); \
            do \
            $(INSTALL_PROG) $$i $(DESTDIR)$(XEN_SCRIPT_DIR); \
        done
-       for i in $(XEN_SCRIPT_DATA); \
+       set -e; for i in $(XEN_SCRIPT_DATA); \
            do \
            $(INSTALL_DATA) $$i $(DESTDIR)$(XEN_SCRIPT_DIR); \
        done
@@ -98,7 +98,7 @@ install-hotplug:
 install-hotplug:
        [ -d $(DESTDIR)$(XEN_HOTPLUG_DIR) ] || \
                $(INSTALL_DIR) $(DESTDIR)$(XEN_HOTPLUG_DIR)
-       for i in $(XEN_HOTPLUG_SCRIPTS); \
+       set -e; for i in $(XEN_HOTPLUG_SCRIPTS); \
            do \
            $(INSTALL_PROG) $$i $(DESTDIR)$(XEN_HOTPLUG_DIR); \
        done
@@ -107,11 +107,10 @@ install-udev:
 install-udev:
        [ -d $(DESTDIR)$(UDEV_RULES_DIR) ] || \
                $(INSTALL_DIR) $(DESTDIR)$(UDEV_RULES_DIR)/rules.d
-       for i in $(UDEV_RULES); \
+       set -e; for i in $(UDEV_RULES); \
            do \
            $(INSTALL_DATA) $$i $(DESTDIR)$(UDEV_RULES_DIR); \
-           ( cd $(DESTDIR)$(UDEV_RULES_DIR)/rules.d ; \
-               ln -sf ../$$i . ) \
+           ln -sf ../$$i $(DESTDIR)$(UDEV_RULES_DIR)/rules.d; \
        done
 
 .PHONY: clean
diff -r e2fa79c659e2 tools/firmware/rombios/32bit/Makefile
--- a/tools/firmware/rombios/32bit/Makefile     Thu Dec 20 10:44:06 2007 +0000
+++ b/tools/firmware/rombios/32bit/Makefile     Thu Dec 20 11:10:27 2007 +0000
@@ -19,7 +19,7 @@ MODULES = tcgbios/tcgbiosext.o
 .PHONY: all subdirs clean
 
 subdirs:
-       @for subdir in $(SUBDIRS); do \
+       @set -e; for subdir in $(SUBDIRS); do \
                $(MAKE) -C $$subdir all; \
        done;
 
diff -r e2fa79c659e2 tools/ioemu/Makefile
--- a/tools/ioemu/Makefile      Thu Dec 20 10:44:06 2007 +0000
+++ b/tools/ioemu/Makefile      Thu Dec 20 11:04:25 2007 +0000
@@ -89,7 +89,7 @@ install: all $(if $(BUILD_DOCS),install-
 #      done
 ifndef CONFIG_WIN32
        mkdir -p "$(DESTDIR)$(datadir)/keymaps"
-       for x in $(KEYMAPS); do \
+       set -e; for x in $(KEYMAPS); do \
                $(INSTALL_DATA) -m 644 $(SRC_PATH)/keymaps/$$x 
"$(DESTDIR)$(datadir)/keymaps"; \
        done
 endif
@@ -140,12 +140,12 @@ tar:
 tar:
        rm -rf /tmp/$(FILE)
        cp -r . /tmp/$(FILE)
-       ( cd /tmp ; tar zcvf ~/$(FILE).tar.gz $(FILE) --exclude CVS )
+       cd /tmp && tar zcvf ~/$(FILE).tar.gz $(FILE) --exclude CVS
        rm -rf /tmp/$(FILE)
 
 # generate a binary distribution
 tarbin:
-       ( cd / ; tar zcvf ~/qemu-$(VERSION)-i386.tar.gz \
+       cd / && tar zcvf ~/qemu-$(VERSION)-i386.tar.gz \
        $(bindir)/qemu \
        $(bindir)/qemu-system-ppc \
        $(bindir)/qemu-system-sparc \
@@ -173,7 +173,7 @@ tarbin:
         $(datadir)/pxe-pcnet.bin \
        $(docdir)/qemu-doc.html \
        $(docdir)/qemu-tech.html \
-       $(mandir)/man1/qemu.1 $(mandir)/man1/qemu-img.1 )
+       $(mandir)/man1/qemu.1 $(mandir)/man1/qemu-img.1
 
 ifneq ($(wildcard .depend),)
 include .depend
diff -r e2fa79c659e2 tools/ioemu/tests/Makefile
--- a/tools/ioemu/tests/Makefile        Thu Dec 20 10:44:06 2007 +0000
+++ b/tools/ioemu/tests/Makefile        Thu Dec 20 11:04:25 2007 +0000
@@ -91,7 +91,7 @@ hello-mipsel: hello-mips.c
 
 # XXX: find a way to compile easily a test for each arch
 test2:
-       @for arch in i386 arm armeb sparc ppc mips mipsel; do \
+       @set -e; for arch in i386 arm armeb sparc ppc mips mipsel; do \
            ../$${arch}-linux-user/qemu-$${arch} $${arch}/ls -l linux-test.c ; \
         done
 
diff -r e2fa79c659e2 tools/libaio/Makefile
--- a/tools/libaio/Makefile     Thu Dec 20 10:44:06 2007 +0000
+++ b/tools/libaio/Makefile     Thu Dec 20 11:04:25 2007 +0000
@@ -25,9 +25,9 @@ tag-archive:
 
 create-archive: tag-archive
        @rm -rf /tmp/$(NAME)
-       @cd /tmp; cvs -Q -d $(CVSROOT) export -r$(CVSTAG) $(NAME) || echo 
GRRRrrrrr -- ignore [export aborted]
+       @cd /tmp && cvs -Q -d $(CVSROOT) export -r$(CVSTAG) $(NAME) || echo 
GRRRrrrrr -- ignore [export aborted]
        @mv /tmp/$(NAME) /tmp/$(NAME)-$(VERSION)
-       @cd /tmp; tar czSpf $(NAME)-$(VERSION).tar.gz $(NAME)-$(VERSION)
+       @cd /tmp && tar czSpf $(NAME)-$(VERSION).tar.gz $(NAME)-$(VERSION)
        @rm -rf /tmp/$(NAME)-$(VERSION)
        @cp /tmp/$(NAME)-$(VERSION).tar.gz .
        @rm -f /tmp/$(NAME)-$(VERSION).tar.gz 
diff -r e2fa79c659e2 tools/libxen/Makefile
--- a/tools/libxen/Makefile     Thu Dec 20 10:44:06 2007 +0000
+++ b/tools/libxen/Makefile     Thu Dec 20 11:04:25 2007 +0000
@@ -61,7 +61,7 @@ install: all
        ln -sf libxenapi.so.$(MAJOR).$(MINOR) 
$(DESTDIR)/usr/$(LIBDIR)/libxenapi.so.$(MAJOR)
        ln -sf libxenapi.so.$(MAJOR) $(DESTDIR)/usr/$(LIBDIR)/libxenapi.so
        $(INSTALL_DATA) libxenapi.a $(DESTDIR)/usr/$(LIBDIR)
-       for i in $(LIBXENAPI_HDRS); do \
+       set -e; for i in $(LIBXENAPI_HDRS); do \
            $(INSTALL_DATA) $$i $(DESTDIR)/usr/include/xen/api; \
        done
 
diff -r e2fa79c659e2 tools/libxen/Makefile.dist
--- a/tools/libxen/Makefile.dist        Thu Dec 20 10:44:06 2007 +0000
+++ b/tools/libxen/Makefile.dist        Thu Dec 20 11:04:25 2007 +0000
@@ -71,7 +71,7 @@ install: all
        ln -sf libxenapi.so.$(MAJOR).$(MINOR) 
$(DESTDIR)/usr/$(LIBDIR)/libxenapi.so.$(MAJOR)
        ln -sf libxenapi.so.$(MAJOR) $(DESTDIR)/usr/$(LIBDIR)/libxenapi.so
        $(INSTALL_DATA) libxenapi.a $(DESTDIR)/usr/$(LIBDIR)
-       for i in $(LIBXENAPI_HDRS); do \
+       set -e; for i in $(LIBXENAPI_HDRS); do \
            $(INSTALL_DATA) $$i $(DESTDIR)/usr/include/xen/api; \
        done
 
diff -r e2fa79c659e2 tools/python/Makefile
--- a/tools/python/Makefile     Thu Dec 20 10:44:06 2007 +0000
+++ b/tools/python/Makefile     Thu Dec 20 11:10:27 2007 +0000
@@ -40,7 +40,7 @@ refresh-pot: $(I18NSRCFILES)
                $(I18NSRCFILES)
        sed -f remove-potcdate.sed < $(POTFILE) > $(POTFILE)-1
        sed -f remove-potcdate.sed < $(POTFILE)-tmp > $(POTFILE)-2
-       if cmp -s $(POTFILE)-1 $(POTFILE)-2; then \
+       set -e; if cmp -s $(POTFILE)-1 $(POTFILE)-2; then \
                rm -f $(POTFILE)-tmp $(POTFILE)-1 $(POTFILE)-2; \
        else \
                mv $(POTFILE)-tmp $(POTFILE); \
@@ -48,7 +48,7 @@ refresh-pot: $(I18NSRCFILES)
        fi
 
 refresh-po: $(POTFILE)
-       for l in $(LINGUAS); do \
+       set -e; for l in $(LINGUAS); do \
                if $(MSGMERGE) $(PODIR)/$$l.po $(POTFILE) > $(PODIR)/$$l-tmp ; 
then \
                        mv -f $(PODIR)/$$l-tmp $(PODIR)/$$l.po ; \
                        echo "$(MSGMERGE) of $$l.po succeeded" ; \
@@ -88,7 +88,7 @@ install-dtd: all
        $(INSTALL_DATA) xen/xm/create.dtd $(DESTDIR)/usr/share/xen
 
 install-messages: all
-       if which $(MSGFMT) >/dev/null ; then \
+       set -e; if which $(MSGFMT) >/dev/null ; then \
                mkdir -p $(DESTDIR)$(NLSDIR); \
                for l in $(LINGUAS); do \
                        $(INSTALL_DIR) $(DESTDIR)$(NLSDIR)/$$l; \
diff -r e2fa79c659e2 tools/security/Makefile
--- a/tools/security/Makefile   Thu Dec 20 10:44:06 2007 +0000
+++ b/tools/security/Makefile   Thu Dec 20 11:04:25 2007 +0000
@@ -51,10 +51,10 @@ install: all $(ACM_CONFIG_FILE)
        $(INSTALL_DIR) $(DESTDIR)$(ACM_POLICY_DIR)
        $(INSTALL_DATA) policies/$(ACM_SCHEMA) $(DESTDIR)$(ACM_POLICY_DIR)
        $(INSTALL_DIR) $(DESTDIR)$(ACM_POLICY_DIR)/example
-       for i in $(ACM_EXAMPLES); do \
+       set -e; for i in $(ACM_EXAMPLES); do \
                $(INSTALL_DATA) policies/example/$$i-$(ACM_POLICY_SUFFIX) 
$(DESTDIR)$(ACM_POLICY_DIR)/example/; \
        done
-       for i in $(ACM_DEF_POLICIES); do \
+       set -e; for i in $(ACM_DEF_POLICIES); do \
                $(INSTALL_DATA) policies/$$i-$(ACM_POLICY_SUFFIX) 
$(DESTDIR)$(ACM_POLICY_DIR); \
        done
        $(INSTALL_DIR) $(DESTDIR)$(ACM_SCRIPT_DIR)
diff -r e2fa79c659e2 tools/vtpm/Makefile
--- a/tools/vtpm/Makefile       Thu Dec 20 10:44:06 2007 +0000
+++ b/tools/vtpm/Makefile       Thu Dec 20 11:10:27 2007 +0000
@@ -49,7 +49,7 @@ mrproper:
 # Create vtpm and TPM emulator dirs
 # apply patches for 1) used as dom0 tpm driver 2) used as vtpm device instance
 $(TPM_EMULATOR_DIR): $(TPM_EMULATOR_TARFILE) tpm_emulator.patch 
-       if [ "$(BUILD_EMULATOR)" = "y" ]; then \
+       set -e; if [ "$(BUILD_EMULATOR)" = "y" ]; then \
                rm -rf $(TPM_EMULATOR_DIR); \
                tar -xzf $(TPM_EMULATOR_TARFILE); \
                mv $(TPM_EMULATOR_NAME) $(TPM_EMULATOR_DIR); \
@@ -59,20 +59,20 @@ mrproper:
 
 $(VTPM_DIR): $(TPM_EMULATOR_TARFILE) vtpm.patch
        rm -rf $(VTPM_DIR)
-       tar -xzf $(TPM_EMULATOR_TARFILE);  
-       mv $(TPM_EMULATOR_NAME) $(VTPM_DIR); 
+       tar -xzf $(TPM_EMULATOR_TARFILE)
+       mv $(TPM_EMULATOR_NAME) $(VTPM_DIR)
 
-       cd $(VTPM_DIR); \
+       set -e; cd $(VTPM_DIR); \
        patch -p1 < ../tpm_emulator.patch; \
        patch -p1 < ../vtpm.patch
 
 orig: $(TPM_EMULATOR_TARFILE)
        mkdir $(ORIG_DIR);
-       cd $(ORIG_DIR); \
+       set -e; cd $(ORIG_DIR); \
        tar -xzf ../$(TPM_EMULATOR_TARFILE);
 
 updatepatches: clean orig
-       if [ "$(BUILD_EMULATOR)" = "y" ]; then \
+       set -e; if [ "$(BUILD_EMULATOR)" = "y" ]; then \
                find $(TPM_EMULATOR_DIR) -name "*.orig" -print | xargs rm -f; \
                mv tpm_emulator.patch tpm_emulator.patch.old; \
                diff -uprN orig/$(TPM_EMULATOR_NAME) $(TPM_EMULATOR_DIR) > 
tpm_emulator.patch || true; \
@@ -83,7 +83,7 @@ updatepatches: clean orig
 
 .PHONY: build_sub
 build_sub:
-       @if [ -e $(GMP_HEADER) ]; then \
+       set -e; @if [ -e $(GMP_HEADER) ]; then \
                $(MAKE) -C $(VTPM_DIR); \
                if [ "$(BUILD_EMULATOR)" = "y" ]; then \
                        $(MAKE) -C $(TPM_EMULATOR_DIR); \
diff -r e2fa79c659e2 tools/xm-test/ramdisk/Makefile.am
--- a/tools/xm-test/ramdisk/Makefile.am Thu Dec 20 10:44:06 2007 +0000
+++ b/tools/xm-test/ramdisk/Makefile.am Thu Dec 20 10:57:44 2007 +0000
@@ -59,8 +59,8 @@ endif
        cp configs/buildroot-$(BR_ARCH) $(BR_SRC)/.config
        cp configs/busybox $(BR_SRC)/package/busybox/busybox.config
        cp configs/uClibc $(BR_SRC)/toolchain/uClibc/uClibc.config
-       (for i in patches/buildroot/*.patch; do \
-         cd $(BR_SRC) && patch -p1 <../$$i && cd ..; done )
+       set -e; for i in patches/buildroot/*.patch; do \
+         cd $(BR_SRC) && patch -p1 <../$$i && cd ..; done
        cd $(BR_SRC) && make oldconfig && make
 
 $(XMTEST_VER_IMG): $(BR_IMG)
@@ -69,8 +69,8 @@ endif
        -[ -e "$(BLKDRV)" ] && cp $(BLKDRV) skel/modules
        -[ -e "$(NETDRV)" ] && cp $(NETDRV) skel/modules
        -[ -e "$(PKTDRV)" ] && cp $(PKTDRV) skel/modules
-       (cd skel; tar cf - .) \
-               | (cd $(BR_SRC)/$(BR_ROOT); tar xvf -)
+       (cd skel && tar cf - .) \
+               | (cd $(BR_SRC)/$(BR_ROOT) && tar xvf -)
        cd $(BR_SRC) && make
        cp $(BR_IMG) $(XMTEST_VER_IMG)
 
diff -r e2fa79c659e2 xen/Makefile
--- a/xen/Makefile      Thu Dec 20 10:44:06 2007 +0000
+++ b/xen/Makefile      Thu Dec 20 11:12:26 2007 +0000
@@ -141,13 +141,13 @@ endef
 
 .PHONY: _TAGS
 _TAGS: 
-       rm -f TAGS; \
+       set -e; rm -f TAGS; \
        $(call set_exuberant_flags,etags); \
        $(all_sources) | xargs etags $$exuberant_flags -a
 
 .PHONY: _tags
 _tags: 
-       rm -f tags; \
+       set -e; rm -f tags; \
        $(call set_exuberant_flags,ctags); \
        $(all_sources) | xargs ctags $$exuberant_flags -a
 
diff -r e2fa79c659e2 xen/include/Makefile
--- a/xen/include/Makefile      Thu Dec 20 10:44:06 2007 +0000
+++ b/xen/include/Makefile      Thu Dec 20 11:12:26 2007 +0000
@@ -37,7 +37,7 @@ all: $(headers-y)
 all: $(headers-y)
 
 compat/%.h: compat/%.i Makefile
-       id=_$$(echo $@ | tr '[:lower:]-/.' '[:upper:]___'); \
+       set -e; id=_$$(echo $@ | tr '[:lower:]-/.' '[:upper:]___'); \
        echo "#ifndef $$id" >$@.new; \
        echo "#define $$id" >>$@.new; \
        echo "#include <xen/compat.h>" >>$@.new; \
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.