Merge runtime/Makefile in the root Makefile#11248
Conversation
453497c to
498176c
Compare
This has been forgotten when yacc/Makefile got merged in the root one.
This is so that these recipes can use variables.
498176c to
97c5b3c
Compare
dra27
left a comment
There was a problem hiding this comment.
A few minor typos - but the only error I think is the tightening of the dependencies pulled in by make stdlib/libcamlrun.a.
(for anyone else who wishes to check, this PR is best reviewed commit-by-commit and is much easier to review from a checkout where you can compare the new part of the root Makefile against the deleted runtime/Makefile)
It'll be really great in the next phase of this work when we get to exploit the parallelism opportunities which should come from having eliminating the recursive make calls!
Makefile
Outdated
| startup_byt \ | ||
| )) | ||
| runtime_BYTECODE_C_SOURCES = \ | ||
| $(runtime_COMMON_C_SOURCES) $(runtime_BYTECODE_ONLY_C_SOURCES) |
There was a problem hiding this comment.
Possibility, which would avoid the addsuffix and addprefix in the names:
runtime_BYTECODE_C_SOURCES = \
$(runtime_COMMON_C_SOURCES:%=runtime/%.c) \
$(runtime_BYTECODE_ONLY_C_SOURCES:%=runtime/%.c)
Makefile
Outdated
| runtime_NATIVE_C_SOURCES = \ | ||
| $(runtime_COMMON_C_SOURCES) $(runtime_NATIVE_ONLY_C_SOURCES) | ||
|
|
||
| #* Header files generated by configure |
There was a problem hiding this comment.
Typo: asterisk instead of hash in ##
Makefile
Outdated
| SAK_CFLAGS ?= $(OC_CFLAGS) $(CFLAGS) $(OC_CPPFLAGS) $(CPPFLAGS) | ||
| SAK_LINK ?= $(MKEXE_USING_COMPILER) | ||
|
|
||
|
|
Makefile
Outdated
| rm -f runtime/primitives runtime/primitives.new runtime/prims.c \ | ||
| $(runtime_BUILT_HEADERS) | ||
| rm -f runtime/domain_state*.inc | ||
| rm -rf $(DEPDIR) |
There was a problem hiding this comment.
I think it's fine, but just noting that this erases all dependencies, not just $(DEPDIR)/runtime
Makefile
Outdated
| makeruntimeopt: | ||
| $(MAKE) $(BOOT_FLEXLINK_CMD) runtime-allopt | ||
| stdlib/libasmrun.$(A): runtime-allopt | ||
| cp runtime/libasmrun.$(A) $@ |
There was a problem hiding this comment.
This is as before, but we could take the opportunity to use $(LN) here instead of cp, as for stdlib/libcamlrun.$(A)
Makefile
Outdated
| .PHONY: makeruntime | ||
| makeruntime: | ||
| $(MAKE) $(BOOT_FLEXLINK_CMD) runtime-all | ||
| stdlib/libcamlrun.$(A): runtime/libcamlrun.$(A) |
There was a problem hiding this comment.
Unfortunately, as with stdlib/libasmrun.$(A) below, the dependency needs to be runtime-all, or you can't do make world.opt ; make clean ; make opt.opt (i.e. this step implicitly updates runtime/ocamlrun which has been lost with this change).
|
Many thanks for your comments, David!
I implemented the changes in a dedicated commit to hopefully ease your
reviewing work.
Once done, I'll squash this new commit in the appropriate one.
Let's hope CI remains happy.
|
|
Thanks!
Will squash the extra commit now.
|
This commit moves the computed dependencies of C files from runtime/.dep to .dep/runtime.
c7a794f to
d36c444
Compare
This continues the work started in #11243.
This PR changes where the dependencies for C files are stored, from
runtime/.depbefore to.dep/runtimenow.