Update following Marek's review

lib/duniverse.ml

@@ -142,21 +142,18 @@ module Repo = struct
       | Git { repo; ref } -> Printf.sprintf "%s#%s" repo ref
       | Other s -> s
     in
-    let pp_package fmt { Package.opam = { name; version }; url; _ } =
-      Format.fprintf fmt "%a.%a: %s" Opam.Pp.package_name name Opam.Pp.version
-        version (url_to_string url)
+    let pp_package fmt { Package.opam; url; _ } =
+      Fmt.pf fmt "%a: %s" Opam.Pp.package opam (url_to_string url)
     in
-    let sep fmt () = Format.fprintf fmt "\n" 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)
-          (Fmt.list ~sep pp_package) packages
-          Fmt.(list ~sep pp_package)
-          pinned_packages)
+          pp_packages packages pp_packages pinned_packages)
 
   let from_packages ~dev_repo (packages : Package.t list) =
     let open Result.O in
@@ -186,17 +183,10 @@ module Repo = struct
                %!"
               Fmt.Dump.(list (pair (Url.pp string) (list pp_hash)))
               urls
-        | pinned :: pinneds ->
-            if
-              not
-                (List.for_all pinneds ~f:(fun p ->
-                     String.equal pinned.Package.dev_repo p.Package.dev_repo
-                     && (* not necessary? *)
-                     Url.equal Git.Ref.equal pinned.url p.url))
-            then failwith "multiple pinned packages for same dir";
-            log_url_selection ~dev_repo ~packages (pinned :: pinneds);
-            let url = pinned.url in
-            let hashes = pinned.hashes in
+        | 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' =

lib/opam.ml

@@ -216,7 +216,7 @@ module Package_summary = struct
     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;@ pinned = %b;@ flags = %a;@ \
+       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)

lib/opam_solve.ml

@@ -844,23 +844,24 @@ struct
       =
     let env varname = String.Map.find_opt varname env in
     let pins = OpamPackage.Set.of_list pins in
+    let pkg_of_opam pkg =
+      let name = OpamFile.OPAM.name pkg in
+      let version = OpamFile.OPAM.version pkg in
+      OpamPackage.create name version
+    in
     (* remove pinned packages from the universe -- as that's what's the
        opam solver is doing. *)
-    let pp_pkg ppf pkg =
-      Fmt.pf ppf "%s.%s"
-        (OpamPackage.Name.to_string (OpamFile.OPAM.name pkg))
-        (OpamPackage.Version.to_string (OpamFile.OPAM.version pkg))
-    in
     let pkgs =
-      List.filter pkgs ~f:(fun pkg ->
+      List.filter pkgs ~f:(fun opam ->
+          let pkg = pkg_of_opam opam in
           let keep =
             match
-              OpamPackage.package_of_name_opt pins (OpamFile.OPAM.name pkg)
+              OpamPackage.package_of_name_opt pins (OpamPackage.name pkg)
             with
             | None -> true
-            | Some pin -> OpamFile.OPAM.version pkg = OpamPackage.version pin
+            | Some pin -> OpamPackage.version pkg = OpamPackage.version pin
           in
-          Logs.debug (fun l -> l "keep %a = %b" pp_pkg pkg keep);
+          Logs.debug (fun l -> l "keep %a = %b" Opam.Pp.package pkg keep);
           keep)
     in
     { pkgs; constraints; test; env; pins }

test/lib/test_duniverse.ml

@@ -74,18 +74,18 @@ module Repo = struct
         summary_factory ~dev_repo:"d" ~url_src:(Other "u") ~name:"y"
           ~version:"v" ~hashes:[] ?has_build_commands ?has_install_commands
       in
-      let simple_package =
-        Ok
-          (Some
-             Duniverse.Repo.Package.
-               {
-                 opam = opam_factory ~name:"y" ~version:"v";
-                 dev_repo = "d";
-                 url = Other "u";
-                 hashes = [];
-                 pinned = false;
-               })
+      let pkg =
+        Duniverse.Repo.Package.
+          {
+            opam = opam_factory ~name:"y" ~version:"v";
+            dev_repo = "d";
+            url = Other "u";
+            hashes = [];
+            pinned = false;
+          }
       in
+      let simple_package = Ok (Some pkg) in
+      let pinned_package = Ok (Some { pkg with pinned = true }) in
       [
         make_test ~name:"Base package"
           ~summary:(summary_factory ~name:"dune" ())
@@ -99,6 +99,9 @@ module Repo = struct
         make_test ~name:"Regular"
           ~summary:(simple_summary ~has_build_commands:true ())
           ~expected:simple_package ();
+        make_test ~name:"pinned"
+          ~summary:(simple_summary ~has_build_commands:true ~pinned:true ())
+          ~expected:pinned_package ();
         make_test ~name:"Has only install commands"
           ~summary:(simple_summary ~has_install_commands:true ())
           ~expected:simple_package ();