Make dune describe correctly handle overlapping implementations for virtual libraries

[esope]

Fixes issue #5747

Here is what happened before this fix: if your project was using a
virtual library and at least two implementations for that virtual
library, then dune describe would not produce any information for the
libraries of your project (it would only show information about the
executables, if there were any). The issue was in the computation of
the transitive closure of libraires, that raised an error by default
in the presence of overlapping implementations of virtual libraries.
In such a case, the error was treated by discarding all information
about the libraries.

The fix consists in computing the transitive closure of libraries
without performing any overlap checks between implementations of a
virtual library. We have defined the descriptive_closure function
for this very purpose.

The overlap check is necessary when you compute the transitive closure
of libraries that you intend to build (otherwise you do not know which
implementation should be picked).

In dune describe, you want to describe the whole workspace, regardless
of what you want to build or not, which means that you need to handle
all the libraries of your workspace, that may include several
implementations for virtual modules.

If you absolutely want to perform the overlap check in dune describe,
then will not always be able to describe the whole workspace.

Signed-off-by: Benoît Montagu [email protected]

[rgrinberg]

I haven't looked at the PR closely, but the first question that comes to mind is why is it correct to ignore the overlap check for dune describe?

[esope]

The overlap check is necessary when you compute the transitive closure of libraries that you intend to build (otherwise you do not know which implementation should be picked).
In dune describe, you want to describe the whole workspace, regardless of what you want to build or not, which means that you need to handle all the libraries of your workspace, that may include several implementations for virtual modules.
I you absolutely want to perform the overlap check in dune describe, then will not always be able to describe the whole workspace.

[rgrinberg]

Thanks for the explanation. It makes sense and I'd like it included either in the commit message or the PR message.

Now for some points about the implementation:

• The test commit demonstrating the bug should come first. The bug should be described and the corresponding github issue be linked.
• Instead of adding a flag to our closure functions used for compilation/linking, you should introduce a new function that does what you want without introducing all these concerns about linking, virtual libraries, etc. It should be perhaps called a descriptive_closure and it should be much simpler.

[esope]

@rgrinberg : if you want to have a look, here is a new version with the changes that you suggested.

[rgrinberg]

Thanks, it looks good. I simplified the descriptive closure a bit further. None of the Resolve monad book keeping is necessary because we don't need to delay errors like we do for setting up build rules.

[esope]

Thanks for the cleanup. I must confess it is not completely clear to me what role each monad is actually playing in dune.

[emillon]

Thanks!