Check that the repositories given to "opam repository remove" actually exist

master_changes.md

@@ -145,6 +145,12 @@ users)
   * Get rid of OPAM_USER_PATH_RO (never used on macOS and no longer needed on Linux) [#4795 @kit-ty-kate]
   * Print error message if command doesn't exist [#4971 @kit-ty-kat - fix #4112]
 
+## Repository
+  * Don't display global message when `this-switch` is given [#4899 @rjbou - fix #4889]
+  * Set the priority of user-set archive-mirrors higher than the repositories'.
+    This allows opam-repository to use the default opam.ocaml.org cache and be more resilient to changed/force-pushed or unavailable archives. [#4830 @kit-ty-kate - fixes #4411]
+  * Check that the repositories given to "opam repository remove" actually exist [#5014 @kit-ty-kate - fixes #5012]
+
 ## VCS
   * Pass --depth=1 to git-fetch in the Git repo backend [#4442 @dra27]
   * Use 4.08's unnamed functor arguments to silence warning 67 [#4775 @dra27]

src/client/opamCommands.ml

@@ -2198,8 +2198,9 @@ let repository cli =
             (OpamRepositoryState.drop @@ OpamRepositoryCommand.remove rt name;
              OpamConsole.error_and_exit `Sync_error
                "Initial repository fetch failed"));
-      OpamGlobalState.drop @@ OpamRepositoryCommand.update_selection gt ~global
-        ~switches (update_repos name);
+      OpamGlobalState.drop @@ fst @@
+      OpamRepositoryCommand.update_selection gt ~global ~switches
+        (update_repos name);
       if scope = [`Current_switch] then
         OpamConsole.note
           "Repository %s has been added to the selections of switch %s \
@@ -2213,10 +2214,17 @@ let repository cli =
       `Ok ()
     | Some `remove, names ->
       let names = List.map OpamRepositoryName.of_string names in
-      let rm = List.filter (fun n -> not (List.mem n names)) in
+      let rm repos =
+        let names, unknown = List.partition (fun n -> List.mem n repos) names in
+        if unknown <> [] then
+          OpamConsole.warning "Repository %s not found, ignoring"
+            (OpamStd.Format.pretty_list
+               (List.map OpamRepositoryName.to_string unknown));
+        List.filter (fun n -> not (List.mem n names)) repos
+      in
       let full_wipe = List.mem `All scope in
       let global = global || full_wipe in
-      let gt =
+      let gt, updated_switches =
         OpamRepositoryCommand.update_selection gt
           ~global ~switches:switches rm
       in
@@ -2227,19 +2235,27 @@ let repository cli =
              "No configured repositories by these names found: %s");
         OpamRepositoryState.drop @@
         List.fold_left OpamRepositoryCommand.remove rt names
-      else if scope = [`Current_switch] then
-        OpamConsole.msg
-          "Repositories removed from the selections of switch %s. \
-           Use '--all' to forget about them altogether.\n"
-          (OpamSwitch.to_string (OpamStateConfig.get_switch ()));
+      else
+        (let unchanged_switches =
+           let usw = List.map fst updated_switches in
+           List.filter (fun s -> not (List.mem s usw)) switches in
+         if unchanged_switches <> [] then
+           OpamConsole.warning "Unchanged switches %s"
+             (OpamStd.Format.pretty_list
+                (List.map OpamSwitch.to_string unchanged_switches));
+         if scope = [`Current_switch] && updated_switches <> [] then
+           OpamConsole.msg
+             "Repositories removed from the selections of switch %s. \
+              Use '--all' to forget about them altogether.\n"
+             (OpamSwitch.to_string (OpamStateConfig.get_switch ())));
       `Ok ()
     | Some `add, [name] ->
       let name = OpamRepositoryName.of_string name in
       OpamRepositoryState.with_ `Lock_none gt (fun rt ->
           check_for_repos rt [name]
             (OpamConsole.error_and_exit `Not_found
                "No configured repository '%s' found, you must specify an URL"));
-      OpamGlobalState.drop @@
+      OpamGlobalState.drop @@ fst @@
       OpamRepositoryCommand.update_selection gt ~global ~switches
         (update_repos name);
       `Ok ()
@@ -2276,7 +2292,7 @@ let repository cli =
         List.filter (fun r -> not (OpamRepositoryName.Map.mem r repos)) names
       in
       if not_found = [] then
-        (OpamGlobalState.drop @@
+        (OpamGlobalState.drop @@ fst @@
          OpamRepositoryCommand.update_selection gt ~global ~switches
            (fun _ -> names);
          `Ok ())

src/client/opamRepositoryCommand.ml

@@ -24,19 +24,23 @@ let update_global_selection gt update_fun =
   gt
 
 let update_selection gt ~global ~switches update_fun =
-  List.iter (OpamSwitchState.update_repositories gt update_fun) switches;
-  if global then
-    (* ensure all unselected switches aren't modified by changing the default *)
-    (List.iter (fun sw ->
-         if not (List.mem sw switches) then
-           OpamSwitchState.update_repositories gt (fun r -> r) sw)
-        (OpamFile.Config.installed_switches gt.config);
-     let (), gt =
-       OpamGlobalState.with_write_lock gt @@ fun gt ->
-       (), update_global_selection gt update_fun
-     in
-     gt)
-  else gt
+  let updated =
+    List.filter_map
+      (OpamSwitchState.update_repositories gt update_fun)
+      switches
+  in
+  (if global then
+     (* ensure all unselected switches aren't modified by changing the default *)
+     (List.iter (fun sw ->
+          if not (List.mem sw switches) then
+            ignore @@ OpamSwitchState.update_repositories gt (fun r -> r) sw)
+         (OpamFile.Config.installed_switches gt.config);
+      let (), gt =
+        OpamGlobalState.with_write_lock gt @@ fun gt ->
+        (), update_global_selection gt update_fun
+      in
+      gt)
+   else gt), updated
 
 let update_repos_config rt repositories =
   (* Remove cached opam files for changed or removed repos *)

src/client/opamRepositoryCommand.mli

@@ -43,7 +43,7 @@ val update_global_selection:
 val update_selection:
   'a global_state -> global:bool -> switches:switch list ->
   (repository_name list -> repository_name list) ->
-  'a global_state
+  'a global_state * (switch * OpamFile.Switch_config.t) list
 
 (** Change the registered address of a repo *)
 val set_url:

src/state/opamSwitchState.ml

@@ -1247,15 +1247,21 @@ let update_repositories gt update_fun switch =
   OpamFilename.with_flock `Lock_write (OpamPath.Switch.lock gt.root switch)
   @@ fun _ ->
   let conf = load_switch_config ~lock_kind:`Lock_write gt switch in
-  let repos =
+  let repos0 =
     match conf.OpamFile.Switch_config.repos with
     | None -> OpamGlobalState.repos_list gt
     | Some repos -> repos
   in
-  let conf =
-    { conf with
-      OpamFile.Switch_config.repos = Some (update_fun repos) }
-  in
-  OpamFile.Switch_config.write
-    (OpamPath.Switch.switch_config gt.root switch)
-    conf
+  let repos = update_fun repos0 in
+  if
+    List.sort OpamRepositoryName.compare repos
+    = List.sort OpamRepositoryName.compare repos0 then
+    None else
+    (let conf =
+       { conf with
+         OpamFile.Switch_config.repos = Some repos }
+     in
+     OpamFile.Switch_config.write
+       (OpamPath.Switch.switch_config gt.root switch)
+       conf;
+     Some (switch, conf))

src/state/opamSwitchState.mli

@@ -219,7 +219,7 @@ val update_pin: package -> OpamFile.OPAM.t -> 'a switch_state -> 'a switch_state
     repositories in any case, even if unchanged from the defaults. *)
 val update_repositories:
   'a global_state -> (repository_name list -> repository_name list) ->
-  switch -> unit
+  switch -> (switch * OpamFile.Switch_config.t) option
 
 (** {2 User interaction and reporting } *)