Merge pull request #5325 from kit-ty-kate/unpin-version

Allow opam pin remove to take a package as argument

master_changes.md

@@ -99,6 +99,7 @@ users)
   * [BUG] Fix origin opam file retrieval when opam originate from locked file [#5079 @rjbou - fix #4936]
   * [BUG] When reinstalling a package that has a dirty source, if uncommitted changes are the same than the ones stored in opam's cache, opam consider that it is up to date and nothing is updated [4879 @rjbou]
   * [BUG] Handle external dependencies when updating switch state pin status (all pins), instead as a post pin action (only when called with `opam pin` [#5047 @rjbou - fix #5046]
+  * Allow opam pin remove to take a package (<pkg>.<version>) as argument [#5325 @kit-ty-kate]
 
 ## List
   * Some optimisations to 'opam list --installable' queries combined with other filters [#4882 @altgr - fix #4311]
@@ -360,6 +361,7 @@ users)
   * Add a reftest testing for system package manager failure [#5257 @kit-ty-kate]
   * Add autopin test including deps-only, dev-deps, depexts; instrument depext handling to allow depext reftesting [#5236 @AltGr]
   * Add test for init configuration with opamrc [#5315 @rjbou]
+  * Test opam pin remove <pkg>.<version> [#5325 @kit-ty-kate]
 
 ### Engine
   * Add `opam-cat` to normalise opam file printing [#4763 @rjbou @dra27] [2.1.0~rc2 #4715]
@@ -498,6 +500,7 @@ users)
   * `OpamSysInteract.install_packages_command`: change return type to `(['AsAdmin of string | 'AsUser of string] * string list) list
 ` [#5268 @kit-ty-kate]
   * `OpamUpdate`: change `repository` output to update function option, to not write cache and new repo config if nothing changed in `repositories` [#5146 @rjbou]
+  * Add `OpamPinned.version_opt` [#5325 @kit-ty-kate]
 
   * Add optional argument `?env:(variable_contents option Lazy.t * string) OpamVariable.Map.t` to `OpamSysPoll` and `OpamSysInteract` functions. It is used to get syspolling variables from the environment first. [#4892 @rjbou]
 ## opam-solver

src/client/opamCommands.ml

@@ -3036,8 +3036,8 @@ let pin ?(unpin_only=false) cli =
      For source pinnings, the package version may be specified by using the \
      format $(i,NAME).$(i,VERSION) for $(i,PACKAGE), in the source opam file, \
      or with $(b,edit).";
-    cli_original, "remove", `remove, ["NAMES...|TARGET"],
-    "Unpins packages $(i,NAMES), restoring their definition from the \
+    cli_original, "remove", `remove, ["PACKAGES...|TARGET"],
+    "Unpins packages $(i,PACKAGES), restoring their definition from the \
      repository, if any. With a $(i,TARGET), unpins everything that is \
      currently pinned to that target.";
     cli_original, "edit", `edit, ["NAME"],
@@ -3363,8 +3363,15 @@ let pin ?(unpin_only=false) cli =
             match as_url with
             | Some ((_::_) as url) -> err, url @ acc
             | _->
-              match (fst package_name) arg with
-              | `Ok name -> err, name::acc
+              match (fst package) arg with
+              | `Ok (name, None) -> err, name::acc
+              | `Ok (name, Some version) ->
+                (match OpamPinned.version_opt st name with
+                 | Some v when not (OpamPackage.Version.equal v version) ->
+                   OpamConsole.error "%s is pinned but not to version %s. Skipping."
+                     (OpamPackage.Name.to_string name) (OpamPackage.Version.to_string version);
+                   true, acc
+                 | Some _ | None -> err, name::acc)
               | `Error _ ->
                 OpamConsole.error
                   "No package pinned to this target found, or invalid package \

src/state/opamPinned.ml

@@ -22,6 +22,8 @@ let package_opt st name = try Some (package st name) with Not_found -> None
 
 let version st name = (package st name).version
 
+let version_opt st name = try Some (version st name) with Not_found -> None
+
 let packages st = st.pinned
 
 let possible_definition_filenames dir name = [

src/state/opamPinned.mli

@@ -18,6 +18,10 @@ open OpamStateTypes
     @raise Not_found when appropriate *)
 val version: 'a switch_state -> name -> version
 
+(** If the package is pinned, returns its version.
+    Otherwise returns [None]. *)
+val version_opt: 'a switch_state -> name -> version option
+
 (** Returns the package with the pinned-to version from a pinned package name.
     @raise Not_found when appropriate *)
 val package: 'a switch_state -> name -> package

tests/reftests/pin.test

@@ -175,3 +175,14 @@ echo "another-inexistant" "inexistant"
 -> installed qux.dev
 -> installed bar.dev
 Done.
+### : Test opam pin remove <pkg>.<version>
+### opam pin
+bar.dev    rsync             file://${BASEDIR}/bar
+foo.1      local definition
+nip.dev    rsync             file://${BASEDIR}/nip
+qux.dev    rsync             file://${BASEDIR}/qux
+### opam pin remove --no-action bar.dev
+Ok, bar is no longer pinned to file://${BASEDIR}/bar (version dev)
+### opam pin remove --no-action nip.wrong-version
+[ERROR] nip is pinned but not to version wrong-version. Skipping.
+# Return code 2 #