ocaml / dune

A composable build system for OCaml.
https://dune.build/
MIT License
1.62k stars 401 forks source link

Checking of dune-project/opam specification #4109

Open bobot opened 3 years ago

bobot commented 3 years ago

The tool opam-dune-lint of @talex5 is great. However it uses the feature dune external-lib-deps which is not completely correct because of optional libraries and that will be even less true if dune file generation is added #3901.

Since the build is more dynamic, getting and checking the information should also be more dynamic. So a possibility is to gather and check during the build the dependencies.

The two possibilities are not excluded and dune could play the role of the external tool. Additionally a tool like ocaml-ci could by testing different configuration indicate if some opam package are not used. It is the drawback of the approach we only get under-approximation instead of the current over-approximation method (until dune file generation).

rgrinberg commented 3 years ago

I'll paraphrase what I said in the meeting to (hopefully) clarify my points:

Using dune files as a source of information for generating opam files isn't a great idea. We want to limit such non local information flow because it kills incremental builds. It seems nicer to keep the dune-project file as the source of truth for package information and make sure that all downstream rules respect it.

Taking this idea plus a little inspiration from esy, I imagine that opam-dune-lint could be much more useful if it worked as follows:

The 3rd step is slow, but it only needs to run if a dune-project file is edited. This also completely gets rid of the need for dune external-lib-deps.

bobot commented 3 years ago

Using dune files as a source of information for generating opam files isn't a great idea. It seems nicer to keep the dune-project file as the source of truth for package information and make sure that all downstream rules respect it.

I agree, I didn't proposed to get rid of it.

I like your proposal of restricting the switch (It could be done using overlay fs on linux, which is now usable without privilege), but I think if we don't check the dependency of each rule or if we don't handle each package separately we are going to miss missing package dependencies forgotten for one package and not another.

talex5 commented 3 years ago

@rgrinberg your suggestion of only having the mentioned packages installed is what the CI has always done. The problem (as @bobot says) is that the project may build as a whole, but fail when submitted to opam-repository and packages are installed individually.

It would be possible to test each package in isolation (as opam-repo-ci does), but it's quite expensive. Since dune already knows which ocamlfind libraries it used, it seems much more efficient to just check that, which is what opam-dune-lint does.

Also, even if you test each package with -p, it still won't catch the case of a dependency that is only present as a transitive dependency of something else, whereas opam-dune-lint does catch this (at least, when used with implicit_transitive_deps false).

rgrinberg commented 2 years ago

It would be possible to test each package in isolation (as opam-repo-ci does), but it's quite expensive. Since dune already knows which ocamlfind libraries it used, it seems much more efficient to just check that, which is what opam-dune-lint does.

Yes, I agree with that. In fact, I tried implementing something like that with strict_package_deps, but it turned out to be a lot harder than I realized.