pkg: recognize default dune build commands#13592
Conversation
When generating lockfiles, if a package appears to have a build command that was generated by Dune, use the "dune" build command in the package's lockfile rather than including its literal build command. This is a step towards building some dependencies in the same dune process as the one orchestrating the build rather than creating a separate dune process for each dependency. There will always be cases where dune needs to build dependencies in separate processes, such as packages that build with `make`, but packages that use the default dune build commands are good candidates for building in the same process. Dune already has the machinery to recognize "dune" build commands, however it still creates a new dune process to build such packages. Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
gridbugs
force-pushed
the
dune-build-command-in-lockfiles
branch
from
12584d4 to
7761eba
Compare
Leonidas-from-XIV
left a comment
Leonidas-from-XIV
left a comment
There was a problem hiding this comment.
I think this is a very useful PR and I've been trying my hand at implementing something like this before, but I think it needs to be a bit more general to be of use.
Changing build lines slightly (like even swapping arguments, which does not make a semantic difference at all) should not trip up the detection.
| OpamVariable.equal dev_var_ dev_var | ||
| | _ -> false | ||
| in | ||
| let is_default_build_command = function |
There was a problem hiding this comment.
I think this is a bit too constraining. e.g. imagine a package removes @doc or sets -j 1 then it doesn't trigger anymore.
I would rather suggest parsing out whether there's a call to dune build (and maybe potentially some infos about the targets but it doesn't have to be in this PR).
There was a problem hiding this comment.
It seems to me that the point of this PR is not to capture every dune invocation but only the default one.
There was a problem hiding this comment.
Yes, my point is that this is very fragile and the PR would be a lot more valuable if it detected more cases. Otherwise the semantics are pretty hard to understand from a users perspective: a package can't just call dune build to be recognized as building with Dune, but it would need to exactly mirror the exact same, fairly complex and potentially over time changing incantation that dune generates (note that Dune used to generate different lines before). Also note that not all users use Dune to generate their opam files, so they might call dune in other ways, which then would not be recognized.
In my opinion the starting point should be a specification of what we consider a "dune build", because there's a lot of questions from the top of my head that would need answering:
- Is
dune substrequired? Is it optional? Is it forbidden? - Do we detect builds that call dune but not in the OPAM file? How hard do we try to detect whether a package builds with dune? Do we inspect just the OPAM metadata? Do we inspect the tarballs?
- Which dune commands are required for a build to be considered a dune build?
dune buildfor sure, but which targets? Is@installrequired? Is@doc {with-doc}required, even if the package has no docs? - Do additional commands have an influence on whether a package is considered a dune build? Is calling
./configurebeforedune buildstill a dune build? Iscp _build/default/install/bin/foo ${bin}/fooafterdune buildstill a valid dune build? Are there commands that are ok or commands that are forbidden?
I think the answers to questions like these are important because that helps users to understand e.g. why their build is not a dune build and what they need to change to make it a dune build.
There was a problem hiding this comment.
The goal is to detect packages that could be built without shelling out, so we need to make sure that the build command would have had the same meaning as if Dune was to build the package internally. If a package builds with dune build -j1 for example, it's possible that it's not safe to build the package in parallel, so maybe Dune should still shell out in that case. Same for packages that run a configure script before building, or invoke dune indirectly (like from a Makefile).
Perhaps a better alternative would be for dune to add a filed to opam files it generates indicating that the package can be built with dune's default command rather than having Dune attempt to parse the command and decide whether it's the default command?
3 participants
When generating lockfiles, if a package appears to have a build command that was generated by Dune, use the "dune" build command in the package's lockfile rather than including its literal build command.
This is a step towards building some dependencies in the same dune process as the one orchestrating the build rather than creating a separate dune process for each dependency. There will always be cases where dune needs to build dependencies in separate processes, such as packages that build with
make, but packages that use the default dune build commands are good candidates for building in the same process. Dune already has the machinery to recognize "dune" build commands, however it still creates a new dune process to build such packages.