From b1b7cc0b9ddd24e3aa9d4220495d36da1a42c591 Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Wed, 23 Aug 2017 07:23:25 -0700 Subject: [PATCH] [make] better way of limiting concurrency and better jbuilder concurrency Summary: This gets rid of the horrible hack of asking jbuilder to build _build/{default,test}/.ppx/ppx_compare/ppx.exe prior to starting several jbuilds in parallel. We now refrain from starting several jbuilds in parallel. Also, I finally understood that order-only deps do nothing for phony targets, and not much else for non-phony ones. I hate make even more. Reviewed By: jberdine Differential Revision: D5678699 fbshipit-source-id: 52179e8 --- .gitignore | 1 + Makefile | 36 ++++++++++++++++++------------------ infer/src/Makefile | 38 +++++++++----------------------------- 3 files changed, 28 insertions(+), 47 deletions(-) diff --git a/.gitignore b/.gitignore index eed23f7ee..dd604e85a 100644 --- a/.gitignore +++ b/.gitignore @@ -42,6 +42,7 @@ duplicates.txt /infer/tests/build_systems/differential_*/infer-previous /infer/tests/build_systems/diff/src /infer/tests/build_systems/diff_*/src +/infer/tests/build_systems/buck_flavors_deterministic/capture_hash-*.sha /_release # generated by oUnit diff --git a/Makefile b/Makefile index 9fa4040c8..b1c9de677 100644 --- a/Makefile +++ b/Makefile @@ -13,7 +13,7 @@ include $(ROOT_DIR)/Makefile.config ifneq ($(UTOP),no) BUILD_SYSTEMS_TESTS += infertop -build_infertop_print build_infertop_replace build_infertop_test: toplevel_test +build_infertop_print build_infertop_replace build_infertop_test: test_build endif ifeq ($(BUILD_C_ANALYZERS),yes) @@ -85,9 +85,13 @@ BUILD_SYSTEMS_TESTS += ant endif ifneq ($(BUCK),no) BUILD_SYSTEMS_TESTS += buck genrule -# do not run these two tests in parallel otherwise Buck has a bad time -build_genrule_test: | build_buck_test -build_genrule_print: | build_buck_print +# Introduce the dependency only if the two tests are going to be built in parallel, so that they do +# not run in parallel (otherwise Buck has a bad time). This works by checking if one of the main +# testing targets was passed as a goal on the command line. +ifneq ($(filter build_systems_tests config_tests test,${MAKECMDGOALS}),) +build_genrule_test: build_buck_test +build_genrule_print: build_buck_print +endif endif ifneq ($(MVN),no) BUILD_SYSTEMS_TESTS += mvn @@ -96,10 +100,13 @@ endif ifeq ($(BUILD_C_ANALYZERS)+$(BUILD_JAVA_ANALYZERS),yes+yes) BUILD_SYSTEMS_TESTS += make utf8_in_pwd waf -# the waf test and the make test run the same `make` command +# the waf test and the make test run the same `make` command; use the same trick as for +# "build_buck_test" to prevent make from running them in parallel +ifneq ($(filter build_systems_tests config_tests test,${MAKECMDGOALS}),) build_waf_test: build_make_test build_waf_print: build_make_print endif +endif ifeq ($(IS_INFER_RELEASE),no) configure: configure.ac $(wildcard m4/*.m4) @@ -153,7 +160,7 @@ byte: src_build_common .PHONY: test_build test_build: src_build_common $(QUIET)$(call silent_on_success,Testing Infer builds without warnings,\ - $(MAKE) -C $(SRC_DIR) TEST=1 byte_no_install) + $(MAKE) -C $(SRC_DIR) test) ifeq ($(IS_FACEBOOK_TREE),yes) byte src_build_common src_build test_build: fb-setup @@ -296,18 +303,6 @@ check_missing_mli: $(QUIET)for x in $$(find $(INFER_DIR)/src -name "*.ml"); do \ test -f "$$x"i || echo Missing "$$x"i; done -# depend on test_build because jbuilder doesn't like running concurrently with itself -.PHONY: toplevel_test -toplevel_test: src_build_common | test_build -# build with TEST=1 as the infer_repl scripts expects to find the toplevel in the test build - $(QUIET)$(call silent_on_success,Building infer toplevel (test mode),\ - $(MAKE) -C $(SRC_DIR) TEST=1 toplevel) - -.PHONY: toplevel -toplevel: src_build_common - $(QUIET)$(call silent_on_success,Building infer toplevel,\ - $(MAKE) -C $(SRC_DIR) toplevel) - .PHONY: checkCopyright checkCopyright: src_build_common $(QUIET)$(call silent_on_success,Building checkCopyright,\ @@ -351,6 +346,11 @@ mod_dep: src_build_common .PHONY: config_tests config_tests: test_build ocaml_unit_test endtoend_test checkCopyright validate-skel mod_dep +ifneq ($(filter config_tests test,${MAKECMDGOALS}),) +test_build: src_build +checkCopyright: src_build test_build +endif + .PHONY: test test: crash_if_not_all_analyzers_enabled config_tests ifeq (,$(findstring s,$(MAKEFLAGS))) diff --git a/infer/src/Makefile b/infer/src/Makefile index 9bce27046..da37d0d18 100644 --- a/infer/src/Makefile +++ b/infer/src/Makefile @@ -12,13 +12,7 @@ include $(ROOT_DIR)/Makefile.config ETC_DIR = $(INFER_DIR)/etc # paths to BUILD_DIR are relative because that's how jbuilder likes it -JBUILDER_BUILD_DEFAULT = _build/default -JBUILDER_BUILD_TEST = _build/test -ifeq ($(TEST),1) -INFER_BUILD_DIR = $(JBUILDER_BUILD_TEST) -else -INFER_BUILD_DIR = $(JBUILDER_BUILD_DEFAULT) -endif +INFER_BUILD_DIR = _build/default ATDGEN_SUFFIXES = _t.ml _t.mli _j.ml _j.mli @@ -98,10 +92,6 @@ ifeq ($(IS_FACEBOOK_TREE),yes) INFER_CONFIG_TARGETS += $(INFER_BUILD_DIR)/$(INFER_CREATE_TRACEVIEW_LINKS_MAIN).exe endif -ifeq ($(TEST),1) -INFER_CONFIG_TARGETS += $(INFER_BUILD_DIR)/$(INFERUNIT_MAIN).exe -endif - OCAML_GENERATED_SOURCES = \ base/Version.ml $(STACKTREE_ATDGEN_STUBS) $(INFERPRINT_ATDGEN_STUBS) @@ -116,17 +106,7 @@ OCAML_SOURCES = \ .PHONY: all all: infer -$(INFER_BUILD_DIR)/.ppx/ppx_compare/ppx.exe: \ - jbuild atd/jbuild istd/jbuild scripts/jbuild jbuild-workspace \ - $(OCAML_GENERATED_SOURCES) $(MAKEFILE_LIST) -# some voodoo to make jbuilder tolerate being run in parallel: force jbuilder to build its -# jbuild files and some files that have been seen to race otherwise - jbuilder build \ - $(JBUILDER_BUILD_DEFAULT)/.ppx/ppx_compare/ppx.exe \ - $(JBUILDER_BUILD_TEST)/.ppx/ppx_compare/ppx.exe - touch $@ - -SRC_BUILD_COMMON = $(INFER_BUILD_DIR)/.ppx/ppx_compare/ppx.exe $(OCAML_SOURCES) +SRC_BUILD_COMMON = jbuild atd/jbuild istd/jbuild scripts/jbuild jbuild-workspace $(OCAML_SOURCES) ifeq ($(BUILD_C_ANALYZERS),yes) SRC_BUILD_COMMON += $(CLANG_BINIOU_DICT) endif @@ -137,10 +117,16 @@ src_build_common: $(SRC_BUILD_COMMON) # single out infer.exe as the source of truth for make, knowing that in fact several targets are # produced by the build $(INFER_BUILD_DIR)/$(INFER_MAIN).exe: $(SRC_BUILD_COMMON) $(MAKEFILE_LIST) - jbuilder build $(INFER_CONFIG_TARGETS) + $(QUIET)jbuilder build $(INFER_CONFIG_TARGETS) # let make know that the target is up-to-date even if ocamlbuild cached it $(QUIET)touch $@ +.PHONY: test +test: $(SRC_BUILD_COMMON) $(MAKEFILE_LIST) + $(QUIET)jbuilder build \ + $(patsubst _build/default/%.exe,_build/test/%.bc,$(INFER_CONFIG_TARGETS)) \ + _build/test/scripts/checkCopyright.bc _build/test/$(INFERUNIT_MAIN).bc _build/test/infertop.bc + INFER_BIN_ALIASES = $(foreach alias,$(INFER_COMMANDS),$(BIN_DIR)/$(alias)) $(INFER_BIN_ALIASES): Makefile @@ -168,9 +154,6 @@ $(INFER_BUILD_DIR)/$(INFER_MAIN).bc: $(SRC_BUILD_COMMON) $(MAKEFILE_LIST) $(INFER_BIN).bc: $(INFER_BUILD_DIR)/$(INFER_MAIN).bc $(INFER_BIN_ALIASES) $(INSTALL_PROGRAM) $(INFER_BUILD_DIR)/$(INFER_MAIN).bc $(INFER_BIN) $(INSTALL_PROGRAM) $(INFER_BUILD_DIR)/$(INFER_MAIN).bc $(INFER_BIN).bc -ifeq ($(TEST),1) - $(INSTALL_PROGRAM) $(INFER_BUILD_DIR)/$(INFERUNIT_MAIN).bc $(INFERUNIT_BIN) -endif ifeq ($(IS_FACEBOOK_TREE),yes) $(INSTALL_PROGRAM) $(INFER_BUILD_DIR)/$(INFER_CREATE_TRACEVIEW_LINKS_MAIN).bc \ $(INFER_CREATE_TRACEVIEW_LINKS_BIN) @@ -179,9 +162,6 @@ endif .PHONY: byte byte: $(INFER_BIN).bc -.PHONY: byte_no_install -byte_no_install: $(INFER_BUILD_DIR)/$(INFER_MAIN).bc - roots:=Infer ifeq ($(IS_FACEBOOK_TREE),yes) roots += $(INFER_CREATE_TRACEVIEW_LINKS_MODULE)