Further C dependency fixes

[dra27]

macOS still installs a 14 year old version of GNU make (3.81). It turns out that the changes in #9332 need a feature added a decade ago in GNU make 3.82. The fix I propose here is to remove the virtual rule in runtime/Makefile and otherlibs/systhreads/Makefile which were supposed to allow make to realise that it's OK for sys.b.c not to exist and instead to generate the dependency rules for each case (in runtime/Makefile this is virtually identical; in otherlibs/systhreads/Makefile it takes nearly the same number of lines, as it happens).

Triggered by #9528, various fixes:

• It is readily apparent both from Build fails on macOS after e5c8bee85e7ae2d974069d63528333ffacc1923b #9528 and Specify C compiler flags correctly in win32unix Makefile #9518 that the CI strategy for --disable-dependency-generation was wrong - Travis and AppVeyor both do a test using it, and now Inria's CI does virtually all of its tests with --enable-dependency-generation. This is also more logical, as it tests more diverse tools than the --disable-dependency-generation which only tests GNU make, and in a very obvious way.
• There was a missing blank definition of RUNTIME_HEADERS in Makefile.common which has been added to satisfy the warning.
• In the flurry of renamings at the end of Cease storing C dependencies in the repository #9332, the wrong variable was used at the bottom of Makefile.common so in --disable-dependency-generation mode, C files depended on the runtime headers only - they now depend on runtime headers and the local headers
• Switching Inria's CI to --enable-dependency-generation revealed the expected failure on macOS from Build fails on macOS after e5c8bee85e7ae2d974069d63528333ffacc1923b #9528, but also failed the ARM32 and ARM64 builds. It turns out those workers have OCaml installed on them, which revealed that using -MG is completely flawed (cf. Specify C compiler flags correctly in win32unix Makefile #9518) and this is removed here. Doing that also revealed that the old $(otherfiles) (which was previously used to cause Unix builds to generate the dependency information for win32.o and vice versa) was still there, even though that's never needed.

An earlier version of this branch has already been through precheck, but this exact branch is presently precheck#371

[gasche]

The first two commits ("turn on dependency checking" and "fix an undefined variable") are obviously fine. I had a question on "local headers". I am happy with the "Don't use -MG" explanation and "otherfiles" removal. What remains is the "GNU Make 3.81" one which uses too many functions for me to be able to read, you will need someone else to review it.

[gasche]

I'm confused, if $(REQUIRED_HEADERS) is either empty or $(RUNTIME_HEADERS) $(wildcard *.h), how can this make a difference given that $(wildcard *.h) is already a dependency? (Could this be a matter of the wildcard being evaluated at a different time?).

[dra27]

No, that's another artefact of the change... too many variables being renamed and refactored in the review, and the semantics get messed up!

[dra27]

The point here is that if COMPUTE_DEPS is false then you want to depend on $(RUNTIME_HEADERS) (which is everything in runtime/caml/) and any headers in the current build directory. If COMPUTE_DEPS is not false, then $(DEP_CC) will have built the .d files indicating exactly which .h files to depend on.

It's a relatively minor optimisation in this Makefile, because in the directories apart from runtime which have .c files, changing a header file in that directory does usually rebuild everything (unixsupport.h, for example)

[gasche]

I still don't understand what this commit is or was fixing.

The question applies in fact to both the old state (as you initially submitted the PR) and the new state, after your new commit "Tighten dependencies":

• The old change looks like a no-op to me, so was it really fixing an issue? What issue? (Was the change wrong, or am I missing something?)
• With the new change you make in "Tighten dependencies", then we end up with no .h dependencies at all if COMPUTE_DEPS is false. Is that the point of the change to this file, or another beneficial change?

[dra27]

The old change in this PR accidentally a no-op, yes - while adding the empty case for $(RUNTIME_HEADERS) I noticed that the wrong variable was being used below it, but didn't fully take in why it was all still OK.

The pattern rules are not normally supposed to mention dependencies (cf. the .cmo and .cmx suffix/pattern rules) - you have separate specific rules which list the dependencies. $(REQUIRED_HEADERS) is to allow development of the compiler even with --disable-dependency-generation (so $(COMPUTE_DEPS) is false). In that case, you won't have the specific dependencies for each .o file as they're not generated. So the idea is that each .o file depends on every single .h file - so any change to any header rebuilds all .o files (it's the best you can do in the absence of dependency information).

The mistake is that before I'd left one of those cases for $(COMPUTE_DEPS) = false in global scope.

(the reason for this is that it means that instead of requiring developers on MSVC to have GCC to work on the compiler, we recommend developers on MSVC have GCC because incremental compilation of the compiler will be faster. It also means that there is not a configuration where incremental compilation of the C sources is actually broken)

[dra27]

For the "Restore compatibility" commit. Starting with otherlibs/systhreads/Makefile as it only has two cases.

What was done before is that you wanted a single rule for generating foo.b.d and foo.n.d from foo.c. The rule which was there before actually makes foo.b.d depend on foo.b.c and foo.n.d depend on foo.n.c. Those C files don't exist, which is why there were "dummy" rules which direct make to be satisfied that foo.b.c depends on foo.c. For reasons I don't fully understand, this requires the shortest-stem changes for pattern rules which were introduced in GNU make 3.82 (see the breaking changes of its NEWS file) which I verified by building GNU make 3.82 on my MacBook. The presence of those rules for some reason causes the standard .o: .c rule to activate. So that's the problem, now...

What is now done in otherlibs/systhreads/Makefile is that two different pattern rules are generated which are:

.dep/%.b.d: %.c | .dep
	$(DEP_CC) $(OC_CPPFLAGS) $< -MT '$*.b.o' -MF $@

.dep/%.n.d: %.c | .dep
	$(DEP_CC) $(OC_CPPFLAGS) $< -MT '$*.n.o' -MF $@

However, as in runtime/Makefile, those rules are generated using meta-programming to eliminate the recipe duplication. So GEN_RULE constructs the exact text of one of those rules, receiving the n or b as a parameter in $(1). Any $s in the rule have to be escaped to $$ - the ones which aren't escaped are those which can clearly be evaulated once ($(DEPDIR) and $(D) obviously doesn't change in the Makefile). Note that technically $$(DEP_CC) does not have to be $-escaped, but I've done that because Sébastien's other rules do that for $$(CC). The variable which must be $-escaped (apart from the rule's automatic variables, obviously) is $$(OC_CPPFLAGS) because it needs to pick-up the rules which amend that variable dynamically (the OC_CPPFLAGS += bit for native code). The foreach call is exactly as in 9bf6d49#diff-fa4cef881ee67fe396ca277ebc75ded1R371-R372 and $(calls that GEN_RULE macro twice to generate the text for the .dep/%.b.d: and .dep/%.n.d: rules and then $(evals it to execute the additional Makefile rules.

In runtime/Makefile, the $(foreach machinery was already there from Sébastien's previous refactoring of that Makefile, so the change here is to delete the "dummy" %.b.c: %.c rules (which similarly aren't working in GNU make 3.81) and move the general %.$(D) rule into COMPILE_C_FILE in the same way.

[dra27]

Enough iterations over this that it's running through precheck again

[xavierleroy]

I haven't reviewed, but precheck is green and it's a MAJOR INCONVENIENCE that the trunk is broken on macOS, so I'll merge.

[dra27]

Thanks, @xavierleroy (and sorry for the breakage in the first place)

[dra27]

(I have notes for a few things to ping Sébastien to look over when he's back, incidentally)

[damiendoligez]

macOS still installs a 14 year old version of GNU make (3.81).

Note that this is unlikely to change anytime soon: 3.81 is the latest version under GPLv2 and Apple seems to be allergic to GPLv3.