Delete plugin links on root upgrade

master_changes.md

@@ -141,6 +141,7 @@ New option/command/subcommand are prefixed with ◈.
   * Externalise cli versioning tools from `OpamArg` into `OpamArgTools` [#4606 @rjbou]
   * Each library defines its own environment variables, that fills the config record [#4606 @rjbou]
   * Harden cygpath wrapper [#4625 @dra27]
+  * Reset the plugin symlinks when the root is upgraded [#4641 @dra27 - partial fix for #4619]
 
 ## Test
   * Make the reference tests dune-friendly [#4376 @emillon]

src/client/opamCliMain.ml

@@ -136,7 +136,24 @@ let check_and_run_external_commands () =
     OpamCoreConfig.init ~answer ();
     OpamFormatConfig.init ();
     let root_dir = OpamStateConfig.opamroot () in
-    let has_init = OpamStateConfig.load_defaults root_dir <> None in
+    let has_init, root_upgraded =
+      match OpamStateConfig.load_defaults root_dir with
+      | None -> (false, false)
+      | Some config ->
+          let root_upgraded =
+            let config_version = OpamFile.Config.opam_version config in
+            let cmp =
+              OpamVersion.(compare OpamFile.Config.format_version config_version)
+            in
+            if cmp < 0 then
+              OpamConsole.error_and_exit `Configuration_error
+                "%s reports a newer opam version, aborting."
+                 (OpamFilename.Dir.to_string root_dir)
+            else
+              cmp = 0
+          in
+            (true, root_upgraded)
+    in
     let plugins_bin = OpamPath.plugins_bin root_dir in
     let plugin_symlink_present =
       OpamFilename.is_symlink (OpamPath.plugin_bin root_dir (OpamPackage.Name.of_string name))
@@ -157,7 +174,7 @@ let check_and_run_external_commands () =
         Unix.environment ()
     in
     match OpamSystem.resolve_command ~env command with
-    | Some command when plugin_symlink_present ->
+    | Some command when plugin_symlink_present && root_upgraded ->
       let argv = Array.of_list (command :: args) in
       raise (OpamStd.Sys.Exec (command, argv, env))
     | None when not has_init -> (cli, argv)
@@ -202,9 +219,12 @@ let check_and_run_external_commands () =
              (OpamPackage.to_string (OpamPackage.Set.max_elt candidates));
            exit (OpamStd.Sys.get_exit_code `Bad_arguments))
         else if
-          OpamConsole.confirm "Opam plugin \"%s\" is not installed. \
-                               Install it on the current switch?"
-            name
+          (if cmd = None then
+             OpamConsole.confirm "Opam plugin \"%s\" is not installed. \
+                                  Install it on the current switch?"
+           else
+             OpamConsole.confirm "Opam plugin \"%s\" may require upgrading/reinstalling. \
+                                  Reinstall the plugin on the current switch?") name
         then
           let nv =
             try
@@ -228,8 +248,10 @@ let check_and_run_external_commands () =
               OpamSwitchState.drop @@ (
               if cmd = None then
                 OpamClient.install st [OpamSolution.eq_atom_of_package nv]
+              else if root_upgraded then
+                OpamClient.reinstall st [OpamSolution.eq_atom_of_package nv]
               else
-                OpamClient.reinstall st [OpamSolution.eq_atom_of_package nv])
+                OpamClient.upgrade st ~all:false [OpamSolution.eq_atom_of_package nv])
             );
           match OpamSystem.resolve_command ~env command with
           | None ->

src/core/opamFilename.ml

@@ -238,6 +238,10 @@ let files d =
   let fs = OpamSystem.files (Dir.to_string d) in
   List.rev_map of_string fs
 
+let files_and_links d =
+  let fs = OpamSystem.files_all_not_dir (Dir.to_string d) in
+  List.rev_map of_string fs
+
 let copy ~src ~dst =
   if src <> dst then OpamSystem.copy_file (to_string src) (to_string dst)
 

src/core/opamFilename.mli

@@ -170,6 +170,10 @@ val rec_files: Dir.t -> t list
 (** List all the filename. Do not recurse. *)
 val files: Dir.t -> t list
 
+(** Returns alll the files, including dangling symlinks (which means that
+    not all entries will satisfy {!exists}). *)
+val files_and_links: Dir.t -> t list
+
 (** Apply a function on the contents of a file *)
 val with_contents: (string -> 'a) -> t -> 'a
 

src/core/opamSystem.mli

@@ -140,6 +140,10 @@ val rec_files: string -> string list
 (** Return the list of files in the current directory. *)
 val files: string -> string list
 
+(** Return the list of files in the current directory, inclduing any
+    dangling symlinks. *)
+val files_all_not_dir: string -> string list
+
 (** [rec_dirs dir] return the list list of all directories recursively
     (going through symbolink links). *)
 val rec_dirs: string -> string list

src/state/opamFormatUpgrade.ml

@@ -1098,14 +1098,22 @@ let as_necessary requested_lock global_lock root config =
     |> List.partition (fun (v,_) ->
         OpamVersion.compare v latest_hard_upgrade <= 0)
   in
+  let erase_plugin_links root =
+    let plugins_bin = OpamPath.plugins_bin root in
+    if OpamFilename.exists_dir plugins_bin then begin
+      List.iter OpamFilename.remove @@ OpamFilename.files_and_links plugins_bin
+    end
+  in
   let light config =
     let config =
       List.fold_left (fun config (v, from) ->
           from root config |> OpamFile.Config.with_opam_version v)
         config light_upg
     in
-    (if not on_the_fly then
-       OpamFile.Config.write (OpamPath.config root) config);
+    if not on_the_fly then begin
+      OpamFile.Config.write (OpamPath.config root) config;
+      erase_plugin_links root;
+    end;
     config
   in
   let hard config =
@@ -1114,6 +1122,7 @@ let as_necessary requested_lock global_lock root config =
         (* save the current version to mitigate damage is the upgrade goes
            wrong afterwards *)
         OpamFile.Config.write (OpamPath.config root) config;
+        erase_plugin_links root;
         config)
       config hard_upg
   in