From 11d0d662ae637663982e46e7c314c1eeb543417a Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Sun, 3 May 2020 12:29:23 +0100 Subject: [PATCH 1/7] Turn on dependency checking on Inria CI Only the "old school build" test uses --disable-dependency-generation. This is also tested on both Travis and AppVeyor, so we have good release coverage checking of this option. --- tools/ci/inria/bootstrap | 2 +- tools/ci/inria/extra-checks | 4 ++-- tools/ci/inria/main | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/ci/inria/bootstrap b/tools/ci/inria/bootstrap index 00eb57f9d..382aa0388 100755 --- a/tools/ci/inria/bootstrap +++ b/tools/ci/inria/bootstrap @@ -125,7 +125,7 @@ set -ex # default values make=make instdir="$HOME/ocaml-tmp-install" -confoptions="--enable-ocamltest --disable-dependency-generation \ +confoptions="--enable-ocamltest --enable-dependency-generation \ ${OCAML_CONFIGURE_OPTIONS}" make_native=true cleanup=false diff --git a/tools/ci/inria/extra-checks b/tools/ci/inria/extra-checks index e5d56142b..03cce8ff5 100755 --- a/tools/ci/inria/extra-checks +++ b/tools/ci/inria/extra-checks @@ -149,7 +149,7 @@ git clean -q -f -d -x # We cannot give the sanitizer options as part of -cc because # then various autoconfiguration tests fail. # Instead, we'll fix OC_CFLAGS a posteriori. -./configure CC=clang-9 --disable-stdlib-manpages --disable-dependency-generation +./configure CC=clang-9 --disable-stdlib-manpages --enable-dependency-generation # These are the undefined behaviors we want to check # Others occur on purpose e.g. signed arithmetic overflow @@ -200,7 +200,7 @@ echo "======== clang 9, thread sanitizer ==========" git clean -q -f -d -x -./configure CC=clang-9 --disable-stdlib-manpages --disable-dependency-generation +./configure CC=clang-9 --disable-stdlib-manpages --enable-dependency-generation # Select thread sanitizer # Don't optimize too much to get better backtraces of errors diff --git a/tools/ci/inria/main b/tools/ci/inria/main index ee062dc9d..976402447 100755 --- a/tools/ci/inria/main +++ b/tools/ci/inria/main @@ -125,7 +125,7 @@ host='' conffile=Makefile.config make=make instdir="$HOME/ocaml-tmp-install" -confoptions="--enable-ocamltest --disable-dependency-generation \ +confoptions="--enable-ocamltest --enable-dependency-generation \ ${OCAML_CONFIGURE_OPTIONS}" make_native=true cleanup=false From e3dffa8a7f77bb730b006d99e1082f15772b7661 Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Sun, 3 May 2020 15:26:53 +0100 Subject: [PATCH 2/7] Silence CI warning for undefined variable --- Makefile.common | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile.common b/Makefile.common index 482418559..267d63a35 100644 --- a/Makefile.common +++ b/Makefile.common @@ -81,6 +81,7 @@ OCAMLLEX_FLAGS ?= -q # general (it supports both .o and .obj) ifneq "$(COMPUTE_DEPS)" "false" +RUNTIME_HEADERS := REQUIRED_HEADERS := else RUNTIME_HEADERS := $(wildcard $(ROOTDIR)/runtime/caml/*.tbl) \ From b3af8ca2a950c3256f6700b388846d26d7439b3a Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Sun, 3 May 2020 15:27:14 +0100 Subject: [PATCH 3/7] Tighten dependencies for C files The $(wildcard *.h) should only be there with --disable-dependency-generation, since otherwise gcc -MM will be determining exactly which header files should be checked. --- Makefile.common | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.common b/Makefile.common index 267d63a35..55d44fdfe 100644 --- a/Makefile.common +++ b/Makefile.common @@ -89,7 +89,7 @@ RUNTIME_HEADERS := $(wildcard $(ROOTDIR)/runtime/caml/*.tbl) \ REQUIRED_HEADERS := $(RUNTIME_HEADERS) $(wildcard *.h) endif -%.$(O): %.c $(RUNTIME_HEADERS) $(wildcard *.h) +%.$(O): %.c $(REQUIRED_HEADERS) $(CC) -c $(OC_CFLAGS) $(OC_CPPFLAGS) $(OUTPUTOBJ)$@ $< $(DEPDIR): From 617522ec9561873644cb4ce3e9e028a3230b906f Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Sun, 3 May 2020 16:00:28 +0100 Subject: [PATCH 4/7] Restore compatibility with GNU make 3.81 It's the macOS default installed version still. The dependency generation inadvertently relies on behaviour introduced in GNU make 3.82 a decade ago. The fix in otherlibs/systhreads/Makefile also corrects missing NATIVE_CPPFLAGS when generating st_stubs.n.d, so st_stubs.n.o now correctly depends on caml/stacks.h instead of caml/stack.h --- otherlibs/systhreads/Makefile | 13 ++++++------- runtime/Makefile | 13 ++++--------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/otherlibs/systhreads/Makefile b/otherlibs/systhreads/Makefile index 1645fe1a8..e848c9fcd 100644 --- a/otherlibs/systhreads/Makefile +++ b/otherlibs/systhreads/Makefile @@ -156,15 +156,14 @@ include $(addprefix $(DEPDIR)/, $(DEP_FILES)) endif %.n.$(O): OC_CPPFLAGS += $(NATIVE_CPPFLAGS) +%.n.$(D): OC_CPPFLAGS += $(NATIVE_CPPFLAGS) -%.b.c: %.c - @ +define GEN_RULE +$(DEPDIR)/%.$(1).$(D): %.c | $(DEPDIR) + $$(DEP_CC) $$(OC_CPPFLAGS) $$< -MT '$$*.$(1).$(O)' -MF $$@ +endef -%.n.c: %.c - @ - -$(DEPDIR)/%.$(D): %.c | $(DEPDIR) - $(DEP_CC) $(OC_CPPFLAGS) $(basename $*).c -MT '$*.$(O)' -MF $@ +$(foreach object_type, b n, $(eval $(call GEN_RULE,$(object_type)))) .PHONY: depend depend: diff --git a/runtime/Makefile b/runtime/Makefile index 317c8b98b..98ade8115 100644 --- a/runtime/Makefile +++ b/runtime/Makefile @@ -351,10 +351,10 @@ libasmrun_shared.$(SO): $(libasmrunpic_OBJECTS) define COMPILE_C_FILE ifneq "$(COMPUTE_DEPS)" "false" ifneq "$(1)" "%" -# This rule states that, for example, sys.b.c is sys.c and is needed for the -# dependency generation pattern rule. -$(1).c: %.c - @ +# -MG ensures that the dependencies are generated even if the files listed in +# $$(GENERATED_HEADERS) haven't been assembled yet. +$(DEPDIR)/$(1).$(D): %.c | $(DEPDIR) + $$(DEP_CC) $$(OC_CPPFLAGS) $$< -MG -MT '$$*$(subst %,,$(1)).$(O)' -MF $$@ endif $(1).$(O): %.c else @@ -418,8 +418,3 @@ DEP_FILES := $(addsuffix .$(D), $(DEP_FILES)) ifeq "$(COMPUTE_DEPS)" "true" include $(addprefix $(DEPDIR)/, $(DEP_FILES)) endif - -# -MG ensures that the dependencies are generated even if the files listed in -# $(GENERATED_HEADERS) haven't been assembled yet. -$(DEPDIR)/%.$(D): %.c | $(DEPDIR) - $(DEP_CC) $(OC_CPPFLAGS) $(basename $*).c -MG -MT '$*.$(O)' -MF $@ From c948b8d051fe0e982ca39674ebbf553d5bee1b7b Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Sun, 3 May 2020 16:03:46 +0100 Subject: [PATCH 5/7] Don't use -MG It's a subtly broken thing to do, as discovered on the ARM workers on Inria's CI. --- runtime/Makefile | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/runtime/Makefile b/runtime/Makefile index 98ade8115..5a0af8c6f 100644 --- a/runtime/Makefile +++ b/runtime/Makefile @@ -351,10 +351,14 @@ libasmrun_shared.$(SO): $(libasmrunpic_OBJECTS) define COMPILE_C_FILE ifneq "$(COMPUTE_DEPS)" "false" ifneq "$(1)" "%" -# -MG ensures that the dependencies are generated even if the files listed in -# $$(GENERATED_HEADERS) haven't been assembled yet. -$(DEPDIR)/$(1).$(D): %.c | $(DEPDIR) - $$(DEP_CC) $$(OC_CPPFLAGS) $$< -MG -MT '$$*$(subst %,,$(1)).$(O)' -MF $$@ +# -MG would ensure that the dependencies are generated even if the files listed +# in $$(GENERATED_HEADERS) haven't been assembled yet. However, this goes subtly +# wrong if the user has the headers installed, as gcc will pick up a dependency +# on those instead and the local ones will not be generated. For this reason, we +# don't use -MG and instead include $(GENERATED_HEADERS) in the order only +# dependencies to ensure that they exist before dependencies are computed. +$(DEPDIR)/$(1).$(D): %.c | $(DEPDIR) $(GENERATED_HEADERS) + $$(DEP_CC) $$(OC_CPPFLAGS) $$< -MT '$$*$(subst %,,$(1)).$(O)' -MF $$@ endif $(1).$(O): %.c else From 942105f49e3f758a7ee0c1cfe003f7415ec64832 Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Sun, 3 May 2020 16:07:07 +0100 Subject: [PATCH 6/7] Remove unneeded $(otherfiles) in runtime/Makefile No longer required, and in fact causing breakages now that -MG isn't used. --- runtime/Makefile | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/runtime/Makefile b/runtime/Makefile index 5a0af8c6f..c62c1e0fe 100644 --- a/runtime/Makefile +++ b/runtime/Makefile @@ -37,15 +37,6 @@ NATIVE_C_SOURCES := $(addsuffix .c, \ dynlink clambda_checks spacetime_nat spacetime_snapshot afl bigarray \ memprof domain) -# The other_files variable stores the list of files whose dependencies -# should be computed by `make depend` although they do not need to be -# compiled on the current build system -ifeq "$(UNIX_OR_WIN32)" "win32" -other_files := unix.c -else -other_files := win32.c -endif - GENERATED_HEADERS := caml/opnames.h caml/version.h caml/jumptbl.h CONFIG_HEADERS := caml/m.h caml/s.h @@ -409,8 +400,7 @@ i386nt.obj: i386nt.asm domain_state32.inc # Dependencies -DEP_FILES := $(addsuffix .b, $(basename $(BYTECODE_C_SOURCES) \ - instrtrace $(other_files))) +DEP_FILES := $(addsuffix .b, $(basename $(BYTECODE_C_SOURCES) instrtrace)) ifneq "$(NATIVE_COMPILER)" "false" DEP_FILES += $(addsuffix .n, $(basename $(NATIVE_C_SOURCES))) endif From 312ec987b673be43620f13abf0f25395135e232f Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Sun, 3 May 2020 17:07:17 +0100 Subject: [PATCH 7/7] Update Changes --- Changes | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Changes b/Changes index 46c0a230d..a27b8b77c 100644 --- a/Changes +++ b/Changes @@ -51,9 +51,9 @@ Working version ### Build system: -- #9332, #9518: Cease storing C dependencies in the codebase. C dependencies are - generated on-the-fly in development mode. For incremental compilation, the - MSVC ports require GCC to be present. +- #9332, #9518, #9529: Cease storing C dependencies in the codebase. C + dependencies are generated on-the-fly in development mode. For incremental + compilation, the MSVC ports require GCC to be present. (David Allsopp, review by Sébastien Hinderer, YAML-fu by Stephen Dolan) ### Bug fixes: