From beca699bc49b0e0f9a502f669f0d0b75076af220 Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Wed, 19 Aug 2020 04:29:29 -0700 Subject: [PATCH] [build] swap the build defaults to dev-with-errors Summary: Before: 3 modes: (where "lenient" build has warnings not crash the build, while "strict" build errors on warning): - opt (default): flambda optimisations + lenient build - dev (recommand for dev): lenient build, no flambda - test: strict build (used in tests), no flambda Now: - dev (default): *strict* build, no flambda - opt: lenient build, flambda - dev-noerror: lenient build, no flambda (use when you want to test infer but there are build warnings) The goal is to give faster feedback to infer developers and reduce the amount of times diffs are sent with build warnings. Also it's now faster to alternate between changing infer and running unit tests since test mode is just dev mode. Reviewed By: ngorogiannis Differential Revision: D23167416 fbshipit-source-id: d663b6054 --- CONTRIBUTING.md | 15 ++++++------ Makefile | 38 +++++++---------------------- Makefile.config | 7 ++++++ build-infer.sh | 2 +- infer/src/Makefile | 32 ++++++++++++------------- infer/src/deadcode/Makefile | 48 ++++++++++++++++--------------------- infer/src/dune.in | 4 ++-- 7 files changed, 63 insertions(+), 83 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 393c82f30..0f6740cda 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -20,13 +20,16 @@ make devsetup ### Building Infer for Development -- Build the code faster: `make -j BUILD_MODE=dev`. By default `make` builds infer with flambda - enabled, which makes it very slow (but makes infer significantly faster). +- The default build mode ("dev") makes all build warnings *fatal*. If you want the build to ignore + warnings, for example to be able to test an infer executable before polishing the code to remove + warnings, you can build in "dev-noerror" mode with `make BUILD_MODE=dev-noerror`. - Faster edit/build cycle when working on OCaml code inside infer/src/: build inside infer/src/ - (skips building the models after infer has been built), and build bytecode instead of native: - `make -j -C infer/src byte`. You need to have run `make -j` (with or without `BUILD_MODE=dev`) - at some point before. + (skips building the models after infer has been built), and build only what is needed for type + checking with `make -j -C infer/src check`. You need to have run `make -j` at some point before. + +- Alternatively, if you want to test your changes on a small example, build in bytecode mode: + `make -j -C infer/src byte`. - In general, `make` commands from the root of the repository make sure that dependencies are in a consistent and up-to-date state (e.g., they rebuild infer and the models before running steps that @@ -37,8 +40,6 @@ make devsetup necessary before running the test, but running `make -C infer/tests/codetoanalyze/java/biabduction/ test` will just execute the test. -- To switch the default build mode to flambda disabled, you can `export BUILD_MODE=dev` in your - shell. ### Debugging OCaml Code diff --git a/Makefile b/Makefile index dabb76128..df1ef96f1 100644 --- a/Makefile +++ b/Makefile @@ -9,10 +9,6 @@ default: infer ROOT_DIR = . include $(ROOT_DIR)/Makefile.config -ORIG_SHELL_BUILD_MODE = $(BUILD_MODE) -# override this for faster builds (but slower infer) -BUILD_MODE ?= opt - MAKE_SOURCE = $(MAKE) -C $(SRC_DIR) ifneq ($(UTOP),no) @@ -230,7 +226,7 @@ SRC_ML:=$(shell find * \( -name _build -or -name facebook-clang-plugins -or -pat fmt_all: parallel $(OCAMLFORMAT_EXE) $(OCAMLFORMAT_ARGS) -i ::: $(SRC_ML) $(DUNE_ML) -# pre-building these avoids race conditions when building, eg src_build and test_build in parallel +# pre-building these avoids race conditions when doing multiple builds in parallel .PHONY: src_build_common src_build_common: $(QUIET)$(call silent_on_success,Generating source dependencies,\ @@ -251,14 +247,11 @@ check: src_build_common $(QUIET)$(call silent_on_success,Building artifacts for tooling support,\ $(MAKE_SOURCE) check) -.PHONY: test_build -test_build: src_build_common - $(QUIET)$(call silent_on_success,Testing Infer builds without warnings,\ - $(MAKE_SOURCE) test) - # deadcode analysis: only do the deadcode detection on Facebook builds and if GNU sed is available .PHONY: real_deadcode real_deadcode: src_build_common + $(QUIET)$(call silent_on_success,Building all OCaml code,\ + $(MAKE_SOURCE) build_all) $(QUIET)$(call silent_on_success,Testing there is no dead OCaml code,\ $(MAKE) -C $(SRC_DIR)/deadcode) @@ -293,11 +286,11 @@ toplevel_test: $(MAKE_SOURCE) toplevel) ifeq ($(IS_FACEBOOK_TREE),yes) -byte src_build_common src_build test_build: fb-setup +byte src_build_common src_build: fb-setup endif ifeq ($(BUILD_C_ANALYZERS),yes) -byte src_build src_build_common test_build: clang_plugin +byte src_build src_build_common: clang_plugin endif ifneq ($(NINJA),no) @@ -439,10 +432,9 @@ clang_plugin_test_replace: clang_setup ) .PHONY: ocaml_unit_test -ocaml_unit_test: test_build - $(QUIET)$(REMOVE_DIR) infer-out-unit-tests +ocaml_unit_test: src_build_common $(QUIET)$(call silent_on_success,Running OCaml unit tests,\ - INFER_ARGS=--results-dir^infer-out-unit-tests $(INFERUNIT_BIN)) + $(MAKE_SOURCE) unit) define silence_make $(1) 2> >(grep -v 'warning: \(ignoring old\|overriding\) \(commands\|recipe\) for target') @@ -583,11 +575,8 @@ mod_dep: src_build_common $(QUIET)$(call silent_on_success,Building Infer source dependency graph,\ $(MAKE) -C $(SRC_DIR) mod_dep.dot) -# `test_build` and `src_build` (which is a dependency of `endtoend_test`) should not be run in -# parallel since they build infer with different profiles (and therefore conflict). Therefore, -# `test_build` is in the dependency, and `endtoend_test` in the recipe. .PHONY: config_tests -config_tests: test_build ocaml_unit_test validate-skel mod_dep +config_tests: ocaml_unit_test validate-skel mod_dep $(MAKE) endtoend_test checkCopyright $(MAKE) manuals @@ -886,17 +875,6 @@ devsetup: Makefile.autoconf printf "$(TERM_INFO) echo 'export MANPATH=\"%s/infer/man\":\$$MANPATH' >> \"$$shell_config_file\"$(TERM_RESET)\n" "$(ABSOLUTE_ROOT_DIR)" >&2; \ fi; \ fi; \ - if [ -z "$(ORIG_SHELL_BUILD_MODE)" ]; then \ - echo >&2; \ - echo '$(TERM_INFO)*** NOTE: Set `BUILD_MODE=dev` in your shell to disable flambda by default.$(TERM_RESET)' >&2; \ - echo '$(TERM_INFO)*** NOTE: Compiling with flambda is ~5 times slower than without, so unless you are$(TERM_RESET)' >&2; \ - echo '$(TERM_INFO)*** NOTE: testing infer on a very large project it will not be worth it. Use the$(TERM_RESET)' >&2; \ - echo '$(TERM_INFO)*** NOTE: commands below to set the default build mode. You can then use `make opt`$(TERM_RESET)' >&2; \ - echo '$(TERM_INFO)*** NOTE: when you really do want to enable flambda.$(TERM_RESET)' >&2; \ - echo >&2; \ - printf "$(TERM_INFO) export BUILD_MODE=dev$(TERM_RESET)\n" >&2; \ - printf "$(TERM_INFO) echo 'export BUILD_MODE=dev' >> \"$$shell_config_file\"$(TERM_RESET)\n" >&2; \ - fi $(QUIET)PATH='$(ORIG_SHELL_PATH)'; if [ "$$(ocamlc -where 2>/dev/null)" != "$$($(OCAMLC) -where)" ]; then \ echo >&2; \ echo '$(TERM_INFO)*** NOTE: The current shell is not set up for the right opam switch.$(TERM_RESET)' >&2; \ diff --git a/Makefile.config b/Makefile.config index caed07b73..9924622ae 100644 --- a/Makefile.config +++ b/Makefile.config @@ -14,6 +14,13 @@ export INFER_STRICT_MODE=1 include $(ROOT_DIR)/Makefile.autoconf +# possible values: +# - dev: flambda optimizations disabled, build warnings are errors +# - dev-noerror: flambda optimizations disabled, build warnings do not fail the build +# - opt: flambda optimizations enabled, build warnings do not fail the build +BUILD_MODE = dev +DUNE_BUILD = dune build --profile=$(BUILD_MODE) + PLATFORM = $(shell uname) COPY = cp -f diff --git a/build-infer.sh b/build-infer.sh index 8250a4d00..117cb9c71 100755 --- a/build-infer.sh +++ b/build-infer.sh @@ -199,7 +199,7 @@ if [ "$BUILD_CLANG" == "yes" ] && ! facebook-clang-plugins/clang/setup.sh --only fi fi -make -j "$JOBS" || ( +make -j "$JOBS" opt || ( echo >&2 echo ' compilation failure; you can try running' >&2 echo >&2 diff --git a/infer/src/Makefile b/infer/src/Makefile index e9682d877..28819a326 100644 --- a/infer/src/Makefile +++ b/infer/src/Makefile @@ -8,13 +8,6 @@ include $(ROOT_DIR)/Makefile.config #### Global declarations #### -# Override this for faster builds (but slower infer) -BUILD_MODE ?= opt - -DUNE_BUILD = dune build --profile=$(BUILD_MODE) - -ETC_DIR = $(INFER_DIR)/etc - INFER_MAIN = infer INFERTOP_MAIN = infertop INFERUNIT_MAIN = inferunit @@ -22,8 +15,6 @@ INFER_CREATE_TRACEVIEW_LINKS_MODULE = InferCreateTraceViewLinks INFER_CREATE_TRACEVIEW_LINKS_MAIN = $(INFER_CREATE_TRACEVIEW_LINKS_MODULE) CHECKCOPYRIGHT_MAIN = checkCopyright -FACEBOOK_DIR = facebook - #### Clang declarations #### CLANG_PLUGIN_MIRROR = atd @@ -88,18 +79,27 @@ src_build_common: $(SRC_BUILD_COMMON) # NOTE: the complexity below is because @runtest and @fmt interact poorly: we want dune to format # test files *after* having promoted them when the test outputs change -.PHONY: test -test: BUILD_MODE = test -test: $(SRC_BUILD_COMMON) $(MAKEFILE_LIST) +.PHONY: runtest +runtest: $(SRC_BUILD_COMMON) $(MAKEFILE_LIST) $(QUIET)exit_code=0; \ - $(DUNE_BUILD) \ - $(patsubst %.exe, %.bc.exe, $(INFER_CONFIG_TARGETS)) \ - scripts/$(CHECKCOPYRIGHT_MAIN).bc.exe $(INFERUNIT_MAIN).bc.exe $(INFERTOP_MAIN).bc.exe \ - || exit_code=$$?; \ $(DUNE_BUILD) @runtest --auto-promote || exit_code=$$?; \ $(DUNE_BUILD) @fmt --auto-promote || exit_code=$$?; \ exit $$exit_code +.PHONY: test +test: $(SRC_BUILD_COMMON) $(MAKEFILE_LIST) runtest + $(DUNE_BUILD) \ + $(patsubst %.exe, %.bc.exe, $(INFER_CONFIG_TARGETS)) \ + scripts/$(CHECKCOPYRIGHT_MAIN).bc.exe $(INFERUNIT_MAIN).bc.exe $(INFERTOP_MAIN).bc.exe + +.PHONY: unit +unit: runtest test + $(QUIET)$(REMOVE_DIR) infer-out-unit-tests + $(QUIET)INFER_ARGS=--results-dir^infer-out-unit-tests $(INFERUNIT_BIN) + +.PHONY: build_all +build_all: $(SRC_BUILD_COMMON) + $(DUNE_BUILD) @all .PHONY: doc doc: $(SRC_BUILD_COMMON) $(MAKEFILE_LIST) diff --git a/infer/src/deadcode/Makefile b/infer/src/deadcode/Makefile index ba0a95667..9fd369a73 100644 --- a/infer/src/deadcode/Makefile +++ b/infer/src/deadcode/Makefile @@ -41,9 +41,9 @@ default: detect_dead_code # Annoying find being different on OSX/BSD and GNU/Linux ifeq ($(PLATFORM),Linux) -DEPTH_ONE=-mindepth 1 -maxdepth 1 +DEPTH_ONE = -mindepth 1 -maxdepth 1 else -DEPTH_ONE=-depth 1 +DEPTH_ONE = -depth 1 endif # ./ is necessary for find to work correctly. @@ -53,29 +53,23 @@ endif LIBRARY_FOLDERS = . ./IR ./absint ./al ./atd ./backend ./base ./biabduction ./bufferoverrun ./c_stubs ./checkers ./clang ./clang/unit ./concurrency ./cost ./integration ./istd ./java ./labs ./nullsafe ./nullsafe/unit ./pulse ./quandary ./scripts ./test_determinator ./topl ./unit INCLUDE_FOLDERS = -I IR -I absint -I al -I atd -I backend -I base -I biabduction -I bufferoverrun -I c_stubs -I checkers -I clang -I clang/unit -I concurrency -I cost -I integration -I istd -I java -I labs -I nullsafe -I nullsafe/unit -I pulse -I quandary -I scripts -I test_determinator -I topl -I unit -ml_src_files:=$(shell \ - cd $(INFER_BUILD_DIR); \ - for d in $(LIBRARY_FOLDERS); do \ - [ -d $$d ] && echo $$(find $$d $(DEPTH_ONE) -regex '\./[a-zA-Z].*\.ml' \( -not -regex '.*\.pp\.ml' \) \ - | sed 's/^\.\///'); \ - done) -mli_src_files:=$(shell \ - cd $(INFER_BUILD_DIR); \ - for d in $(LIBRARY_FOLDERS); do \ - [ -d $$d ] && echo $$(find $$d $(DEPTH_ONE) -regex '\./[a-zA-Z].*\.mli' \( -not -regex '.*\.pp\.mli' \) \ - | sed 's/^\.\///'); \ - done) -ml_src_files_without_mli:=$(shell \ - cd $(INFER_BUILD_DIR); \ - for i in $(ml_src_files); do [ -f $${i}i ] || echo $$i; done) +ml_src_files = $(shell \ + cd $(INFER_BUILD_DIR); \ + for d in $(LIBRARY_FOLDERS); do \ + [ -d $$d ] && echo $$(find $$d $(DEPTH_ONE) -regex '\./[a-zA-Z].*\.ml' \( -not -regex '.*\.pp\.ml' \) \ + | sed 's/^\.\///'); \ + done) + +mli_src_files = $(shell \ + cd $(INFER_BUILD_DIR); \ + for d in $(LIBRARY_FOLDERS); do \ + [ -d $$d ] && echo $$(find $$d $(DEPTH_ONE) -regex '\./[a-zA-Z].*\.mli' \( -not -regex '.*\.pp\.mli' \) \ + | sed 's/^\.\///'); \ + done) -.PHONY: dump_ml dump_mli dump_ml_only -dump_ml: - @echo $(ml_src_files) -dump_mli: - @echo $(mli_src_files) -dump_ml_only: - @echo $(ml_src_files_without_mli) +ml_src_files_without_mli:=$(shell \ + cd $(INFER_BUILD_DIR); \ + for i in $(ml_src_files); do [ -f $${i}i ] || echo $$i; done) .PHONY: depend depend: @@ -174,7 +168,7 @@ detect_dead_code: dune # dune file for this directory touch $(ALL_INFER_IN_ONE_FILE_ML) $(ALL_INFER_IN_ONE_FILE_ML:.ml=.mli) # needed to get the generated code for the lexers and parsers in ../_build - $(MAKE) -C .. test + $(MAKE) -C .. $(MAKE) depend # Need to be sequential to avoid getting a garbled file. Need to re-include .depend as it may # have changed. For both of these reasons, run another `make`. @@ -185,10 +179,10 @@ detect_dead_code: dune mv "$$tmp_file" $(ALL_INFER_IN_ONE_FILE_ML); \ sort -u "$$tmp_file_copied" > $(ALL_ML_FILES_COPIED); \ rm "$$tmp_file_copied" - $(MAKE) -j 1 detect_dead_src_file # build and get dead code warnings - dune build --profile test all_infer_in_one_file.bc + $(DUNE_BUILD) all_infer_in_one_file.bc $(REMOVE) dune + $(MAKE) -j 1 detect_dead_src_file .PHONY: detect_dead_src_file detect_dead_src_file: diff --git a/infer/src/dune.in b/infer/src/dune.in index e8bc3e9f9..c4b9477f6 100644 --- a/infer/src/dune.in +++ b/infer/src/dune.in @@ -46,12 +46,12 @@ let env_stanza = (opt (flags %s) (ocamlopt_flags (:standard -O3))) - (test + (dev-noerror (flags %s) (inline_tests enabled)) ) |} - lenient_flags lenient_flags strict_flags + strict_flags lenient_flags lenient_flags (** Put this *first* in (libraries) specifications to prevent a clash between extlib's and base64's