Skip to content

Commit

Permalink
[RFC][build] Use dune environments and profiles instead of contexts
Browse files Browse the repository at this point in the history
Summary:
With profiles and `(env ...)` stanza it's possible to consolidate
various ocamlc/ocamlopt/etc setups in a single place.

Where previously we needed to append `dune.common` to every dune file
and specify `flags` and `ocamlopt_flags` now the flags are specified
in `env` and applied accross the board.

This allows to
1. simplify build definitions,
2. avoid the need to generate dune files,
3. use plain sexps instead of OCaml and JBuilder plugin in build
files.

(I'll try to address 2 and 3 in the followup patches).

Existing `make` targets should continue working as before. Also, we
can use dune CLI like so:
```
infer/src$ dune printenv --profile opt # <- very useful for introspection
infer/src$ dune build check
infer/src$ dune build check --profile test
infer/src$ dune build infer.exe --profile dev
infer/src$ dune build infer.exe --profile opt
```

Also, with just 1 context something like `dune runtest` will run unit
tests only once instead of N times, where N is the number of contexts.

Now, there's one difference compared to the previous setup with
contexts:
- Previously, each context had its own build folder, and building infer
in opt context didn't invalidate any of the build artifacts in default
context. Therefore, alternating between `make` and `make opt` had low
overhead at the expense of having N copies of all build artifacts (1
for every context).
- Now, there's just 1 build folder and switching between profiles does
invalidate some artifacts (but not all) and rebuild takes a bit more
time.

So, if you're alternating like crazy between profiles your experience
may get worse (but not necessarily, more on that below). If you want
to trigger an opt build occasionally, you shouldn't notice much
difference.

For those who are concerned about slower build times when alternating
between different build profiles, there's a solution: [dune
cache](https://dune.readthedocs.io/en/stable/caching.html).

You can enable it by creating a file `~/.config/dune/config` with the
following contents:
```
(lang dune 2.0)
(cache enabled)
```

With cache enabled switching between different build profiles (but
also branches and commits) has ~0 overhead.

Dune cache works fine on Linux, but unfortunately there are [certain
problems with
MacOS](ocaml/dune#3233) (hopefully, those
will be fixed soon and then there will be no downsides to using
profiles compared to contexts for our case).

Reviewed By: jvillard

Differential Revision: D20247864

fbshipit-source-id: 5f8afa0db
  • Loading branch information
artempyanykh authored and facebook-github-bot committed Mar 11, 2020
1 parent 40e5a3c commit fcce3c0
Show file tree
Hide file tree
Showing 16 changed files with 183 additions and 176 deletions.
6 changes: 3 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ make devsetup

### Building Infer for Development

- Build the code faster: `make -j BUILD_MODE=default`. By default `make` builds infer with flambda
- 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).

- 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=default`)
`make -j -C infer/src byte`. You need to have run `make -j` (with or without `BUILD_MODE=dev`)
at some point before.

- In general, `make` commands from the root of the repository make sure that dependencies are in a
Expand All @@ -37,7 +37,7 @@ make devsetup
before running the test, but running `make -C infer/tests/codetoanalyze/java/infer test` will just
execute the test.

- To switch the default build mode to flambda disabled, you can `export BUILD_MODE=default` in your
- To switch the default build mode to flambda disabled, you can `export BUILD_MODE=dev` in your
shell.

### Debugging OCaml Code
Expand Down
44 changes: 22 additions & 22 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ ORIG_SHELL_BUILD_MODE = $(BUILD_MODE)
# override this for faster builds (but slower infer)
BUILD_MODE ?= opt

MAKE_SOURCE = $(MAKE) -C $(SRC_DIR) INFER_BUILD_DIR=_build/$(BUILD_MODE)
MAKE_SOURCE = $(MAKE) -C $(SRC_DIR)

ifneq ($(UTOP),no)
BUILD_SYSTEMS_TESTS += infertop
Expand Down Expand Up @@ -293,9 +293,9 @@ toplevel:
$(QUIET)echo "You can now use the infer REPL:"
$(QUIET)echo " \"$(ABSOLUTE_ROOT_DIR)/scripts/infer_repl\""

toplevel_test: test_build
$(QUIET)$(call silent_on_success,Building Infer REPL (test mode),\
$(MAKE_SOURCE) BUILD_MODE=test toplevel)
toplevel_test:
$(QUIET)$(call silent_on_success,Building Infer REPL,\
$(MAKE_SOURCE) toplevel)

ifeq ($(IS_FACEBOOK_TREE),yes)
byte src_build_common src_build test_build: fb-setup
Expand Down Expand Up @@ -331,16 +331,14 @@ $(INFER_GROFF_MANUALS_GZIPPED): %.gz: %

infer_models: src_build
ifeq ($(BUILD_JAVA_ANALYZERS),yes)
$(MAKE) -C $(ANNOTATIONS_DIR)
$(QUIET)$(call silent_on_success,Building infer annotations,\
$(MAKE) -C $(ANNOTATIONS_DIR))
endif
$(MAKE) -C $(MODELS_DIR) all
$(QUIET)$(call silent_on_success,Building infer models,\
$(MAKE) -C $(MODELS_DIR) all)

.PHONY: infer byte_infer
infer byte_infer:
$(QUIET)$(call silent_on_success,Building Infer models,\
$(MAKE) infer_models)
$(QUIET)$(call silent_on_success,Building Infer manuals,\
$(MAKE) $(INFER_MANUALS))
infer byte_infer: infer_models
infer: src_build
byte_infer: byte

Expand Down Expand Up @@ -442,8 +440,7 @@ ocaml_unit_test: test_build
$(QUIET)$(REMOVE_DIR) infer-out-unit-tests
$(QUIET)$(call silent_on_success,Running OCaml unit tests,\
INFER_ARGS=--results-dir^infer-out-unit-tests \
BUILD_DIR=$(BUILD_DIR)/test \
$(SCRIPT_DIR)/dune_exec_shim.sh $(BUILD_DIR)/test/inferunit.bc)
$(SCRIPT_DIR)/dune_exec_shim.sh $(BUILD_DIR)/default/inferunit.bc)

define silence_make
$(1) 2> >(grep -v 'warning: \(ignoring old\|overriding\) \(commands\|recipe\) for target')
Expand Down Expand Up @@ -580,11 +577,14 @@ mod_dep: src_build_common
$(MAKE) -C $(SRC_DIR) mod_dep.dot)

.PHONY: config_tests
config_tests: test_build ocaml_unit_test endtoend_test checkCopyright validate-skel mod_dep
config_tests: test_build ocaml_unit_test validate-skel mod_dep
$(MAKE) endtoend_test checkCopyright
$(QUIET)$(call silent_on_success,Building Infer manuals,\
$(MAKE) $(INFER_MANUALS))

ifneq ($(filter config_tests test,${MAKECMDGOALS}),)
test_build: src_build
checkCopyright: src_build test_build
ifneq ($(filter endtoend_test,${MAKECMDGOALS}),)
checkCopyright: src_build
toplevel_test: checkCopyright
endif

.PHONY: test
Expand Down Expand Up @@ -841,14 +841,14 @@ devsetup: Makefile.autoconf
fi; \
if [ -z "$(ORIG_SHELL_BUILD_MODE)" ]; then \
echo >&2; \
echo '$(TERM_INFO)*** NOTE: Set `BUILD_MODE=default` in your shell to disable flambda by default.$(TERM_RESET)' >&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=default$(TERM_RESET)\n" >&2; \
printf "$(TERM_INFO) echo 'export BUILD_MODE=default' >> \"$$shell_config_file\"$(TERM_RESET)\n" >&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; \
Expand All @@ -867,7 +867,7 @@ doc: src_build_common
# do not call the browser if we are publishing the docs
ifeq ($(filter doc-publish,${MAKECMDGOALS}),)
$(QUIET)$(call silent_on_success,Opening in browser,\
browse $(SRC_DIR)/_build/$(BUILD_MODE)/_doc/_html/index.html)
browse $(SRC_DIR)/_build/default/_doc/_html/index.html)
$(QUIET)echo "Tip: you can generate the doc for all the opam dependencies of infer like this:"
$(QUIET)echo
$(QUIET)echo " odig odoc # takes a while, run it only when the dependencies change"
Expand Down Expand Up @@ -895,7 +895,7 @@ endif
ifeq ($(IS_FACEBOOK_TREE),no)
$(QUIET)$(call silent_on_success,Copying OCaml modules documentation,\
version=$$($(INFER_BIN) --version | head -1 | cut -d ' ' -f 3 | cut -c 2-); \
rsync -a --delete $(SRC_DIR)/_build/$(BUILD_MODE)/_doc/_html/ "$(GHPAGES)"/static/odoc/"$$version"; \
rsync -a --delete $(SRC_DIR)/_build/default/_doc/_html/ "$(GHPAGES)"/static/odoc/"$$version"; \
$(REMOVE) "$(GHPAGES)"/static/odoc/latest; \
$(LN_S) "$$version" "$(GHPAGES)"/static/odoc/latest)
else
Expand Down
2 changes: 1 addition & 1 deletion docker/master-java/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ RUN eval $(opam env) && \
ENV PATH /infer-host/infer/bin:/infer/bin:$PATH

# build in non-optimized mode by default to speed up build times
ENV BUILD_MODE=default
ENV BUILD_MODE=dev

# prevent exiting by compulsively hitting Control-D
ENV IGNOREEOF=9
Expand Down
5 changes: 1 addition & 4 deletions infer/src/IR/dune.in
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ Format.sprintf
(library
(name InferIR)
(public_name InferIR)
(flags (%s -open Core -open InferStdlib -open IStd -open InferGenerated -open InferBase))
(ocamlopt_flags (%s))
(flags (:standard -open Core -open InferStdlib -open IStd -open InferGenerated -open InferBase))
(libraries %s)
(preprocess (pps ppx_compare))
)
Expand All @@ -24,7 +23,5 @@ Format.sprintf
(mld_files index)
)
|}
(String.concat " " common_cflags)
(String.concat " " common_optflags)
(String.concat " " ("InferBase" :: common_libraries))
|> Jbuild_plugin.V1.send
48 changes: 31 additions & 17 deletions infer/src/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,15 @@ include $(ROOT_DIR)/Makefile.config

#### Global declarations ####

ETC_DIR = $(INFER_DIR)/etc
# override this for faster builds (but slower infer)
BUILD_MODE ?= opt
BUILD_ROOT = _build
# paths to BUILD_DIR are relative because that's how dune likes it
# can be overriden to specify another build mode (eg opt)
INFER_BUILD_DIR = _build/default
INFER_BUILD_DIR = $(BUILD_ROOT)/default

DUNE_BUILD = dune build --profile=$(BUILD_MODE) --build-dir=$(BUILD_ROOT)

ETC_DIR = $(INFER_DIR)/etc

#### Backend declarations ####

Expand Down Expand Up @@ -103,28 +108,33 @@ endif
.PHONY: src_build_common
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
# Single out infer.exe as the source of truth for make, knowing that in fact several
# targets are produced by the build.
# The target is marked as .PHONY because if you build with one profile `make
# BUILD_MODE=dev` and then try with another profile `make BUILD_MODE=opt`, make won't
# trigger a rebuild since it has only partial view on the dependencies.
.PHONY: $(INFER_BUILD_DIR)/$(INFER_MAIN).exe
$(INFER_BUILD_DIR)/$(INFER_MAIN).exe: $(SRC_BUILD_COMMON) $(MAKEFILE_LIST)
$(QUIET)dune build $(INFER_CONFIG_TARGETS)
$(QUIET)$(DUNE_BUILD) $(INFER_CONFIG_TARGETS)
# let make know that the target is up-to-date even if ocamlbuild cached it
$(QUIET)touch $@

.PHONY: test
test: BUILD_MODE = test
test: $(SRC_BUILD_COMMON) $(MAKEFILE_LIST)
# needed for dune to "install" the relevant dynamic libraries for the C stubs
$(QUIET)cd c_stubs; dune build InferCStubs.install
$(QUIET)dune build \
$(patsubst $(INFER_BUILD_DIR)/%.exe,_build/test/%.bc,$(INFER_CONFIG_TARGETS)) \
_build/test/scripts/checkCopyright.bc _build/test/$(INFERUNIT_MAIN).bc _build/test/infertop.bc
$(QUIET)cd c_stubs; $(DUNE_BUILD) InferCStubs.install
$(QUIET)$(DUNE_BUILD) \
$(patsubst $(INFER_BUILD_DIR)/%.exe,%.bc, $(INFER_CONFIG_TARGETS)) \
scripts/checkCopyright.bc $(INFERUNIT_MAIN).bc infertop.bc

.PHONY: doc
doc: $(SRC_BUILD_COMMON) $(MAKEFILE_LIST)
$(QUIET)dune build @$(INFER_BUILD_DIR)/doc
$(QUIET)$(DUNE_BUILD) @doc

.PHONY: check
check: src_build_common
dune build @check
$(DUNE_BUILD) @check


INFER_BIN_ALIASES = $(foreach alias,$(INFER_COMMANDS),$(BIN_DIR)/$(alias))
Expand Down Expand Up @@ -152,10 +162,12 @@ infer: $(INFER_BIN).exe
$(INSTALL_PROGRAM) -C $(INFER_BIN).exe $(INFER_BIN)
$(MAKE) $(INFER_BIN_ALIASES)

.PHONY: $(INFER_BUILD_DIR)/$(INFER_MAIN).bc
$(INFER_BUILD_DIR)/$(INFER_MAIN).bc: $(SRC_BUILD_COMMON) $(MAKEFILE_LIST)
dune build $(INFER_CONFIG_TARGETS:.exe=.bc)
$(DUNE_BUILD) $(INFER_CONFIG_TARGETS:.exe=.bc)
$(QUIET)touch $@

.PHONY: $(INFER_BIN).bc
$(INFER_BIN).bc: $(INFER_BUILD_DIR)/$(INFER_MAIN).bc
$(QUIET)$(MKDIR_P) $(BIN_DIR)
ifeq ($(WINDOWS_BUILD),yes)
Expand All @@ -168,7 +180,7 @@ ifeq ($(IS_FACEBOOK_TREE),yes)
$(INFER_CREATE_TRACEVIEW_LINKS_BIN)
endif
# needed for dune to "install" the relevant dynamic libraries for the C stubs
cd c_stubs; dune build InferCStubs.install
cd c_stubs; $(DUNE_BUILD) InferCStubs.install

.PHONY: byte
byte: $(INFER_BIN).bc
Expand Down Expand Up @@ -200,10 +212,11 @@ mod_dep.pdf: mod_dep.dot
dsort:
$(QUIET)ocamldep.opt -sort $(inc_flags) $(ml_src_files)

.PHONY: $(INFER_BUILD_DIR)/infertop.bc
$(INFER_BUILD_DIR)/infertop.bc: $(SRC_DIR)/infertop.ml $(SRC_BUILD_COMMON) $(MAKEFILE_LIST)
dune build $@
$(DUNE_BUILD) $@
# needed for dune to "install" the relevant dynamic libraries for the C stubs
cd c_stubs; dune build InferCStubs.install
cd c_stubs; $(DUNE_BUILD) InferCStubs.install
$(QUIET)touch $@

$(INFERTOP_BIN): $(INFER_BUILD_DIR)/infertop.bc
Expand All @@ -216,8 +229,9 @@ toplevel: $(INFERTOP_BIN)
.PHONY: checkCopyright
checkCopyright: $(CHECKCOPYRIGHT_BIN)

.PHONY: $(CHECKCOPYRIGHT_BIN)
$(CHECKCOPYRIGHT_BIN): $(SRC_BUILD_COMMON) $(MAKEFILE_LIST)
dune build $(INFER_BUILD_DIR)/scripts/$(CHECKCOPYRIGHT_MAIN).exe
$(DUNE_BUILD) $(INFER_BUILD_DIR)/scripts/$(CHECKCOPYRIGHT_MAIN).exe
$(INSTALL_PROGRAM) -C $(INFER_BUILD_DIR)/scripts/$(CHECKCOPYRIGHT_MAIN).exe $(CHECKCOPYRIGHT_BIN)

define gen_atdgen_rules
Expand Down
7 changes: 1 addition & 6 deletions infer/src/atd/dune.in
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,13 @@
*)
(* NOTE: prepend dune.common to this file! *)

let cflags = common_cflags @ ["-w"; "-27-32-34-35-39"]

;;
Format.sprintf
{|
(library
(name InferGenerated)
(public_name InferGenerated)
(flags (%s))
(ocamlopt_flags (%s))
(flags (:standard -w -27-32-34-35-39))
(libraries atdgen core)
(preprocess (pps ppx_compare))
)
Expand All @@ -26,6 +23,4 @@ Format.sprintf
(mld_files index)
)
|}
(String.concat " " cflags)
(String.concat " " common_optflags)
|> Jbuild_plugin.V1.send
14 changes: 4 additions & 10 deletions infer/src/base/dune.in
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,12 @@ Format.sprintf
(library
(name InferBase)
(public_name InferBase)
(flags (%s -open Core -open InferStdlib -open IStd -open InferGenerated))
(ocamlopt_flags (%s))
(libraries %s)
(preprocess (pps ppx_compare ppx_enumerate))
)
(flags (:standard -open Core -open InferStdlib -open IStd -open InferGenerated))
(libraries InferStdlib InferGenerated core)
(preprocess (pps ppx_compare ppx_enumerate)))

(documentation
(package InferBase)
(mld_files index)
)
(mld_files index))
|}
(String.concat " " common_cflags)
(String.concat " " common_optflags)
(String.concat " " ("InferStdlib" :: "InferGenerated" :: common_libraries))
|> Jbuild_plugin.V1.send
4 changes: 2 additions & 2 deletions infer/src/deadcode/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
ROOT_DIR = ../../..
include $(ROOT_DIR)/Makefile.config

INFER_BUILD_DIR = ../_build/test
INFER_BUILD_DIR = ../_build/default

ALL_INFER_IN_ONE_FILE_ML = all_infer_in_one_file.ml
ALL_ML_FILES = all_ml_files
Expand Down Expand Up @@ -168,7 +168,7 @@ detect_dead_code:
rm "$$tmp_file_copied"
$(MAKE) -j 1 detect_dead_src_file
# build and get dead code warnings; clean in case of errors so as not to leave rubbish around
if ! dune build $(INFER_BUILD_DIR)/deadcode/all_infer_in_one_file.bc; then \
if ! dune build --profile test $(INFER_BUILD_DIR)/deadcode/all_infer_in_one_file.bc; then \
$(MAKE) clean; \
exit 1; \
fi
Expand Down
5 changes: 1 addition & 4 deletions infer/src/deadcode/dune.in
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,11 @@ Format.sprintf
(executable
(name all_infer_in_one_file)
(modes byte)
(flags (%s -w +60))
(ocamlopt_flags (%s))
(flags (:standard -w +60))
(libraries %s)
(modules All_infer_in_one_file)
(preprocess (pps ppx_compare ppx_enumerate ppx_fields_conv ppx_hash ppx_sexp_conv ppx_variants_conv -no-check))
)
|}
(String.concat " " common_cflags)
(String.concat " " common_optflags)
(String.concat " " ("InferCStubs" :: common_libraries))
|> Jbuild_plugin.V1.send
2 changes: 0 additions & 2 deletions infer/src/dune-workspace.in
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,3 @@
; LICENSE file in the root directory of this source tree.

(context (opam (switch @OPAMSWITCH@) (name default) (merlin)))
(context (opam (switch @OPAMSWITCH@) (name opt)))
(context (opam (switch @OPAMSWITCH@) (name test)))
Loading

0 comments on commit fcce3c0

Please sign in to comment.