Merge otherlibs/dynlink/Makefile into the root Makefile#13285
Merged
dra27 merged 6 commits intoocaml:trunkfrom Jul 19, 2024
Merged
Merge otherlibs/dynlink/Makefile into the root Makefile#13285dra27 merged 6 commits intoocaml:trunkfrom
dra27 merged 6 commits intoocaml:trunkfrom
Conversation
ghost
reviewed
Jul 10, 2024
4283bd0 to
ab3f2ee
Compare
Contributor
Author
|
Fixed, thanks!
|
f7f9459 to
f3d3ae3
Compare
0f433f6 to
6a215ce
Compare
Contributor
Author
|
A few updates: The PR is now ready to be reviewed, after a bit of work, To elaborate on the change of checksums, this is actually due Also, thePR now begins with a commit that simplifies the The commit which had been added to copy |
Before this commit, the test was adding dynlink.cmx?a to the all_modules variable but this is actually not necessary because the "include dynlink" directive already adds them as appropriate to the libraries variable. This has been spotted by @dra27 as it led to a test failure when -linkall was (incorrectly) used because this led to some modules being linked twice.
This commit introduces build variables
foo_{COMMON,BYTECODE,NATIVE}_LINKFLAGS
where foo is the name of a program to build.
These variables are initialised with empty strings (no flags) and
can then be overridden by each program which needs to.
Such variables allow more fine-grained control over flags so that they
do not propagate when put in target-specific variables.
It may be the case that these variables become unnecessary once we
use a version of GNU make which is recent enough to support the
private keyword.
This variable lists the other libraries that get built by the recursive makefiles. At this point it has the same content as the OTHERLIBRARIES variable but, when the other libraries will be built by the root Makefile their names will be removed from this variable. It is thus expected that this variable becomes empty and can be removed once all the other libraries get built by the root Makefile.
otherlibs/dynlink/dynlink_platform_intf.mli is a copy of the corresponding otherlibs/dynlink/dynlink_platform_intf.ml file. Before this commit, the copy was done during the build. With this commit the copy (more likely a symlink) is done by configure as is already done for other files that need to be duplicated.
6a215ce to
4c6a5d5
Compare
Contributor
Author
|
Plenty of thanks @ëra27 for your super supportive review.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR continues the build system unification work started in #11243, #11248, #11268, #11420, #11675, #12198, #12321, #12586, #12616, #12706 and #13048.
The present PR merges
otherlibs/dynlink/Makefileinto the rootMakefileand consists of four commits. The three first ones have the property of not altering the installed artefacts at all. The only commit that does so is the last one, which is the core of the PR, and the only thing it changes to the installed artifacts is the checksums, which is due to the fact that the compilation of the OCaml sources is done from the top-level of the source tree rather than from theotherlibs/dynlinkdirectory as before.The first commit introduces the private
OTHERLIBSbuild variable to keep track of which other libraries are still built byotherlibs/Makefile. This variable is initially equal toOTHERLIBRARIES(the list of other libraries to build) but this list will become shorter andshohrter as more and more other libraries will be built directly by the root Makefile. Once all the other libraries are built by the root Makefile this variable is expected to become empty and to finally vanish.The second commit delegates some file copying which was previously done during the build to the configure script.
THe third (and last preparatory) commit extends the general framework defined in
Makefile.commonto make sure the.cmxfiles of a native library get copied to the directory where its.cmxafile resides, as this is necessary when linking with dynlink's native version. As the copy happens also for all the other libraries built by the framework, thecleanrule ofcompilerlibshad to be adjusted to clean the newly copied artefacts.Finally, the PR's last commit is its core, which is the actual merge of
otherlibs/dynlink/Makefileinto the rootMakefile. As usually, this goes hand in hand withotherlibs/dynlink/.dependbeing merged into the root.depend.