[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
4 years ago · beca699bc4
parent dc29f8eeca
commit beca699bc4
7 changed files with 63 additions and 83 deletions
--- 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

--- a/38
+++ b/38
@ -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; \
--- 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
--- 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
--- 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)
--- 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:
--- 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