Treat packages with no build command as virtual

Signed-off-by: Stephen Sherratt <stephen@sherra.tt>

CHANGES.md

@@ -4,6 +4,9 @@
 
 ### Changed
 
+- Treat packages with no build commands as if they can be built with dune (#355,
+  @gridbugs)
+
 ### Deprecated
 
 ### Fixed

lib/opam.ml

@@ -129,6 +129,14 @@ module Hash = struct
     Format.fprintf fmt "%s:%s" kind contents
 end
 
+module Command = struct
+  type t = OpamTypes.command
+
+  let pp fmt (t : t) =
+    OpamPp.print OpamFormat.V.command t
+    |> OpamPrinter.FullPos.value |> Format.pp_print_string fmt
+end
+
 module Depexts = struct
   let pp fmt t =
     let pp_filter fmt filter =
@@ -196,18 +204,21 @@ module Package_summary = struct
     dev_repo : string option;
     depexts : (OpamSysPkg.Set.t * OpamTypes.filter) list;
     flags : Package_flag.t list;
+    build_commands : OpamTypes.command list;
   }
 
-  let pp fmt { package; url_src; hashes; dev_repo; depexts; flags } =
+  let pp fmt
+      { package; url_src; hashes; dev_repo; depexts; flags; build_commands } =
     let open Pp_combinators.Ocaml in
     Format.fprintf fmt
       "@[<hov 2>{ name = %a;@ version = %a;@ url_src = %a;@ hashes = %a;@ \
-       dev_repo = %a;@ depexts = %a;@ flags = %a }@]"
+       dev_repo = %a;@ depexts = %a;@ flags = %a;@ build_commands = %a }@]"
       Pp.package_name package.name Pp.version package.version
       (option ~brackets:true Url.pp)
       url_src (list Hash.pp) hashes
       (option ~brackets:true string)
-      dev_repo Depexts.pp depexts (list Package_flag.pp) flags
+      dev_repo Depexts.pp depexts (list Package_flag.pp) flags (list Command.pp)
+      build_commands
 
   let from_opam package opam_file =
     let url_field = OpamFile.OPAM.url opam_file in
@@ -220,14 +231,16 @@ module Package_summary = struct
     in
     let depexts = OpamFile.OPAM.depexts opam_file in
     let flags = OpamFile.OPAM.flags opam_file in
-    { package; url_src; hashes; dev_repo; depexts; flags }
+    let build_commands = OpamFile.OPAM.build opam_file in
+    { package; url_src; hashes; dev_repo; depexts; flags; build_commands }
 
   let has_flag flag { flags; _ } = List.mem flag ~set:flags
   let is_compiler v = has_flag OpamTypes.Pkgflag_Compiler v
 
   let is_virtual = function
     | { url_src = None; _ } -> true
     | { dev_repo = None | Some ""; _ } -> true
+    | { build_commands = []; _ } -> true
     | _ -> false
 
   let is_compiler_package { package; _ } =

lib/opam.mli

@@ -21,6 +21,7 @@ module Package_summary : sig
     dev_repo : string option;
     depexts : (OpamSysPkg.Set.t * OpamTypes.filter) list;
     flags : OpamTypes.package_flag list;
+    build_commands : OpamTypes.command list;
   }
 
   val pp : t Fmt.t

test/bin/invalid-package-version.t/repo/packages/a/a.0.1/opam

@@ -3,6 +3,7 @@ dev-repo: "git+https://a.com/a.git"
 depends: [
   "dune"
 ]
+build: [ "dune" "build" ]
 url {
   src: "https://a.com/a.0.1.tbz"
   checksum: [

test/bin/invalid-package-version.t/repo/packages/a/a.1.0/opam

@@ -3,6 +3,7 @@ dev-repo: "git+https://a.com/a.git"
 depends: [
   "ocaml"
 ]
+build: [ "make" ]
 url {
   src: "https://a.com/a.1.0.tbz"
   checksum: [

test/bin/invalid-package-version.t/repo/packages/a/a.1.1/opam

@@ -3,6 +3,7 @@ dev-repo: "git+https://a.com/a.git"
 depends: [
   "ocaml"
 ]
+build: [ "make" ]
 url {
   src: "https://a.com/a.1.1.tbz"
   checksum: [

test/bin/local-solver-picks-higher-version.t/repo/packages/a/a.0.2/opam

@@ -3,6 +3,7 @@ dev-repo: "git+https://a.com/a.git"
 depends: [
   "dune"
 ]
+build: [ "dune" "build" ]
 url {
   src: "https://a.com/a.0.2.tbz"
   checksum: [

test/bin/local-solver-picks-higher-version.t/run.t

@@ -42,6 +42,7 @@ solutions:
   depends: [
     "dune"
   ]
+  build: [ "dune" "build" ]
   url {
     src: "https://a.com/a.0.2.tbz"
     checksum: [

test/bin/minimal-update.t/repo/packages/a/a.0.1/opam

@@ -2,6 +2,7 @@ opam-version: "2.0"
 depends: [
   "dune"
 ]
+build: [ "dune" "build" ]
 dev-repo: "git+https://github.com/a/a"
 url {
   src: "https://a.com/a.tbz"

test/bin/minimal-update.t/repo/packages/b/b.0.1/opam

@@ -2,6 +2,7 @@ opam-version: "2.0"
 depends: [
   "dune"
 ]
+build: [ "dune" "build" ]
 dev-repo: "git+https://github.com/b/b"
 url {
   src: "https://b.com/b.tbz"

test/bin/minimal-update.t/repo/packages/c/c.0.1/opam

@@ -2,6 +2,7 @@ opam-version: "2.0"
 depends: [
   "dune"
 ]
+build: [ "dune" "build" ]
 dev-repo: "git+https://github.com/c/c"
 url {
   src: "https://c.com/c.tbz"

test/bin/no-build-command.t/repo/packages/depend-with-dune/depend-with-dune.0.1/opam

@@ -0,0 +1,12 @@
+opam-version: "2.0"
+dev-repo: "git+https://github.com/test-no-build-command/test"
+depends: [
+  "with-dune"
+]
+dev-repo: "git+https://github.com/b/b"
+url {
+  src: "https://test.com/test.tbz"
+  checksum: [
+    "sha256=0000000000000000000000000000000000000000000000000000000000000000"
+  ]
+}

test/bin/no-build-command.t/repo/packages/depend-without-dune/depend-without-dune.0.1/opam

@@ -0,0 +1,12 @@
+opam-version: "2.0"
+dev-repo: "git+https://github.com/test-no-build-command/test"
+depends: [
+  "without-dune"
+]
+dev-repo: "git+https://github.com/b/b"
+url {
+  src: "https://test.com/test.tbz"
+  checksum: [
+    "sha256=0000000000000000000000000000000000000000000000000000000000000000"
+  ]
+}

test/bin/no-build-command.t/repo/packages/with-dune/with-dune.0.1/opam

@@ -0,0 +1,12 @@
+opam-version: "2.0"
+dev-repo: "git+https://github.com/test-no-build-command/test"
+depends: [
+  "dune"
+]
+build: [ "dune" "build" ]
+url {
+  src: "https://test.com/test.tbz"
+  checksum: [
+    "sha256=0000000000000000000000000000000000000000000000000000000000000000"
+  ]
+}

test/bin/no-build-command.t/repo/packages/without-dune/without-dune.0.1/opam

@@ -0,0 +1,12 @@
+opam-version: "2.0"
+dev-repo: "git+https://github.com/test-no-build-command/test"
+depends: []
+build: [
+  ["make"]
+]
+url {
+  src: "https://test.com/test.tbz"
+  checksum: [
+    "sha256=0000000000000000000000000000000000000000000000000000000000000000"
+  ]
+}

test/bin/no-build-command.t/repo/repo

@@ -0,0 +1 @@
+opam-version: "2.0"

test/bin/no-build-command.t/run.t

@@ -0,0 +1,30 @@
+Test that packages with no build commands and no direct dependency on dune and
+no build command are not assumed to not build with dune.
+
+The repo contains 4 packages:
+- with-dune: directly depends on dune and has a build command
+- without-dune: doesn't depend on dune and has a build command
+- depend-with-dune: depends on with-dune and doesn't have a build command
+- depend-without-dune: depends on without-dune and doesn't have a build command
+
+This test asserts that it's an error to generate a lockfile for a package
+depending on depend-without-dune due to the transitive dependency on
+without-dune, but that it's not an error to generate a lockfile for a package
+which only depends on depend-with-dune, despite the latter not directly
+depending on dune.
+
+We setup the default base repository
+
+  $ gen-minimal-repo
+
+Attempt to generate a lockfile for a package which depends on
+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 2>&1 | grep -o "Doesn't build with dune"
+  Doesn't build with dune
+
+Attempt to generate a lockfile for a package which depends only on
+depend-with-dune
+
+  $ opam-monorepo lock test-depend-with-dune.opam > /dev/null

test/bin/no-build-command.t/test-depend-with-dune.opam

@@ -0,0 +1,8 @@
+opam-version: "2.0"
+depends: [
+  "depend-with-dune"
+]
+x-opam-monorepo-opam-repositories: [
+  "file://$OPAM_MONOREPO_CWD/minimal-repo"
+  "file://$OPAM_MONOREPO_CWD/repo"
+]

test/bin/no-build-command.t/test-depend-without-dune.opam

@@ -0,0 +1,8 @@
+opam-version: "2.0"
+depends: [
+  "depend-without-dune"
+]
+x-opam-monorepo-opam-repositories: [
+  "file://$OPAM_MONOREPO_CWD/minimal-repo"
+  "file://$OPAM_MONOREPO_CWD/repo"
+]

test/bin/opam-provided.t/repo/packages/b/b.1/opam

@@ -3,6 +3,7 @@ depends: [
   "dune"
 ]
 dev-repo: "git+https://github.com/b/b"
+build: [ "dune" "build" ]
 url {
   src: "https://b.com/b.tbz"
   checksum: [

test/bin/opam-provided.t/repo/packages/depends-on-b/depends-on-b.1/opam

@@ -3,6 +3,7 @@ depends: [
   "dune"
   "b"
 ]
+build: [ "dune" "build" ]
 dev-repo: "git+https://github.com/b/depends-on-b"
 url {
   src: "https://b.com/depends-on-b.tbz"

test/bin/require-cross-compiling-packages.t/mirage-opam-overlays/packages/b/b.0.1+dune+mirage/opam

@@ -3,6 +3,7 @@ dev-repo: "git+https://b.com/b.git"
 depends: [
   "dune"
 ]
+build: [ "dune" "build" ]
 tags: ["cross-compile"]
 url {
   src: "https://mirage.com/b.0.1-dune-mirage.tbz"

test/bin/require-cross-compiling-packages.t/opam-overlays/packages/b/b.0.1+dune/opam

@@ -3,6 +3,7 @@ dev-repo: "git+https://b.com/b.git"
 depends: [
   "dune"
 ]
+build: [ "dune" "build" ]
 url {
   src: "https://dune.com/b.0.1-dune.tbz"
   checksum: "sha256=0000000000000000000000000000000000000000000000000000000000000001"

test/bin/require-cross-compiling-packages.t/opam-repository/packages/b/b.0.1/opam

@@ -1,5 +1,6 @@
 opam-version: "2.0"
 dev-repo: "git+https://b.com/b.git"
+build: [ "make" ]
 url {
   src: "https://b.com/b.0.1.tbz"
   checksum: "sha256=0000000000000000000000000000000000000000000000000000000000000000"

test/bin/require-cross-compiling-packages.t/run.t

@@ -11,6 +11,7 @@ original 0.1 release. There is a 0.1+dune port of it in opam-overlays:
   depends: [
     "dune"
   ]
+  build: [ "dune" "build" ]
   url {
     src: "https://dune.com/b.0.1-dune.tbz"
     checksum: "sha256=0000000000000000000000000000000000000000000000000000000000000001"
@@ -25,6 +26,7 @@ maintainers created a 0.1+dune+mirage port in mirage-opam-overlays:
   depends: [
     "dune"
   ]
+  build: [ "dune" "build" ]
   tags: ["cross-compile"]
   url {
     src: "https://mirage.com/b.0.1-dune-mirage.tbz"
@@ -72,6 +74,7 @@ to upstream the dune port before `0.2` so the `0.2` release builds with dune.
   > depends: [
   >   "dune"
   > ]
+  > build: [ "dune" "build" ]
   > url {
   >   src: "https://b.com/b.0.2.tbz"
   >   checksum: "sha256=0000000000000000000000000000000000000000000000000000000000000003"

test/bin/simple-lock.t/repo/packages/b/b.1/opam

@@ -2,6 +2,7 @@ opam-version: "2.0"
 depends: [
   "dune"
 ]
+build: [ "dune" "build" ]
 dev-repo: "git+https://github.com/b/b"
 url {
   src: "https://b.com/b.tbz"

test/bin/simple-lock.t/repo/packages/c/c.1/opam

@@ -2,6 +2,7 @@ opam-version: "2.0"
 depends: [
   "dune"
 ]
+build: [ "dune" "build" ]
 dev-repo: "git+https://github.com/c/c"
 url {
   src: "https://c.com/c.tbz"

test/lib/test_duniverse.ml

@@ -28,14 +28,23 @@ let opam_factory ~name ~version =
   OpamPackage.create name version
 
 let summary_factory ?(name = "undefined") ?(version = "1") ?dev_repo ?url_src
-    ?(hashes = []) ?(depexts = []) ?(flags = []) () =
+    ?(hashes = []) ?(depexts = []) ?(flags = []) ?(build_commands = []) () =
   let package = opam_factory ~name ~version in
-  { Opam.Package_summary.package; dev_repo; url_src; hashes; depexts; flags }
+  {
+    Opam.Package_summary.package;
+    dev_repo;
+    url_src;
+    hashes;
+    depexts;
+    flags;
+    build_commands;
+  }
 
 let dependency_factory ?(vendored = true) ?name ?version ?dev_repo ?url_src
-    ?hashes ?depexts ?flags () =
+    ?hashes ?depexts ?flags ?build_commands () =
   let package_summary =
-    summary_factory ?name ?version ?dev_repo ?url_src ?hashes ?depexts ?flags ()
+    summary_factory ?name ?version ?dev_repo ?url_src ?hashes ?depexts ?flags
+      ?build_commands ()
   in
   { Opam.Dependency_entry.vendored; package_summary }
 
@@ -71,7 +80,9 @@ module Repo = struct
         make_test ~name:"Regular"
           ~summary:
             (summary_factory ~dev_repo:"d" ~url_src:(Other "u") ~name:"y"
-               ~version:"v" ~hashes:[] ())
+               ~version:"v" ~hashes:[]
+               ~build_commands:[ ([], None) ]
+               ())
           ~expected:
             (Ok
                (Some
@@ -88,7 +99,9 @@ module Repo = struct
           ~summary:
             (summary_factory ~dev_repo:"d"
                ~url_src:(Git { repo = "r"; ref = None })
-               ~name:"y" ~version:"v" ~hashes:[] ())
+               ~name:"y" ~version:"v" ~hashes:[]
+               ~build_commands:[ ([], None) ]
+               ())
           ~expected:
             (Ok
                (Some
@@ -234,7 +247,9 @@ let test_from_dependency_entries =
       ~dependency_entries:
         [
           dependency_factory ~name:"x" ~version:"v" ~url_src:(Other "u")
-            ~dev_repo:"d" ~hashes:[] ();
+            ~dev_repo:"d" ~hashes:[]
+            ~build_commands:[ ([], None) ]
+            ();
         ]
       ~expected:
         (Ok
@@ -258,9 +273,13 @@ let test_from_dependency_entries =
       ~dependency_entries:
         [
           dependency_factory ~name:"y" ~version:"v" ~url_src:(Other "u")
-            ~dev_repo:"d" ~hashes:[] ();
+            ~dev_repo:"d" ~hashes:[]
+            ~build_commands:[ ([], None) ]
+            ();
           dependency_factory ~name:"y-lwt" ~version:"v" ~url_src:(Other "u")
-            ~dev_repo:"d" ~hashes:[] ();
+            ~dev_repo:"d" ~hashes:[]
+            ~build_commands:[ ([], None) ]
+            ();
         ]
       ~expected:
         (Ok