Merge pull request #227 from ocamllabs/no-dune-project

Properly handle missing dune-project

CHANGES.md

@@ -19,6 +19,8 @@
 - Properly detect all opam packages defined in the current repository, preventing it
   from later pulling duplicates into the duniverse if they were part of the target packages
   dependencies. (#203, @Leonidas-from-XIV)
+- Properly report missing dune-project file when trying to determine the
+  to-be-genrated lockfile name (#227, @NathanReb)
 
 ### Removed
 

cli/lock.ml

@@ -135,8 +135,14 @@ let lockfile_path ~explicit_lockfile ~target_packages repo =
   | Some path -> Ok path
   | None ->
       Project.lockfile
-        ~local_packages:(OpamPackage.Name.Set.elements target_packages)
+        ~target_packages:(OpamPackage.Name.Set.elements target_packages)
         repo
+      |> Result.map_error ~f:(function `Msg msg ->
+             Rresult.R.msgf
+               "Could not infer the target lockfile name: %s\n\
+                Try setting it explicitly using --lockfile or add a project \
+                name in a root dune-project file."
+               msg)
 
 let root_pin_depends local_opam_files =
   OpamPackage.Name.Map.fold

dune-project

@@ -1,7 +1,9 @@
-(lang dune 2.6)
+(lang dune 2.7)
 (generate_opam_files true)
 (name opam-monorepo)
 
+(cram enable)
+
 (source (github ocamllabs/opam-monorepo))
 (license ISC)
 (authors "Anil Madhavapeddy" "Nathan Rebours" "Lucas Pluvinage" "Jules Aguillon")

lib/project.ml

@@ -37,20 +37,17 @@ let dune_project t = Fpath.(t / "dune-project")
 let name t =
   let open Result.O in
   let dune_project = dune_project t in
-  Dune_file.Raw.as_sexps dune_project >>= Dune_file.Project.name
+  Bos.OS.File.exists dune_project >>= function
+  | true -> Dune_file.Raw.as_sexps dune_project >>= Dune_file.Project.name
+  | false ->
+      Rresult.R.error_msgf "Missing dune-project file at the root: %a" Fpath.pp
+        dune_project
 
 let lockfile ~name t = Fpath.(t / (name ^ Config.lockfile_ext))
 
-let lockfile ?local_packages:lp t =
+let lockfile ~target_packages t =
   let open Result.O in
-  let local_packages =
-    match lp with
-    | Some lp -> Ok lp
-    | None -> local_packages ~recurse:false t >>| List.map ~f:fst
-  in
-
-  local_packages >>= fun names ->
-  match names with
+  match target_packages with
   | [ name ] ->
       let name = OpamPackage.Name.to_string name in
       Ok (lockfile ~name t)

lib/project.mli

@@ -24,16 +24,16 @@ val name : t -> (string, [> `Msg of string ]) result
 (** Returns the name of the project, as set in the dune-project. *)
 
 val lockfile :
-  ?local_packages:OpamPackage.Name.t list ->
+  target_packages:OpamPackage.Name.t list ->
   t ->
   (Fpath.t, [> `Msg of string ]) result
-(** Returns the path to the opam-monorepo lockfile for the given project.
-    If the repo contains a single package, then it's the ["<package_name>.opam.locked"]
+(** Returns the path to the opam-monorepo lockfile to generate for the given
+    project and lockfile target packages.
+    If there is a single target package, then it is the ["<package_name>.opam.locked"]
     file at the root of the project.
     If it contains multiple packages, then it's the ["<project_name>.opam.locked"] file
-    at the root of the project.
-    One can provide [local_packages] if they were already computed or if only a subset
-    of the local packages must be taken into account. *)
+    at the root of the project, where <project_name> is the name as defined in the
+    dune-project file. *)
 
 val local_lockfiles : t -> (Fpath.t list, Rresult.R.msg) result
 (** Returns all the lockfiles located at the root of the project i.e. all

opam-monorepo.opam

@@ -13,7 +13,7 @@ license: "ISC"
 homepage: "https://github.com/ocamllabs/opam-monorepo"
 bug-reports: "https://github.com/ocamllabs/opam-monorepo/issues"
 depends: [
-  "dune" {>= "2.6"}
+  "dune" {>= "2.7"}
   "ocaml" {>= "4.08.0"}
   "dune-build-info"
   "base"
@@ -29,6 +29,7 @@ depends: [
   "sexplib"
   "uri"
   "alcotest" {with-test}
+  "odoc" {with-doc}
 ]
 conflicts: [
   "dune-build-info" {= "2.7.0" | = "2.7.1"}

test/bin/dune

@@ -0,0 +1,3 @@
+(cram
+ (applies_to :whole_subtree)
+ (deps %{bin:opam-monorepo}))

test/bin/missing-dune-project.t/a.opam

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

test/bin/missing-dune-project.t/b.opam

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

test/bin/missing-dune-project.t/run.t

@@ -0,0 +1,19 @@
+We have a simple project with two local packages, defined at the root
+
+  $ cat a.opam
+  opam-version: "2.0"
+
+  $ cat b.opam
+  opam-version: "2.0"
+
+The project has no dune-project file. That means that if we run `opam-monorepo lock`,
+it will have more than onw target: `a` and `b`. It therefore has to determine the name of the
+lockfile based on the project's name in the dune-project file. It's expected to fail but it should
+to that nicely, letting the user know that it couldn't infer the lockfile and that they should
+either explicitly specify it on the command line or add a valid dune-project file at the root.
+
+  $ opam-monorepo lock
+  ==> Using 2 locally scanned packages as the targets.
+  opam-monorepo: Could not infer the target lockfile name: Missing dune-project file at the root: $TESTCASE_ROOT/dune-project
+                 Try setting it explicitly using --lockfile or add a project name in a root dune-project file.
+  [1]