Do not run ocamldep on single module buildables

[rgrinberg]

For executables & libraries with only a single module, we know the
dependency graph without running ocamldep.

[nojb]

Nice. LGTM.

[rgrinberg]

Let's hear from @aalekseyev if he has experience with such an optimization on the jenga side.

And from @jeremiedimino since he is more familiar with the menhir code.

[aalekseyev]

I think we never even thought of this optimization, so we don't have any experience.

A more general thing we can do is to never run ocamldep on the main module of the library or executable because we know it's going to be the terminal node in the dependency graph.

Two conceivable problems with this general approach:

1. Error messages may be different between ocamldep+cycle_detection and compilation.
2. Maybe calling ocamldep will let you reject some pathological dependency. If doing the compilation skipping ocamldep, you should be prepared to the compiler reading an arbitrary .cmi.

(1) can be confusing to the user because the same error will manifest differently. We should try as best as we can to make them look similar.
(2) is probably relevant for the binaries when multiple binaries are compiled by the same executables stanza (I think we use ocamldep to forbid the binaries' main modules depending on each other). I'm not sure if ocamldep is avoidable here, but maybe.

[ghost]

FTR, there is currently a custom error message when Dune detects (via ocamldep) that something depends on the main module:

dune/src/dune_rules/ocamldep.ml

Line 48 in d629481

User_error.raise

[rgrinberg]

A more general thing we can do is to never run ocamldep on the main module of the library or executable because we know it's going to be the terminal node in the dependency graph.

That seems to be like a nice optimization as well. Do you have special a special check for when other modules depend on the terminal module? E.g.

$ cat dune
(executable (name foo))
$ cat foo.ml
let x = 42
let () = print_int Bar.x
$ cat bar.ml
let x = Foo.x

Error messages may be different between ocamldep+cycle_detection and compilation.

Maybe, but I don't think there's any different in behavior introduced in this PR.

is probably relevant for the binaries when multiple binaries are compiled by the same executables stanza (I think we use ocamldep to forbid the binaries' main modules depending on each other). I'm not sure if ocamldep is avoidable here, but maybe.

This optimization does not apply to such executables stanza. It only applies to an executable stanza if there's a single module in the group.

[rgrinberg]

FTR, there is currently a custom error message when Dune detects (via ocamldep) that something depends on the main module:

I don't think that ever worked for single module libraries and executables. Moreover, it would be annoying if it did. It would fail on some valid programs like:

(executable
 (name foo)
 (libraries foo))

Foo should be visible foo.ml from the foo library.

It's a good think to keep in mind though, so I've added tests for all of this stuff in this PR. They confirm that that this optimization doesn't change the behavior.

[rgrinberg]

@aalekseyev gentle ping. Did I address your concerns? There's now tests to show that no error messages changed.

[aalekseyev]

Sorry, I haven't had much time this week.
I haven't read the code, but from what you say it sounds like this PR is benign and I have nothing against it.
I think, given review from @nojb, you should feel free to merge.

[rgrinberg]

OK. Will merge then. Thanks for the feedback.