Allow packages with no build commands #355
Conversation
gridbugs
force-pushed
the
allow-metapackages
branch
from
2486323 to
fbd3864
Compare
lib/opam_solve.ml
Outdated
| @@ -95,11 +95,12 @@ module Opam_monorepo_context (Base_context : BASE_CONTEXT) : | |||
| Opam.depends_on_dune ~allow_jbuilder depends | |||
| || Opam.depends_on_dune ~allow_jbuilder depopts | |||
| in | |||
| let has_no_build_command = List.is_empty (OpamFile.OPAM.build opam_file) in | |||
| let summary = Opam.Package_summary.from_opam pkg opam_file in | |||
| let is_valid_dune_wise = | |||
| Opam.Package_summary.is_base_package summary | |||
| || Opam.Package_summary.is_virtual summary | |||
There was a problem hiding this comment.
I'm wondering, wouldn't it make sense to add this to is_virtual? Or a new category in Opam.Package_summary?
There was a problem hiding this comment.
I considered that and conceptually it would make sense for packages without build commands to be considered "virtual". We could add a new field to Package_summary.t:
build : OpamTypes.command listThe problem is that in order to update Package_summary.equal and Package_summary.pp we'd need to implement equality and pretty-printing for OpamTypes.command and it looks like that would add a lot of code.
However, it appears that neither Package_summary.equal nor 'Package_summary.pp' are ever called, so perhaps they can just be deleted.
There was a problem hiding this comment.
I've updated this PR so that the check is now done inside is_virtual
gridbugs
force-pushed
the
allow-metapackages
branch
3 times, most recently
from
a59e6b9 to
c8ced2a
Compare
test/bin/no-build-command.t/run.t
Outdated
| depend-without-dune (this should fail due to the transitive dependency on | ||
| without-dune which has a build command but doesn't depend on dune) | ||
|
|
||
| $ opam-monorepo lock test-depend-without-dune.opam |
There was a problem hiding this comment.
Can you please filter the output? Either with some grep trickery (which is admittedly quite annoying) or just by throwing the output away, as long as the [1] matches the test is passing. :)
All the text would make the output quite fragile if versions change or the wording changes slightly.
gridbugs
force-pushed
the
allow-metapackages
branch
2 times, most recently
from
3ef4fd5 to
16b2793
Compare
Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
gridbugs
force-pushed
the
allow-metapackages
branch
from
16b2793 to
57b8c97
Compare
CHANGES: ### Changed - Treat packages with no build commands as if they can be built with dune (tarides/opam-monorepo#355, @gridbugs) ### Fixed - Fix resolving refs of locally pinned repositories (tarides/opam-monorepo#326, tarides/opam-monorepo#332, @hannesm, @Leonidas-from-XIV) - Read the `compiler` flag from OPAM metadata thus classifying more packages correctly as base packages (tarides/opam-monorepo#328, @Leonidas-from-XIV) - Fix bug where dev repo urls ending with a "/" would result in `opam monorepo pull` placing package source code directly inside the duniverse directory instead of in a subdirectory of the duniverse directory (tarides/opam-monorepo#359, @gridbugs)
In tarides/opam-monorepo#355 opam-monorepo marked packages without build instructions as virtual (e.g. packages that are conf-packages, dependency-only packages, etc). This requires that the dummy OPAM files used here also have `build` stanzas, otherwise they are considered virtual and skipped.
If a package has no build commands, chances are that it's a metapackage which exists just so that its dependencies can be installed.
Signed-off-by: Stephen Sherratt stephen@sherra.tt