Merge pull request #4621 from dra27/reinstall-plugins

Reinstall plugin package if the symlink is missing

master_changes.md

@@ -12,6 +12,7 @@ New option/command/subcommand are prefixed with ◈.
   * Add cli versioning for opam environment variables [#4606 @rjbou]
   * Deprecated `build-doc`, `build-test`, `make` [#4581 @rjbou]
   * Add cli versioning for enums of flags with predefined enums [#4606 @rjbou]
+  * Ensure the symlink for a plugin is maintained on each invocation [#4621 @dra27 - partially fixes #4619]
 
 ## Init
   * Introduce a `default-invariant` config field, restore the 2.0 semantics for

src/client/opamCliMain.ml

@@ -88,7 +88,6 @@ let rec preprocess_argv cli yes args =
 
 (* Handle git-like plugins *)
 let check_and_run_external_commands () =
-  let plugin_prefix = "opam-" in
   (* Pre-process the --yes and --cli options *)
   let (cli, yes, argv) =
     match Array.to_list Sys.argv with
@@ -132,13 +131,16 @@ let check_and_run_external_commands () =
     then (cli, argv)
     else
     (* No such command, check if there is a matching plugin *)
-    let command = plugin_prefix ^ name in
+    let command = OpamPath.plugin_prefix ^ name in
     let answer = if yes then Some true else OpamCoreConfig.E.yes () in
     OpamCoreConfig.init ~answer ();
     OpamFormatConfig.init ();
     let root_dir = OpamStateConfig.opamroot () in
     let has_init = OpamStateConfig.load_defaults root_dir <> None 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))
+    in
     let env =
       if has_init then
         let updates =
@@ -155,18 +157,18 @@ let check_and_run_external_commands () =
         Unix.environment ()
     in
     match OpamSystem.resolve_command ~env command with
-    | Some command ->
+    | Some command when plugin_symlink_present ->
       let argv = Array.of_list (command :: args) in
       raise (OpamStd.Sys.Exec (command, argv, env))
     | None when not has_init -> (cli, argv)
-    | None ->
+    | cmd ->
       (* Look for a corresponding package *)
       match OpamStateConfig.get_switch_opt () with
       | None -> (cli, argv)
       | Some sw ->
         OpamGlobalState.with_ `Lock_none @@ fun gt ->
         OpamSwitchState.with_ `Lock_none gt ~switch:sw @@ fun st ->
-        let prefixed_name = plugin_prefix ^ name in
+        let prefixed_name = OpamPath.plugin_prefix ^ name in
         let candidates =
           OpamPackage.packages_of_names
             (Lazy.force st.available_packages)
@@ -184,7 +186,7 @@ let check_and_run_external_commands () =
         in
         let installed = OpamPackage.Set.inter plugins st.installed in
         if OpamPackage.Set.is_empty candidates then (cli, argv)
-        else if not OpamPackage.Set.(is_empty installed) then
+        else if not OpamPackage.Set.(is_empty installed) && cmd = None then
           (OpamConsole.error
              "Plugin %s is already installed, but no %s command was found.\n\
               Try upgrading, and report to the package maintainer if \
@@ -206,18 +208,28 @@ let check_and_run_external_commands () =
         then
           let nv =
             try
-              OpamPackage.max_version plugins
-                (OpamPackage.Name.of_string prefixed_name)
+              (* If the command was resolved, attempt to find the package to reinstall. *)
+              if cmd = None then
+                raise Not_found
+              else
+                OpamPackage.package_of_name installed (OpamPackage.Name.of_string prefixed_name)
             with Not_found ->
-              OpamPackage.max_version plugins
-                (OpamPackage.Name.of_string name)
+              try
+                OpamPackage.max_version plugins
+                  (OpamPackage.Name.of_string prefixed_name)
+              with Not_found ->
+                OpamPackage.max_version plugins
+                  (OpamPackage.Name.of_string name)
           in
           OpamRepositoryConfig.init ();
           OpamSolverConfig.init ();
           OpamClientConfig.init ();
           OpamSwitchState.with_ `Lock_write gt (fun st ->
-              OpamSwitchState.drop @@
-              OpamClient.install st [OpamSolution.eq_atom_of_package nv]
+              OpamSwitchState.drop @@ (
+              if cmd = None then
+                OpamClient.install st [OpamSolution.eq_atom_of_package nv]
+              else
+                OpamClient.reinstall st [OpamSolution.eq_atom_of_package nv])
             );
           match OpamSystem.resolve_command ~env command with
           | None ->

src/format/opamPath.ml

@@ -57,15 +57,17 @@ let backup_dir t = t / "backup"
 
 let backup t = backup_dir t /- backup_file ()
 
+let plugin_prefix = "opam-"
+
 let plugins t = t / "plugins"
 
 let plugins_bin t = plugins t / "bin"
 
 let plugin_bin t name =
   let sname = OpamPackage.Name.to_string name in
   let basename =
-    if OpamStd.String.starts_with ~prefix:"opam-" sname then sname
-    else "opam-" ^ sname
+    if OpamStd.String.starts_with ~prefix:plugin_prefix sname then sname
+    else plugin_prefix ^ sname
   in
   plugins_bin t // basename
 

src/format/opamPath.mli

@@ -64,6 +64,9 @@ val backup_dir: t -> dirname
 (** Backup file for state export *)
 val backup: t -> switch_selections OpamFile.t
 
+(** The prefix for plugin commands (["opam-"]) *)
+val plugin_prefix : string
+
 (** The directory for plugins data {i $opam/plugins} *)
 val plugins: t -> dirname
 

src/state/opamFileTools.ml

@@ -498,7 +498,7 @@ let t_lint ?check_extra_files ?(check_upstream=false) ?(all=false) t =
        match t.OpamFile.OPAM.name with
        | None -> false
        | Some name ->
-         not (OpamStd.String.starts_with ~prefix:"opam-"
+         not (OpamStd.String.starts_with ~prefix:OpamPath.plugin_prefix
                 (OpamPackage.Name.to_string name)));
     (let unclosed =
        List.fold_left (fun acc s ->