Merge pull request #398 from samoht/do-not-add-conflicts-for-pinned-p…

…ackages

Ignore pinned packages when building dev-repo conflicts

CHANGES.md

@@ -7,6 +7,9 @@
 ### Deprecated
 
 ### Fixed
+- Fix support for pinned packages. In that case, it is not necessary to add
+  dev-repo conflicts as `opam-monorepo` will always use the pinned repository.
+  (#398, #353, @samoht, @reynir, reported by @emillon)
 
 ### Removed
 

lib/duniverse.ml

@@ -67,19 +67,21 @@ module Repo = struct
       dev_repo : Dev_repo.t;
       url : unresolved Url.t;
       hashes : OpamHash.t list;
+      pinned : bool;
     }
 
     let equal t t' =
       OpamPackage.equal t.opam t'.opam
       && Dev_repo.equal t.dev_repo t'.dev_repo
       && Url.equal Git.Ref.equal t.url t'.url
 
-    let pp fmt { opam; dev_repo; url; hashes } =
+    let pp fmt { opam; dev_repo; url; hashes; pinned } =
       let open Pp_combinators.Ocaml in
       Format.fprintf fmt
-        "@[<hov 2>{ opam = %a;@ dev_repo = %a;@ url = %a;@ hashes = %a }@]"
+        "@[<hov 2>{ opam = %a;@ dev_repo = %a;@ url = %a;@ hashes = %a;@ \
+         pinned = %b; }@]"
         Opam.Pp.raw_package opam string dev_repo (Url.pp Git.Ref.pp) url
-        (list Opam.Pp.hash) hashes
+        (list Opam.Pp.hash) hashes pinned
 
     let from_package_summary ~get_default_branch ps =
       let open Opam.Package_summary in
@@ -101,10 +103,11 @@ module Repo = struct
            package;
            dev_repo = Some dev_repo;
            hashes;
+           pinned;
            _;
           } ->
               let* url = url url_src in
-              Ok (Some { opam = package; dev_repo; url; hashes })
+              Ok (Some { opam = package; dev_repo; url; hashes; pinned })
           | { dev_repo = None; package; _ } ->
               Logs.warn (fun l ->
                   l
@@ -113,6 +116,8 @@ module Repo = struct
                     Opam.Pp.package package);
               Ok None
           | _ -> Ok None)
+
+    let is_pinned { pinned; _ } = pinned
   end
 
   type 'ref t = {
@@ -132,6 +137,24 @@ module Repo = struct
     Dev_repo.repo_name dev_repo
     |> Base.Result.map ~f:(function "dune" -> "dune_" | name -> name)
 
+  let log_url_selection ~dev_repo ~packages pinned_packages =
+    let url_to_string : unresolved Url.t -> string = function
+      | Git { repo; ref } -> Printf.sprintf "%s#%s" repo ref
+      | Other s -> s
+    in
+    let pp_package fmt { Package.opam; url; _ } =
+      Fmt.pf fmt "%a: %s" Opam.Pp.package opam (url_to_string url)
+    in
+    let pp_packages = Fmt.(list ~sep:(any "\n") pp_package) in
+    Logs.warn (fun l ->
+        l
+          "The following packages come from the same repository %s but are \
+           associated with different URLs:\n\
+           %a\n\
+           The URL for the pinned package(s) was selected: %a"
+          (Dev_repo.to_string dev_repo)
+          pp_packages packages pp_packages pinned_packages)
+
   let from_packages ~dev_repo (packages : Package.t list) =
     let open Result.O in
     let provided_packages = List.map packages ~f:(fun p -> p.Package.opam) in
@@ -145,19 +168,26 @@ module Repo = struct
     in
     match urls with
     | [ (url, hashes) ] -> Ok { dir; url; hashes; provided_packages }
-    | _ ->
-        let pp_hash = Fmt.of_to_string OpamHash.to_string in
-        (* this should not happen because we passed extra constraints
-           to the opam solver to avoid this situation *)
-        Fmt.failwith
-          "The following packages have the same `dev-repo' but are using  \
-           different versions of the archive tarballs:\n\
-           %a\n\
-           This should not happen, please report the issue to \
-           https://github.com/tarides/opam-monorepo.\n\
-           %!"
-          Fmt.Dump.(list (pair (Url.pp string) (list pp_hash)))
-          urls
+    | _ -> (
+        match List.filter ~f:Package.is_pinned packages with
+        | [] ->
+            let pp_hash = Fmt.of_to_string OpamHash.to_string in
+            (* this should not happen because we passed extra constraints
+                 to the opam solver to avoid this situation *)
+            Fmt.failwith
+              "The following packages have the same `dev-repo' but are using  \
+               different versions of the archive tarballs:\n\
+               %a\n\
+               This should not happen, please report the issue to \
+               https://github.com/tarides/opam-monorepo.\n\
+               %!"
+              Fmt.Dump.(list (pair (Url.pp string) (list pp_hash)))
+              urls
+        | first_pin :: _ as pins ->
+            log_url_selection ~dev_repo ~packages pins;
+            let url = first_pin.url in
+            let hashes = first_pin.hashes in
+            Ok { dir; url; hashes; provided_packages })
 
   let equal equal_ref t t' =
     let { dir; url; hashes; provided_packages } = t in

lib/duniverse.mli

@@ -37,6 +37,7 @@ module Repo : sig
       dev_repo : string;
       url : unresolved Url.t;
       hashes : OpamHash.t list;
+      pinned : bool;
     }
 
     val equal : t -> t -> bool

lib/opam.ml

@@ -195,6 +195,7 @@ module Package_summary = struct
     hashes : OpamHash.t list;
     dev_repo : string option;
     depexts : (OpamSysPkg.Set.t * OpamTypes.filter) list;
+    pinned : bool;
     flags : Package_flag.t list;
     has_build_commands : bool;
     has_install_commands : bool;
@@ -207,23 +208,24 @@ module Package_summary = struct
         hashes;
         dev_repo;
         depexts;
+        pinned;
         flags;
         has_build_commands;
         has_install_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;@ has_build_commands = %B;@ \
-       has_install_commands = %B}@]"
+       dev_repo = %a;@ depexts = %a;@ pinned = %B;@ flags = %a;@ \
+       has_build_commands = %B;@ has_install_commands = %B}@]"
       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 pinned (list Package_flag.pp) flags
       has_build_commands has_install_commands
 
-  let from_opam package opam_file =
+  let from_opam package ~pinned opam_file =
     let url_field = OpamFile.OPAM.url opam_file in
     let url_src = Base.Option.map ~f:Url.from_opam_field url_field in
     let hashes =
@@ -245,6 +247,7 @@ module Package_summary = struct
       hashes;
       dev_repo;
       depexts;
+      pinned;
       flags;
       has_build_commands;
       has_install_commands;

lib/opam.mli

@@ -20,13 +20,14 @@ module Package_summary : sig
     hashes : OpamHash.t list;
     dev_repo : string option;
     depexts : (OpamSysPkg.Set.t * OpamTypes.filter) list;
+    pinned : bool;
     flags : OpamTypes.package_flag list;
     has_build_commands : bool;
     has_install_commands : bool;
   }
 
   val pp : t Fmt.t
-  val from_opam : OpamPackage.t -> OpamFile.OPAM.t -> t
+  val from_opam : OpamPackage.t -> pinned:bool -> OpamFile.OPAM.t -> t
 
   val is_safe_package : t -> bool
   (** A package is safe when it is clear that it can be added to the lockfile