Skip to content

Fix Makefile dependencies for compilerlibs archives#9211

Merged
gasche merged 2 commits intoocaml:trunkfrom
gasche:fix-objinfo-dependencies
Dec 27, 2019
Merged

Fix Makefile dependencies for compilerlibs archives#9211
gasche merged 2 commits intoocaml:trunkfrom
gasche:fix-objinfo-dependencies

Conversation

@gasche
Copy link
Copy Markdown
Member

@gasche gasche commented Dec 26, 2019

compilerlibs archives: add dependencies on .mli-only compilation units

This fixes the issue reported in #2268 (comment).

Before this commit, the Makefile for compilerlibs archives only tracks
their dependencies on implementation object files (.cmo,
.cmx). .mli-only files in those directories result in a .cmi without
a .cmo.

The main makefile uses dependencies on compilerlibs archives as
a proxy for all dependencies on the corresponding compiler
modules. This assumption was broken by .mli-only modules not being
dependencies on the archives.

For example, rebuilding compilerlibs archives would not refresh
file_format/cmxs_format.cmi. The following steps would lead to a build
failure:

make core
make ocamltools
<add a new blank line at the beginning of utils/misc.mli>
make ocamltools

File "/home/gasche/Prog/ocaml/github-trunk/tools/objinfo.ml", line 1:
Error: The files /home/gasche/Prog/ocaml/github-trunk/utils/misc.cmi
       and /home/gasche/Prog/ocaml/github-trunk/file_formats/cmxs_format.cmi
       make inconsistent assumptions over interface Misc

(The first commit in this PR moves the Makefile targets for compilerlibs archives to a separate, included Makefile.)

@gasche gasche changed the title Fix objinfo dependencies Fix Makefile dependencies for compilerlibs archives Dec 26, 2019
Copy link
Copy Markdown
Contributor

@lthls lthls left a comment

Choose a reason for hiding this comment

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

I have found a few likely typos.
I like this approach to explicitly list mli-only files somewhere, and splitting in a different Makefile seems also a good idea to me.

I wish this file could be generated or checked at least semi-automatically (there's no way to notice if you've put the cmo files in the wrong order until you get a not very helpful link error), but that's clearly beyond the scope of this PR.

I haven't tried to check that no make targets are broken by this PR, but if CI passes I'm fine with this getting merged.

@gasche
Copy link
Copy Markdown
Member Author

gasche commented Dec 26, 2019

Thanks a lot! (Apologies for the typos.)

I agree that an automatic analysis would be nice. We want to ensure that all compiler compilation-units end up listed in one of those archive-creating variables, and that the split between mli-only and normal units is correct.

For the record, I used the following command to list all .mli files that may belong to mli-only modules (only an approximation due to .mly, directory-include tricks etc.):

for f in $(find . -name '*.mli' | grep -v testsuite); do
    if test ! -e $(echo $f | sed s/\\.mli/\\.ml/); then
        echo $f;
    fi;
done

@gasche gasche force-pushed the fix-objinfo-dependencies branch from 06b0fa1 to 9b901af Compare December 26, 2019 13:18
@gasche
Copy link
Copy Markdown
Member Author

gasche commented Dec 26, 2019

@lthls I fixed the typos you found and updated the branch.

@gasche gasche force-pushed the fix-objinfo-dependencies branch from 9b901af to d772dfa Compare December 26, 2019 13:29
@gasche gasche force-pushed the fix-objinfo-dependencies branch 2 times, most recently from cef1715 to 52b5815 Compare December 26, 2019 20:05
Before this commit, the Makefile for compilerlibs archives only tracks
their dependencies on implementation object files (.cmo,
.cmx). .mli-only files in those directories result in a .cmi without
a .cmo.

The main makefile uses dependencies on compilerlibs archives as
a proxy for all dependencies on the corresponding compiler
modules. This assumption was broken by .mli-only modules not being
dependencies on the archives.

For example, rebuilding compilerlibs archives would not refresh
file_format/cmxs_format.cmi. The following steps would lead to a build
failure:

    make core
    make ocamltools
    <add a new blank line at the beginning of utils/misc.mli>
    make ocamltools

    File "/home/gasche/Prog/ocaml/github-trunk/tools/objinfo.ml", line 1:
    Error: The files /home/gasche/Prog/ocaml/github-trunk/utils/misc.cmi
           and /home/gasche/Prog/ocaml/github-trunk/file_formats/cmxs_format.cmi
           make inconsistent assumptions over interface Misc
@gasche
Copy link
Copy Markdown
Member Author

gasche commented Dec 27, 2019

( @lthls this now seems to have converged to a test-passing state )

@gasche gasche merged commit c5e875d into ocaml:trunk Dec 27, 2019
@gasche
Copy link
Copy Markdown
Member Author

gasche commented Dec 27, 2019

Thanks!

gasche added a commit that referenced this pull request Jan 8, 2020
(The typo was caught by make's undefined-variables warning on the INRIA CI)
@nojb nojb mentioned this pull request Jan 8, 2020
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