From f1bcb91542bc7a61f159e54a90729f610d801f6b Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Thu, 12 Apr 2018 02:48:02 -0700 Subject: [PATCH] [make] make sure no command fails Summary: Run with `SHELL = bash -e -u -o pipefail` to catch many kinds of failures. We were silently failing during `make install` because of some missing escaping, and the failure was hidden because it was happening inside a bash `for` loop. This fixes the escaping issue and makes sure such issues will result in an error as of now. Also removes dangerous `find -exec` instances: `find` will `exit 0` event if some commands failed. Fixes #887 Reviewed By: mbouaziz Differential Revision: D7569054 fbshipit-source-id: 542fe50 --- Makefile | 176 ++++++++---------- Makefile.config | 18 +- configure.ac | 1 - infer/tests/base.make | 9 +- .../build_systems/fail_on_issue/Makefile | 4 +- infer/tests/clang-frontend.make | 6 +- 6 files changed, 101 insertions(+), 113 deletions(-) diff --git a/Makefile b/Makefile index 89c805217..f6045bc9f 100644 --- a/Makefile +++ b/Makefile @@ -285,8 +285,7 @@ ocaml_unit_test: test_build INFER_ARGS=--results-dir^infer-out-unit-tests $(BUILD_DIR)/test/inferunit.bc) define silence_make - ($(1) 2> >(grep -v "warning: \(ignoring old\|overriding\) \(commands\|recipe\) for target") \ - ; exit $${PIPESTATUS[0]}) + $(1) 2> >(grep -v "warning: \(ignoring old\|overriding\) \(commands\|recipe\) for target") endef .PHONY: $(DIRECT_TESTS:%=direct_%_test) @@ -424,7 +423,7 @@ test-replace: $(BUILD_SYSTEMS_TESTS:%=build_%_replace) $(DIRECT_TESTS:%=direct_% .PHONY: uninstall uninstall: $(REMOVE_DIR) $(DESTDIR)$(libdir)/infer/ - $(REMOVE) $(DESTDIR)$(bindir)/infer + $(REMOVE) $(DESTDIR)$(bindir)/infer* $(REMOVE) $(INFER_COMMANDS:%=$(DESTDIR)$(bindir)/%) $(REMOVE) $(foreach manual,$(INFER_MANUALS_GZIPPED),\ $(DESTDIR)$(mandir)/man1/$(notdir $(manual))) @@ -435,112 +434,101 @@ test_clean: $(DIRECT_TESTS:%=direct_%_clean) $(BUILD_SYSTEMS_TESTS:%=build_%_cle .PHONY: install install: infer $(INFER_MANUALS_GZIPPED) # create directory structure - test -d $(DESTDIR)$(bindir) || \ - $(MKDIR_P) $(DESTDIR)$(bindir) - test -d $(DESTDIR)$(mandir)/man1 || \ - $(MKDIR_P) $(DESTDIR)$(mandir)/man1 - test -d $(DESTDIR)$(libdir)/infer/ || \ - $(MKDIR_P) $(DESTDIR)$(libdir)/infer/ + test -d '$(DESTDIR)$(bindir)' || \ + $(MKDIR_P) '$(DESTDIR)$(bindir)' + test -d '$(DESTDIR)$(mandir)/man1' || \ + $(MKDIR_P) '$(DESTDIR)$(mandir)/man1' + test -d '$(DESTDIR)$(libdir)/infer/' || \ + $(MKDIR_P) '$(DESTDIR)$(libdir)/infer/' ifeq ($(BUILD_C_ANALYZERS),yes) - test -d $(DESTDIR)$(libdir)/infer/facebook-clang-plugins/libtooling/build/ || \ - $(MKDIR_P) $(DESTDIR)$(libdir)/infer/facebook-clang-plugins/libtooling/build/ - $(QUIET)for i in $$(find facebook-clang-plugins/clang/install -type d); do \ - test -d $(DESTDIR)$(libdir)/infer/$$i || \ - $(MKDIR_P) $(DESTDIR)$(libdir)/infer/$$i; \ - done - test -d $(DESTDIR)$(libdir)/infer/infer/lib/clang_wrappers/ || \ - $(MKDIR_P) $(DESTDIR)$(libdir)/infer/infer/lib/clang_wrappers/ - $(QUIET)for i in $$(find infer/models/cpp/include/ -type d); do \ - test -d $(DESTDIR)$(libdir)/infer/$$i || \ - $(MKDIR_P) $(DESTDIR)$(libdir)/infer/$$i; \ - done - test -d $(DESTDIR)$(libdir)/infer/infer/lib/linter_rules/ || \ - $(MKDIR_P) $(DESTDIR)$(libdir)/infer/infer/lib/linter_rules - test -d $(DESTDIR)$(libdir)/infer/infer/etc/ || \ - $(MKDIR_P) $(DESTDIR)$(libdir)/infer/infer/etc + test -d '$(DESTDIR)$(libdir)/infer/facebook-clang-plugins/libtooling/build/' || \ + $(MKDIR_P) '$(DESTDIR)$(libdir)/infer/facebook-clang-plugins/libtooling/build/' + find facebook-clang-plugins/clang/install -type d -print0 | xargs -0 -I \{\} \ + $(SHELL) -c "test -d '$(DESTDIR)$(libdir)'/infer/{} || \ + $(MKDIR_P) '$(DESTDIR)$(libdir)'/infer/{}" + test -d '$(DESTDIR)$(libdir)/infer/infer/lib/clang_wrappers/' || \ + $(MKDIR_P) '$(DESTDIR)$(libdir)/infer/infer/lib/clang_wrappers/' + find infer/models/cpp/include -type d -print0 | xargs -0 -I \{\} \ + $(SHELL) -c "test -d '$(DESTDIR)$(libdir)'/infer/{} || \ + $(MKDIR_P) '$(DESTDIR)$(libdir)'/infer/{}" + test -d '$(DESTDIR)$(libdir)/infer/infer/lib/linter_rules/' || \ + $(MKDIR_P) '$(DESTDIR)$(libdir)/infer/infer/lib/linter_rules/' + test -d '$(DESTDIR)$(libdir)/infer/infer/etc/' || \ + $(MKDIR_P) '$(DESTDIR)$(libdir)/infer/infer/etc' endif ifeq ($(BUILD_JAVA_ANALYZERS),yes) - test -d $(DESTDIR)$(libdir)/infer/infer/lib/java/ || \ - $(MKDIR_P) $(DESTDIR)$(libdir)/infer/infer/lib/java/ -endif - test -d $(DESTDIR)$(libdir)/infer/infer/annotations/ || \ - $(MKDIR_P) $(DESTDIR)$(libdir)/infer/infer/annotations/ - test -d $(DESTDIR)$(libdir)/infer/infer/lib/wrappers/ || \ - $(MKDIR_P) $(DESTDIR)$(libdir)/infer/infer/lib/wrappers/ - test -d $(DESTDIR)$(libdir)/infer/infer/lib/specs/ || \ - $(MKDIR_P) $(DESTDIR)$(libdir)/infer/infer/lib/specs/ - test -d $(DESTDIR)$(libdir)/infer/infer/lib/python/ || \ - $(MKDIR_P) $(DESTDIR)$(libdir)/infer/infer/lib/python/ - test -d $(DESTDIR)$(libdir)/infer/infer/lib/python/inferlib/ || \ - $(MKDIR_P) $(DESTDIR)$(libdir)/infer/infer/lib/python/inferlib/ - test -d $(DESTDIR)$(libdir)/infer/infer/lib/python/inferlib/capture/ || \ - $(MKDIR_P) $(DESTDIR)$(libdir)/infer/infer/lib/python/inferlib/capture/ - test -d $(DESTDIR)$(libdir)/infer/infer/bin/ || \ - $(MKDIR_P) $(DESTDIR)$(libdir)/infer/infer/bin/ - + test -d '$(DESTDIR)$(libdir)/infer/infer/lib/java/' || \ + $(MKDIR_P) '$(DESTDIR)$(libdir)/infer/infer/lib/java/' +endif + test -d '$(DESTDIR)$(libdir)/infer/infer/annotations/' || \ + $(MKDIR_P) '$(DESTDIR)$(libdir)/infer/infer/annotations/' + test -d '$(DESTDIR)$(libdir)/infer/infer/lib/wrappers/' || \ + $(MKDIR_P) '$(DESTDIR)$(libdir)/infer/infer/lib/wrappers/' + test -d '$(DESTDIR)$(libdir)/infer/infer/lib/specs/' || \ + $(MKDIR_P) '$(DESTDIR)$(libdir)/infer/infer/lib/specs/' + test -d '$(DESTDIR)$(libdir)/infer/infer/lib/python/' || \ + $(MKDIR_P) '$(DESTDIR)$(libdir)/infer/infer/lib/python/' + test -d '$(DESTDIR)$(libdir)/infer/infer/lib/python/inferlib/' || \ + $(MKDIR_P) '$(DESTDIR)$(libdir)/infer/infer/lib/python/inferlib/' + test -d '$(DESTDIR)$(libdir)/infer/infer/lib/python/inferlib/capture/' || \ + $(MKDIR_P) '$(DESTDIR)$(libdir)/infer/infer/lib/python/inferlib/capture/' + test -d '$(DESTDIR)$(libdir)/infer/infer/bin/' || \ + $(MKDIR_P) '$(DESTDIR)$(libdir)/infer/infer/bin/' # copy files ifeq ($(BUILD_C_ANALYZERS),yes) - $(INSTALL_DATA) -C facebook-clang-plugins/libtooling/build/FacebookClangPlugin.dylib \ - $(DESTDIR)$(libdir)/infer/facebook-clang-plugins/libtooling/build/FacebookClangPlugin.dylib - $(QUIET)for i in $$(find facebook-clang-plugins/clang/install -not -type d); do \ - $(INSTALL_PROGRAM) -C $$i $(DESTDIR)$(libdir)/infer/$$i; \ - done - $(QUIET)for i in $$(find infer/lib/clang_wrappers/*); do \ - $(INSTALL_PROGRAM) -C $$i $(DESTDIR)$(libdir)/infer/$$i; \ - done + $(INSTALL_DATA) -C 'facebook-clang-plugins/libtooling/build/FacebookClangPlugin.dylib' \ + '$(DESTDIR)$(libdir)/infer/facebook-clang-plugins/libtooling/build/FacebookClangPlugin.dylib' + find facebook-clang-plugins/clang/install -not -type d -print0 | xargs -0 -I \{\} \ + $(INSTALL_PROGRAM) -C \{\} '$(DESTDIR)$(libdir)'/infer/\{\} + find infer/lib/clang_wrappers/* -print0 | xargs -0 -I \{\} \ + $(INSTALL_PROGRAM) -C \{\} '$(DESTDIR)$(libdir)'/infer/\{\} # only for files that point to infer - (cd $(DESTDIR)$(libdir)/infer/infer/lib/wrappers/ && \ - $(foreach cc,$(shell find $(LIB_DIR)/wrappers -type l), \ - [ $(cc) -ef $(INFER_BIN) ] && \ - $(REMOVE) $(notdir $(cc)) && \ - $(LN_S) ../../bin/infer $(notdir $(cc));)) - $(QUIET)for i in $$(find infer/lib/specs/*); do \ - $(INSTALL_DATA) -C $$i $(DESTDIR)$(libdir)/infer/$$i; \ - done - $(QUIET)for i in $$(find infer/models/cpp/include/ -not -type d); do \ - $(INSTALL_DATA) -C $$i $(DESTDIR)$(libdir)/infer/$$i; \ - done - $(INSTALL_DATA) -C infer/lib/linter_rules/linters.al \ - $(DESTDIR)$(libdir)/infer/infer/lib/linter_rules/linters.al - $(INSTALL_DATA) -C infer/etc/clang_ast.dict \ - $(DESTDIR)$(libdir)/infer/infer/etc/clang_ast.dict + (cd '$(DESTDIR)$(libdir)/infer/infer/lib/wrappers/' && \ + $(foreach cc,$(shell find '$(LIB_DIR)/wrappers' -type l), \ + [ $(cc) -ef '$(INFER_BIN)' ] && \ + $(REMOVE) '$(notdir $(cc))' && \ + $(LN_S) ../../bin/infer '$(notdir $(cc))';)) + find infer/lib/specs/* -print0 | xargs -0 -I \{\} \ + $(INSTALL_DATA) -C \{\} '$(DESTDIR)$(libdir)'/infer/\{\} + find infer/models/cpp/include -not -type d -print0 | xargs -0 -I \{\} \ + $(INSTALL_DATA) -C \{\} '$(DESTDIR)$(libdir)'/infer/\{\} + $(INSTALL_DATA) -C 'infer/lib/linter_rules/linters.al' \ + '$(DESTDIR)$(libdir)/infer/infer/lib/linter_rules/linters.al' + $(INSTALL_DATA) -C 'infer/etc/clang_ast.dict' \ + '$(DESTDIR)$(libdir)/infer/infer/etc/clang_ast.dict' endif ifeq ($(BUILD_JAVA_ANALYZERS),yes) - $(INSTALL_DATA) -C infer/annotations/annotations.jar \ - $(DESTDIR)$(libdir)/infer/infer/annotations/annotations.jar - $(QUIET)for i in infer/lib/java/*.jar; do \ - $(INSTALL_DATA) -C $$i $(DESTDIR)$(libdir)/infer/$$i; \ - done - $(INSTALL_PROGRAM) -C $(LIB_DIR)/wrappers/javac \ - $(DESTDIR)$(libdir)/infer/infer/lib/wrappers/ -endif - $(QUIET)for i in $$(find infer/lib/python/inferlib/* -type f); do \ - $(INSTALL_DATA) -C $$i $(DESTDIR)$(libdir)/infer/$$i; \ - done + $(INSTALL_DATA) -C 'infer/annotations/annotations.jar' \ + '$(DESTDIR)$(libdir)/infer/infer/annotations/annotations.jar' + find infer/lib/java/*.jar -print0 | xargs -0 -I \{\} \ + $(INSTALL_DATA) -C \{\} '$(DESTDIR)$(libdir)'/infer/\{\} + $(INSTALL_PROGRAM) -C '$(LIB_DIR)'/wrappers/javac \ + '$(DESTDIR)$(libdir)'/infer/infer/lib/wrappers/ +endif + find infer/lib/python/inferlib/* -type f -print0 | xargs -0 -I \{\} \ + $(INSTALL_DATA) -C \{\} '$(DESTDIR)$(libdir)'/infer/\{\} $(INSTALL_PROGRAM) -C infer/lib/python/infer.py \ - $(DESTDIR)$(libdir)/infer/infer/lib/python/infer.py + '$(DESTDIR)$(libdir)'/infer/infer/lib/python/infer.py $(INSTALL_PROGRAM) -C infer/lib/python/inferTraceBugs \ - $(DESTDIR)$(libdir)/infer/infer/lib/python/inferTraceBugs + '$(DESTDIR)$(libdir)'/infer/infer/lib/python/inferTraceBugs $(INSTALL_PROGRAM) -C infer/lib/python/report.py \ - $(DESTDIR)$(libdir)/infer/infer/lib/python/report.py - $(INSTALL_PROGRAM) -C $(INFER_BIN) $(DESTDIR)$(libdir)/infer/infer/bin/ - (cd $(DESTDIR)$(bindir)/ && \ + '$(DESTDIR)$(libdir)'/infer/infer/lib/python/report.py + $(INSTALL_PROGRAM) -C '$(INFER_BIN)' '$(DESTDIR)$(libdir)'/infer/infer/bin/ + (cd '$(DESTDIR)$(bindir)/' && \ $(REMOVE) infer && \ - $(LN_S) $(libdir_relative_to_bindir)/infer/infer/bin/infer infer) + $(LN_S) '$(libdir_relative_to_bindir)'/infer/infer/bin/infer infer) for alias in $(INFER_COMMANDS); do \ - (cd $(DESTDIR)$(bindir)/ && \ - $(REMOVE) $$alias && \ - $(LN_S) infer $$alias); done + (cd '$(DESTDIR)$(bindir)'/ && \ + $(REMOVE) "$$alias" && \ + $(LN_S) infer "$$alias"); done for alias in $(INFER_COMMANDS); do \ - (cd $(DESTDIR)$(libdir)/infer/infer/bin && \ - $(REMOVE) $$alias && \ - $(LN_S) infer $$alias); done - $(QUIET)for i in $(MAN_DIR)/man1/*; do \ - $(INSTALL_DATA) -C $$i $(DESTDIR)$(mandir)/man1/$$(basename $$i); \ - done - + (cd '$(DESTDIR)$(libdir)'/infer/infer/bin && \ + $(REMOVE) "$$alias" && \ + $(LN_S) infer "$$alias"); done + find '$(MAN_DIR)'/man1 -print0 | xargs -0 \ + $(SHELL) -c '$(INSTALL_DATA) -C $$1 "$(DESTDIR)$(mandir)/man1/$$(basename $$1)"' ifeq ($(IS_FACEBOOK_TREE),yes) - $(QUIET)$(MAKE) -C facebook install + $(MAKE) -C facebook install endif # Nuke objects built from OCaml. Useful when changing the OCaml compiler, for instance. diff --git a/Makefile.config b/Makefile.config index 040c72e90..cd5d1e351 100644 --- a/Makefile.config +++ b/Makefile.config @@ -6,7 +6,7 @@ # of patent rights can be found in the PATENTS file in the same directory. ORIG_SHELL = $(shell echo "$$SHELL") -SHELL = bash +SHELL = bash -e -o pipefail -u ORIG_SHELL_PATH = $(shell printf "%s" "$$PATH") @@ -153,10 +153,10 @@ else # Detect if we are already wrapped inside a silent_on_success call and try not to clutter the output # too much in that case. define silent_on_success - if [ "$$INSIDE_SILENT_ON_SUCCESS" = 1 ]; then \ - echo "*** inner $(1)"; \ - echo "*** inner command: $(2)"; \ - echo "*** inner CWD: $(CURDIR)"; \ + if [ -n "$${INSIDE_SILENT_ON_SUCCESS-}" ]; then \ + echo '*** inner $(1)'; \ + echo '*** inner command: $(2)'; \ + echo '*** inner CWD: $(CURDIR)'; \ ($(2)); \ exit $$?; \ fi; \ @@ -165,7 +165,7 @@ define silent_on_success UNIX_START_DATE=$$(date +"%s"); \ HUMAN_START_DATE=$$(date +"%H:%M:%S"); \ if [ -z $(SILENT) ]; then \ - printf "[%s][%$(MAX_PID_SIZE)s] $(TERM_INFO)%s...$(TERM_RESET)\n" \ + printf '[%s][%$(MAX_PID_SIZE)s] $(TERM_INFO)%s...$(TERM_RESET)\n' \ "$$HUMAN_START_DATE" "$$HASH" "$(1)"; \ fi; \ $(MKDIR_P) $(ABSOLUTE_ROOT_DIR)/_build_logs; \ @@ -173,9 +173,9 @@ define silent_on_success 2>$(ABSOLUTE_ROOT_DIR)/_build_logs/cmd-$$HASH.err; \ ERRCODE=$$?; \ if [ $$ERRCODE != 0 ]; then \ - echo "$(TERM_ERROR)[*ERROR**][$$HASH] *** ERROR $(1)$(TERM_RESET)" >&2; \ - echo "$(TERM_ERROR)[*ERROR**][$$HASH] *** command: $(2)$(TERM_RESET)" >&2; \ - echo "$(TERM_ERROR)[*ERROR**][$$HASH] *** CWD: $(CURDIR)$(TERM_RESET)" >&2; \ + echo "$(TERM_ERROR)[*ERROR**][$$HASH] *** ERROR '$(1)'$(TERM_RESET)" >&2; \ + echo "$(TERM_ERROR)[*ERROR**][$$HASH] *** command: '$(2)'$(TERM_RESET)" >&2; \ + echo "$(TERM_ERROR)[*ERROR**][$$HASH] *** CWD: '$(CURDIR)'$(TERM_RESET)" >&2; \ echo "$(TERM_ERROR)[*ERROR**][$$HASH] *** stdout:$(TERM_RESET)" >&2; \ sed -e "s/^\(.*\)$$/$(TERM_ERROR)[*ERROR**][$$HASH]$(TERM_RESET) \1/" \ $(ABSOLUTE_ROOT_DIR)/_build_logs/cmd-$$HASH.out; >&2; \ diff --git a/configure.ac b/configure.ac index 2a1cf2c50..f8938c2d6 100644 --- a/configure.ac +++ b/configure.ac @@ -177,7 +177,6 @@ pre-compiled here: fi # end if($enable_c_analyzers) - # OCaml dependencies AC_PROG_OCAML AC_ASSERT_PROG([ocamlc], [$OCAMLC]) diff --git a/infer/tests/base.make b/infer/tests/base.make index 0a02d23ee..84c5bb1c5 100644 --- a/infer/tests/base.make +++ b/infer/tests/base.make @@ -14,10 +14,11 @@ include $(ROOT_DIR)/Makefile.config TEST_REL_DIR = $(patsubst $(abspath $(TESTS_DIR))/%,%,$(abspath $(CURDIR))) define check_no_duplicates - grep "DUPLICATE_SYMBOLS" $(1); test $$? -ne 0 || \ - (echo '$(TEST_ERROR)Duplicate symbols found in $(CURDIR).' \ - 'Please make sure all the function names in all the source test files are different.$(TEST_RESET)';\ - exit 1) + if grep -q "DUPLICATE_SYMBOLS" $(1); then \ + printf '$(TERM_ERROR)Duplicate symbols found in $(CURDIR).$(TERM_RESET)\n' >&2; \ + printf '$(TERM_ERROR)Please make sure all the function names in all the source test files are different.$(TERM_RESET)' >&2; \ + exit 1; \ + fi endef define check_no_diff diff --git a/infer/tests/build_systems/fail_on_issue/Makefile b/infer/tests/build_systems/fail_on_issue/Makefile index dbdb59203..bfd45af0d 100644 --- a/infer/tests/build_systems/fail_on_issue/Makefile +++ b/infer/tests/build_systems/fail_on_issue/Makefile @@ -24,8 +24,8 @@ default: compile issues.exp.test: $(CLANG_DEPS) $(SOURCES) $(QUIET)$(call silent_on_success,Testing Infer fails on issue,\ - ($(INFER_BIN) --fail-on-issue -- clang $(CLANG_OPTIONS) $(SOURCES); \ - echo "infer exit code: $$?" > $@)) + exit_code=0; $(INFER_BIN) --fail-on-issue -- clang $(CLANG_OPTIONS) $(SOURCES) || exit_code=$$?; \ + echo "infer exit code: $$exit_code" > $@) .PHONY: compile compile: $(OBJECTS) diff --git a/infer/tests/clang-frontend.make b/infer/tests/clang-frontend.make index 6f9831340..6cb45750d 100644 --- a/infer/tests/clang-frontend.make +++ b/infer/tests/clang-frontend.make @@ -29,10 +29,10 @@ print: capture .PHONY: test test: capture - $(QUIET)for file in $(SOURCES) ; do \ - diff -u $$file.dot $$file.test.dot || error=1 ; \ + $(QUIET)error=0; for file in $(SOURCES) ; do \ + diff -u "$$file.dot" "$$file.test.dot" || error=1 ; \ done ; \ - if [ 0$$error -eq 1 ]; then exit 1; fi + if [ $$error = 1 ]; then exit 1; fi .PHONY: replace replace: capture