From f08c1563bad19d2ec8f82286e62e324ccf3fa8d3 Mon Sep 17 00:00:00 2001
From: Louis Gesbert <louis.gesbert@ocamlpro.com>
Date: Mon, 25 Mar 2019 18:23:01 +0100
Subject: [PATCH] Fix pinnings not installing/upgrading already pinned packages

---
 src/client/opamClient.ml   | 27 ++++++++++++---------------
 src/client/opamClient.mli  |  5 +++++
 src/client/opamCommands.ml | 26 +++++++++-----------------
 3 files changed, 26 insertions(+), 32 deletions(-)

diff --git a/src/client/opamClient.ml b/src/client/opamClient.ml
index 964f3237c60..40965b20de2 100644
--- a/src/client/opamClient.ml
+++ b/src/client/opamClient.ml
@@ -1267,7 +1267,16 @@ let reinstall t ?(assume_built=false) names =
 module PIN = struct
   open OpamPinCommand
 
-  let post_pin_action st names =
+  let post_pin_action st was_pinned names =
+    let names =
+      OpamPackage.Set.Op.(st.pinned -- was_pinned)
+      |> OpamPackage.names_of_packages
+      |> (fun s ->
+          List.fold_left
+            (fun s p -> OpamPackage.Name.Set.add p s)
+            s names)
+      |> OpamPackage.Name.Set.elements
+    in
     try
       upgrade_t
         ~strict_upgrade:false ~auto_install:true ~ask:true ~terse:true
@@ -1311,13 +1320,7 @@ module PIN = struct
           source_pin st name ?version ~edit (Some (get_upstream st name))
         | `None -> source_pin st name ?version ~edit None
       in
-      if action then
-        let names =
-          OpamPackage.Set.Op.(st.pinned -- pinned)
-          |> OpamPackage.Set.elements
-          |> List.map OpamPackage.name
-        in
-        (OpamConsole.msg "\n"; post_pin_action st names)
+      if action then (OpamConsole.msg "\n"; post_pin_action st pinned [name])
       else st
     with
     | OpamPinCommand.Aborted -> OpamStd.Sys.exit_because `Aborted
@@ -1357,13 +1360,7 @@ module PIN = struct
         OpamConsole.error_and_exit `Not_found
           "Package is not pinned, and no existing version was supplied."
     in
-    if action then
-      let names =
-        OpamPackage.Set.Op.(st.pinned -- pinned)
-        |> OpamPackage.Set.elements
-        |> List.map OpamPackage.name
-      in
-      post_pin_action st names
+    if action then post_pin_action st pinned [name]
     else st
 
   let unpin st ?(action=true) names =
diff --git a/src/client/opamClient.mli b/src/client/opamClient.mli
index 3b087f5f092..26e25881770 100644
--- a/src/client/opamClient.mli
+++ b/src/client/opamClient.mli
@@ -123,4 +123,9 @@ module PIN: sig
   (** List the current pinned packages. *)
   val list: 'a switch_state -> short:bool -> unit
 
+  (** Runs an install/upgrade on the listed packages if necessary.
+      [post_pin_action st was_pinned names] takes the set of packages pinned
+      beforehand, and a list of newly pinned packages *)
+  val post_pin_action: rw switch_state -> package_set -> name list -> rw switch_state
+
 end
diff --git a/src/client/opamCommands.ml b/src/client/opamCommands.ml
index 7e931ef5589..a578ca6dc11 100644
--- a/src/client/opamCommands.ml
+++ b/src/client/opamCommands.ml
@@ -2566,26 +2566,18 @@ let pin ?(unpin_only=false) () =
                      OpamPath.Switch.Overlay.tmp_opam
                        st.switch_global.root st.switch name
                    in
-                   if not (OpamFilename.exists (OpamFile.filename opam_localf)) then
-                     OpamFile.OPAM.write opam_localf opam) opam_opt;
-               try OpamPinCommand.source_pin st name ~edit (Some url)
-               with OpamPinCommand.Aborted -> OpamStd.Sys.exit_because `Aborted
-                  | OpamPinCommand.Nothing_to_do -> st)
+                   if not (OpamFilename.exists (OpamFile.filename opam_localf))
+                   then OpamFile.OPAM.write opam_localf opam)
+                 opam_opt;
+               try OpamPinCommand.source_pin st name ~edit (Some url) with
+               | OpamPinCommand.Aborted -> OpamStd.Sys.exit_because `Aborted
+               | OpamPinCommand.Nothing_to_do -> st)
              st names
          in
-         (* names are in newly pinned packages *)
-         let atoms =
-           OpamPackage.Set.Op.(st.pinned -- pinned)
-           |> OpamPackage.Set.elements
-           |> List.map (fun p -> (OpamPackage.name p, None))
-         in
          if action then
-           let _st =
-             OpamClient.upgrade_t
-               ~strict_upgrade:false ~auto_install:true ~ask:true ~all:false
-               atoms st
-           in
-           `Ok ()
+           (OpamSwitchState.drop @@
+            OpamClient.PIN.post_pin_action st pinned (List.map fst names);
+            `Ok ())
          else `Ok ())
     | Some `add, [n; target] | Some `default n, [target] ->
       (match (fst package) n with