Merge pull request #46 from moyodiallo/dune-external-libs

Fix for "dune external-lib-deps" command

CHANGES.md

@@ -1,3 +1,8 @@
+### Unreleased
+
+- Add support for dune 3.0 , the command `dune external-lib-deps` was removed from
+  dune. Now, the `opam-dune-lint` command works without `dune build`. (@moyodiallo #46).
+
 ### v0.2
 
 - Cope with missing `(depends ...)` in `dune-project` (@talex5 #33). We tried to add the missing packages to an existing depends field, but if there wasn't one at all then we did nothing.

README.md

@@ -29,14 +29,15 @@ Wrote "dune-project"
 It works as follows:
 
 1. Lists the `*.opam` files in your project's root (ensuring they're up-to-date, if generated).
-2. Runs `dune external-lib-deps --only-packages $PKG --unstable-by-dir @install` and `... @runtest` to get each package's ocamlfind dependencies.
-3. Filters out local dependencies using `dune describe` (for now; would be good to lint these too in future, but needs a different code path).
-4. Filters out vendored dependencies (by ignoring dependencies from subdirectories with their own `dune-project` file).
-5. For each ocamlfind library, it finds the corresponding opam library by
+2. Runs `dune describe external-lib-deps` to get all externals and internals ocamlfind libraries for all dune libraries, executables and tests. The information about the package is also known except for the private executables.
+3. Runs `dune describe package-entries` to get all packages entries, this is for considering the external ocamlfind libraries of a private executable, because in Dune it is possible to install a private executable.
+4. Resolve for each opam library its internal and external ocamlfind library dependencies using the information of 1. and 2.
+5. Filters out vendored dependencies (by ignoring dependencies from subdirectories with their own `dune-project` file).
+6. For each ocamlfind library, it finds the corresponding opam library by
    finding its directory and then finding the `*.changes` file saying which
    opam package added its `META` file.
-6. Checks that each required opam package is listed in the opam file.
-7. For any missing packages, it offers to add a suitable dependency, using the installed package's version as the default lower-bound.
+7. Checks that each required opam package is listed in the opam file.
+8. For any missing packages, it offers to add a suitable dependency, using the installed package's version as the default lower-bound.
 
 `opam-dune-lint` can be run manually to update your project, or as part of CI to check for missing dependencies.
 It exits with a non-zero status if changes are needed, or if the opam files were not up-to-date with the `dune-project` file.

deps.ml

@@ -0,0 +1,184 @@
+open Types
+open Dune_items
+
+type t = Dir_set.t Libraries.t
+
+let dune_describe_external_lib_deps () = Bos.Cmd.(v "dune" % "describe" % "external-lib-deps")
+
+let dune_describe_entries () = Bos.Cmd.(v "dune" % "describe" % "package-entries")
+
+let describe_external_lib_deps =
+  Lazy.from_fun (fun _ ->
+      sexp @@ dune_describe_external_lib_deps ()
+      |> Describe_external_lib.describe_extern_of_sexp)
+
+let describe_bin_of_entries =
+  Lazy.from_fun (fun _ ->
+      sexp @@ dune_describe_entries ()
+      |> Describe_entries.entries_of_sexp
+      |> Describe_entries.items_bin_of_entries)
+
+let has_dune_subproject path =
+  if Fpath.is_current_dir path then false
+  else
+    Fpath.(path / "dune-project")
+    |> Bos.OS.Path.exists
+    |> Stdlib.Result.get_ok
+
+
+let parent_path path = if Fpath.is_current_dir path then None else Some (Fpath.parent path)
+
+let rec should_use_dir ~dir_types path =
+  match Hashtbl.find_opt dir_types path with
+  | Some x -> x
+  | None ->
+    let r =
+      match parent_path path with
+      | Some parent ->
+        if should_use_dir ~dir_types parent then (
+          not (has_dune_subproject path)
+        ) else false
+      | None ->
+        not (has_dune_subproject path)
+    in
+    Hashtbl.add dir_types path r;
+    r
+
+let copy_rules () =
+  Lazy.force describe_external_lib_deps
+  |> List.concat_map
+    (fun d_item ->
+       d_item
+       |> Describe_external_lib.get_item
+       |> (fun (item:Describe_external_lib.item) -> Fpath.(item.source_dir / "dune"))
+       |> Dune_rules.Copy_rules.get_copy_rules)
+  |> Dune_rules.Copy_rules.copy_rules_map
+
+let bin_of_entries () = Lazy.force describe_bin_of_entries
+
+let is_bin_name_of_describe_lib bin_name (item:Describe_external_lib.item) =
+  item.extensions
+  |> List.exists (fun extension ->
+      String.equal bin_name (String.cat item.name extension))
+
+let find_package_of_exe (item:Describe_external_lib.item) =
+  match item.package with
+  | Some p -> Some p
+  | None ->
+    (* Only allow for private executables to find the package *)
+    item.extensions
+    |> List.find_map (fun extension ->
+        Option.map
+          (fun bin_name ->
+             Option.map
+               (fun (item:Describe_entries.item) -> item.package) (Item_map.find_opt bin_name @@ bin_of_entries ()))
+          (Dune_rules.Copy_rules.find_dest_name ~name:(String.cat item.name extension) @@ copy_rules ()))
+    |> Option.join
+
+let resolve_internal_deps d_items items_pkg =
+  (* After the d_items are filtered to the corresponding package request,
+   * we need to include the internal_deps in order to reach all the deps.
+   * If the internal dep is a public library we skip the recursive resolve
+   * because it will be resolve with separate request *)
+  let open Describe_external_lib in
+  let get_name = function
+    | Lib item  -> String.cat item.name ".lib"
+    | Exe item  -> String.cat item.name ".exe"
+    | Test item -> String.cat item.name ".test"
+  in
+  let d_items_lib =
+    d_items
+    |> List.filter_map (fun d_item ->
+        if is_lib_item d_item then
+          let item = get_item d_item in
+          Some (item.Describe_external_lib.name ^ ".lib", Lib item)
+        else None)
+    |> List.to_seq |> Hashtbl.of_seq
+  in
+  let rec add_internal acc = function
+    | [] -> Hashtbl.to_seq_values acc |> List.of_seq
+    | item::tl ->
+      if Hashtbl.mem acc (get_name item) then
+        add_internal acc tl
+      else begin
+        Hashtbl.add acc (get_name item) item;
+        (get_item item).internal_deps
+        |> List.filter_map (fun (name, k) ->
+            match Hashtbl.find_opt d_items_lib (String.cat name ".lib") with
+            | None -> None
+            | Some d_item_lib ->
+              if Kind.is_required k && Option.is_some (get_item d_item_lib).package then None
+              else Some d_item_lib)
+        |> fun internals -> add_internal acc (tl @ internals)
+      end
+  in
+  add_internal (Hashtbl.create 10) items_pkg
+
+let get_dune_items dir_types ~pkg ~target =
+  let d_items =
+    Lazy.force describe_external_lib_deps
+    |> List.map (fun d_item ->
+        let item = Describe_external_lib.get_item d_item in
+        if Describe_external_lib.is_exe_item d_item && Option.is_none item.package
+        then
+          match find_package_of_exe item with
+          | None -> d_item
+          | Some pkg -> Describe_external_lib.Exe { item with package = Some pkg }
+        else d_item)
+  in
+  let unresolved_entries =
+      let exe_items =
+        List.filter_map (function
+            | Describe_external_lib.Exe item -> Some item
+            | _ -> None) d_items
+      in
+        bin_of_entries ()
+        |> Item_map.partition (fun _ (entry:Describe_entries.item) ->
+            exe_items
+            |> List.exists
+              (fun (item:Describe_external_lib.item) ->
+                 is_bin_name_of_describe_lib  entry.bin_name item
+                 && Option.equal String.equal (Some entry.package) item.package))
+        |> snd
+  in
+  let d_items =
+    d_items
+    |> List.map (function
+        | Describe_external_lib.Exe item as d_item ->
+          item.extensions
+          |> List.find_map (fun extension ->
+              Item_map.find_opt (String.cat item.name extension) unresolved_entries)
+          |> (function
+              | None -> d_item
+              | Some entry -> Describe_external_lib.Exe { item with package = Some entry.package })
+        | d_item -> d_item)
+    |> List.filter (fun item ->
+        match (item,target) with
+        | Describe_external_lib.Test _, `Install -> false
+        | Describe_external_lib.Test _, `Runtest -> true
+        | _ , `Runtest -> false
+        | _, `Install -> true)
+    |> List.filter (fun d_item -> should_use_dir ~dir_types (Describe_external_lib.get_item d_item).source_dir)
+  in
+  List.filter (fun d_item ->
+      let item = Describe_external_lib.get_item d_item in
+      (* if an item has no package, we assume it's used for testing *)
+      if target = `Install then
+        Option.equal String.equal (Some pkg) item.package
+      else
+        Option.equal String.equal (Some pkg) item.package || Option.is_none item.package) d_items
+  |> resolve_internal_deps d_items
+
+let lib_deps ~pkg ~target =
+  get_dune_items (Hashtbl.create 10) ~pkg ~target
+  |> List.fold_left (fun libs item ->
+      let item = Describe_external_lib.get_item item in
+      List.map (fun dep -> fst dep, item.source_dir) item.external_deps
+      |> List.fold_left (fun acc (lib, path) ->
+          if Astring.String.take ~sat:((<>) '.') lib <> pkg then
+            let dirs = Libraries.find_opt lib acc |> Option.value ~default:Dir_set.empty in
+            Libraries.add lib (Dir_set.add path dirs) acc
+          else
+            acc) libs) Libraries.empty
+
+let get_external_lib_deps ~pkg ~target : t = lib_deps ~pkg ~target

dune

@@ -1,4 +1,4 @@
 (executable
  (public_name opam-dune-lint)
  (name main)
- (libraries astring fmt fmt.tty bos opam-format opam-state dune-private-libs.dune-lang cmdliner sexplib))
+ (libraries astring fmt fmt.tty bos opam-format opam-state cmdliner stdune sexplib str fpath))

dune-project

@@ -1,25 +1,44 @@
-(lang dune 2.7)
+(lang dune 3.10)
+
 (name opam-dune-lint)
+
 (formatting disabled)
+
 (generate_opam_files true)
-(source (github ocurrent/opam-dune-lint))
+
+(source
+ (github ocurrent/opam-dune-lint))
+
 (authors "talex5@gmail.com")
+
 (maintainers "alpha@tarides.com" "Tim McGilchrist <timmcgil@gmail.com>")
+
 (license ISC)
+
 (cram enable)
+
 (package
  (name opam-dune-lint)
  (synopsis "Ensure dune and opam dependencies are consistent")
  (description
   "opam-dune-lint checks that all ocamlfind libraries listed as dune dependencies have corresponding opam dependencies listed in the opam files. If not, it offers to add them (either to your opam files, or to your dune-project if you're generating your opam files from that).")
  (depends
-  (astring (>= 0.8.5))
-  (sexplib (>= v0.14.0))
-  (cmdliner (>= 1.1.0))
-  (dune-private-libs (and (>= 2.8.0) (< 3.0)))
-  (dune (< 3.0))
-  (ocaml (>= 4.10.0))
-  (bos (>= 0.2.0))
-  (fmt (>= 0.8.9))
-  (opam-state (>= 2.0.9))
+  (fpath
+   (>= 0.7.3))
+  (astring
+   (>= 0.8.5))
+  (sexplib
+   (>= v0.14.0))
+  (cmdliner
+   (>= 1.1.0))
+  (stdune
+   (>= 3.10.0))
+  (ocaml
+   (>= 4.08.0))
+  (bos
+   (>= 0.2.0))
+  (fmt
+   (>= 0.8.9))
+  (opam-state
+   (>= 2.1.0))
   opam-format))

dune_constraints.ml

@@ -0,0 +1,82 @@
+(* This source code is comming from https://github.com/ocurrent/opam-repo-ci/blob/master/lib/lint.ml
+ * with a slight modification *)
+
+type error =
+  | DuneConstraintMissing
+  | DuneIsBuild
+  | BadDuneConstraint of string * string
+
+let is_dune name =
+    OpamPackage.Name.equal name (OpamPackage.Name.of_string "dune")
+
+let get_dune_constraint opam =
+  let get_max = function
+    | None, None -> None
+    | Some x, None -> Some x
+    | None, Some x -> Some x
+    | Some x, Some y when OpamVersionCompare.compare x y >= 0 -> Some x
+    | Some _, Some y -> Some y
+  in
+  let get_min = function
+    | None, None | Some _, None | None, Some _ -> None
+    | Some x, Some y when OpamVersionCompare.compare x y >= 0 -> Some y
+    | Some x, Some _ -> Some x
+  in
+  let is_build = ref false in
+  let rec get_lower_bound = function
+    | OpamFormula.Atom (OpamTypes.Constraint ((`Gt | `Geq), OpamTypes.FString version)) -> Some version
+    | Atom (Filter (FIdent (_, var, _))) when String.equal (OpamVariable.to_string var) "build" -> is_build := true; None (* TODO: remove this hack *)
+    | Empty | Atom (Filter _) | Atom (Constraint _) -> None
+    | Block x -> get_lower_bound x
+    | And (x, y) -> get_max (get_lower_bound x, get_lower_bound y)
+    | Or (x, y) -> get_min (get_lower_bound x, get_lower_bound y)
+  in
+  let rec aux = function
+    | OpamFormula.Atom (pkg, constr) ->
+      if is_dune pkg then
+        let v = get_lower_bound constr in
+        Some (Option.value ~default:"1.0" v)
+      else
+        None
+    | Empty -> None
+    | Block x -> aux x
+    | And (x, y) -> get_max (aux x, aux y)
+    | Or (x, y) -> get_min (aux x, aux y)
+  in
+  (!is_build, aux opam.OpamFile.OPAM.depends)
+
+let check_dune_constraints ~errors ~dune_version pkg_name opam =
+  let is_build, dune_constraint = get_dune_constraint opam in
+  let errors =
+    match dune_constraint with
+    | None ->
+      if is_dune pkg_name then
+        errors
+      else
+        (pkg_name, DuneConstraintMissing) :: errors
+    | Some dep ->
+      if OpamVersionCompare.compare dep dune_version >= 0 then
+        errors
+      else
+        (pkg_name, BadDuneConstraint (dep, dune_version)) :: errors
+  in
+  if is_build then (pkg_name, DuneIsBuild) :: errors else errors
+
+let print_msg_of_errors =
+    List.iter (fun (package, err) ->
+      let pkg = OpamPackage.Name.to_string package in
+      match err with
+      | DuneConstraintMissing ->
+          Fmt.epr "Warning in %s: The package has a dune-project file but no explicit dependency on dune was found.@." pkg
+      | DuneIsBuild ->
+          Fmt.epr "Warning in %s: The package tagged dune as a build dependency. \
+                   Due to a bug in dune (https://github.com/ocaml/dune/issues/2147) this should never be the case. \
+                   Please remove the {build} tag from its filter.@."
+            pkg
+      | BadDuneConstraint (dep, ver) ->
+          Fmt.failwith
+            "Error in %s: Your dune-project file indicates that this package requires at least dune %s \
+             but your opam file only requires dune >= %s. Please check which requirement is the right one, and fix the other."
+            pkg ver dep
+
+    )