ocaml / dune

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

Scan dune-projects in subdirectories to collect dependencies before generating `.opam` file #4930

Open anchpop opened 3 years ago

anchpop commented 3 years ago

Desired Behavior

Imagine you have some vendored dependencies (possibly in a directory marked vendored_dirs) which themselves have dune-project files and opam dependencies. It would be convenient if your vendored dependencies' opam dependencies were added to the root-level .opam file, so all the dependencies for your project could be installed with just opam install --deps-only projectname.opam. Unless there's a better way of doing that?

anchpop commented 3 years ago

Also, point of confusion: should vendored dependencies be added to the depends field in the root dune-project? I don't want them added to the projectname.opam file because I don't want opam to try to install them when installed the dependencies.

bobot commented 3 years ago

Vendored dependencies doens't have to be added to the depends field.

Root-level dune-project could contain multiple packages, we can't determine in which one we need to add the dependencies. In order to catch during development missing package dependencies, we think the current best plan is to check during build if the dependencies are correct #4109 .

anchpop commented 3 years ago

Checking during build makes sense, but I'm curious what you think of this idea:

1) Instruct users to add vendored dependencies to the depends field for any package that uses them. Before adding those dependencies to packagename.opam, check that there's no dune-project with matching name and version in a directory marked vendored_dirs. (if the name matches but the version doesn't, maybe throw an error?)

Ideally you would also throw an error if a vendored dependency is used in a package without being mentioned in that package's depends.

2) Then, when generating the packagename.opam file, the list of dependencies is the union of all the dependencies you listed in dune-project, and those that your vendored dependencies listed in their .opam files.

That would mean that vendoring now requires no more steps than copying the project into your vendor directory. It stays in your depends, is automatically removed from your packagename.opam, and any dependencies you need to have installed to build it are also automatically added to your packagename.opam.

Thoughts?

bobot commented 3 years ago

I like the deduplication of the dependencies of the vendored projects. I don't like the reuse of the depends field, we can add a vendors field.

However vendored project have other problems more urgent such as #3734. So it is a nice idea to look at, but during or after this issue. (FYI @rgrinberg).

rgrinberg commented 3 years ago

I don't understand why something that is vendored needs to be listed as a dependency. The vendored package is already provided. What it seems like you're looking for is not vendoring but some sort of package management mode that relies on dune's composition? E.g. something like opam monorepo. Perahps you could consider using that?

I like the deduplication of the dependencies of the vendored projects. I don't like the reuse of the depends field, we can add a vendors field.

Do you see any potential uses for such a field? IMHO, vendoring something is an implementation detail that shouldn't be exposed to potential users. I suppose if something useful can be done with this info, then I'll consider it, but otherwise it seems redundant.

bobot commented 3 years ago

In a dune-project, in a project where foo is vendored.

(package
  (name bar)
  (depends ...)
  (vendors foo)
)

would instruct dune to add the dependencies of foo as dependencies of bar. The dependencies of the package foo would be found in its dune-project in the vendored directory. It allows to not copy the dependencies of foo in dune-project, and so to update foo without forgetting to update the dependencies of bar.

aryx commented 2 years ago

Any news on that?