Skip to content

Merge debugger/Makefile into the root Makefile#12198

Merged
dra27 merged 5 commits intoocaml:trunkfrom
shindere:merge-debugger-makefile
May 24, 2023
Merged

Merge debugger/Makefile into the root Makefile#12198
dra27 merged 5 commits intoocaml:trunkfrom
shindere:merge-debugger-makefile

Conversation

@shindere
Copy link
Copy Markdown
Contributor

This continues the work started in #11243, #11248, #11268, #11420 and #11675.

@shindere shindere force-pushed the merge-debugger-makefile branch from 5497950 to 4a19f99 Compare April 21, 2023 09:21
@nojb nojb mentioned this pull request Apr 23, 2023
@shindere shindere force-pushed the merge-debugger-makefile branch 5 times, most recently from 52c235e to 906eb6c Compare April 26, 2023 12:30
@shindere
Copy link
Copy Markdown
Contributor Author

Note that, to be consistent with the rest of the build system, this PR
enables -g for ocamldebug.

Also, the debugger's dependencies are computed in debugger/.depend as
before, so that it is simple to specifiy a different list of directories.

@shindere shindere force-pushed the merge-debugger-makefile branch 4 times, most recently from dc48720 to 836ef17 Compare May 15, 2023 13:45
shindere and others added 3 commits May 22, 2023 11:34
If symlinks are used, they need to be native-Windows compatible ones.
Ensure that the CYGWIN and MSYS environment variables use the
winsymlinks:nativestrict option, which causes symlink(2) to fail if NTFS
symlinks are not available. In this case, config.status will then
fall-back to hardlinks or copying.
…ystem

More precisely, this commit makes use of the following additional flags
when compiling the debugger: -g -principal -bin-annot.

The warning 70 about missing interfaces is re-enabled for the
debugger, which leads to the addition of two interface files.
@shindere
Copy link
Copy Markdown
Contributor Author

This branch now starts with two commits that may seem unrelated but here is
what they are and why they are here.

On trunk, make alldepend computes the dependencies for the other libraries
before those of the debugger, ocamldoc and ocamltest. This matters in a
slightly subtle way because, on trunk, it's the computation of the
dependencies for the Unix library which
creates the otherlibs/unix/unix.ml
file (when building a minimal compiler as is done to verify the dependencies
by our GitHub actions), whose existence has an impact on the
dependencies as computed e.g. for the debugger.

On the PR branch, the order enforced on trunk is no longer enforced so that
there is no guarantee that otherlibs/unix/unix.ml exists when the
dependencies for the debugger get computed.

Although it would be possible to impose an ordering on the computation of
the dependencies as was done on trunk by using constraints on GNU make
targets, that looks convoluted (and, actually, a bit artificial and not very
well justified) and it felt both simpler and more accurate to jsut generate
otherlibs/unix/unix.ml as part of the configure stage.

That's what the first commit of this PR does, using the AC_CONFIG_LINKS
macro to create a link in the build tree so that otherlibs/unix/unix.ml
points to the right implementation of the Unix module. It is worth
mentionning that this macro will work fine even in the context of
out-of-source builds. Also, the macro will always create symlinks if
possible, falling back to hard-links and then plain copies if necessary.

The subsequent commit, authored by @dra27 and reviewed by me, makes sure
that only genuine native Windows symlinks will be used (rather than WSL or
Cygwin ones), because the links are to OCaml files and must thus be
understood by the native Windows OCaml compiler, which only understands
native Windows links.

@shindere
Copy link
Copy Markdown
Contributor Author

One other thing to note is that, in its present state, this PR compiles the
files under the debugger/ directory with -bin-annot, which was not the
case on trunk. The total size of the .cmt and .cmti files created
because of this change is 2,4MB. The option has been added for
uniformity and I believe we can live with the overhead. What's missing here is
a -no-bin-annot option that we could add to the default compiler flags on
a per-target basis.

@shindere shindere force-pushed the merge-debugger-makefile branch 2 times, most recently from fa33f9a to e0cf8c7 Compare May 22, 2023 12:31
So far, program targets could only accept object files to be linked
into the built program as their prerequisites. This was so because the recipes
were using $^ as the list of object files to link, meaning all
the prerequisites.

With this commit, it becomes possible to have prerequisites that
do need to be build before the target is built but without necessarily
being linked into the target.

This is implemented by defining target-specific variables foo_BCOBJS
and foo_NCOBJS containing the list of associated object files.

These variables are then used in the recipes, meaning that the
targets can now take additional prerequisites which won't appear in the recipes.
@shindere shindere force-pushed the merge-debugger-makefile branch 5 times, most recently from 43e5206 to 9e0d87f Compare May 24, 2023 08:59
@shindere shindere force-pushed the merge-debugger-makefile branch from 9e0d87f to aad7f97 Compare May 24, 2023 09:08
Copy link
Copy Markdown
Member

@dra27 dra27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another one bites the dust! The force push history and logs record the offline reviewing which has taken place. This is the first of the merges where the artefacts are all built using the root compiler (i.e. ./ocamlc rather than boot/ocamlc), and so also the first time Makefile.best_binaries gets pulled into the root Makefile. This has been through precheck#832.

@dra27 dra27 merged commit 94a91f5 into ocaml:trunk May 24, 2023
@shindere
Copy link
Copy Markdown
Contributor Author

shindere commented May 24, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants