Fix #6248

master_changes.md

@@ -24,6 +24,8 @@ users)
 ## Actions
 
 ## Install
+  * Fix `opam install <local_dir>` not updating and storing pinned packages' metadata [#6209 @kit-ty-kate - fix #5567]
+  * Fix `opam install --deps-only/--show-action <local_dir>` not updating (without storing) pinned packages' metadata [#6209 @kit-ty-kate - fix #5567]
 
 ## Build (package)
 
@@ -106,6 +108,7 @@ users)
 
 ## Reftests
 ### Tests
+  * Add a test showing the behaviour of opam install when a local opam file changes while being pinned [#6209 @kit-ty-kate]
 
 ### Engine
 

src/client/opamAuxCommands.ml

@@ -250,11 +250,33 @@ let resolve_locals ?(quiet=false) ?locked ?recurse ?subpath
              (OpamUrl.to_string nf.pin.pin_url))
           duplicates)
 
-let autopin_aux st ?quiet ?(for_view=false) ?recurse ?subpath ?locked
+let autopin_aux st ?quiet ?recurse ?subpath ?locked
     atom_or_local_list =
   let to_pin, atoms =
     resolve_locals ?quiet ?recurse ?subpath ?locked atom_or_local_list
   in
+  let atoms =
+    (* Ensure the atoms are set to their expected version *)
+    List.map (function
+        | name, None ->
+          begin match
+            List.find_opt
+              (fun {pin_name; _} -> OpamPackage.Name.equal pin_name name)
+              to_pin
+          with
+          | None -> (name, Some (`Eq, OpamPinCommand.default_version st name))
+          | Some {pin = {pin_file; _}; _} ->
+            let opam_file = OpamFile.OPAM.safe_read pin_file in
+            let v = match OpamFile.OPAM.version_opt opam_file with
+              | None -> OpamPinCommand.default_version st name
+              | Some v -> v
+            in
+            (name, Some (`Eq, v))
+          end
+        | (_, Some _) as atom -> atom
+        (* TODO: does autopin really need other forms of atoms than `Eq ? *)
+      ) atoms
+  in
   if to_pin = [] then
     atoms, to_pin, OpamPackage.Set.empty, OpamPackage.Set.empty
   else
@@ -303,16 +325,31 @@ let autopin_aux st ?quiet ?(for_view=false) ?recurse ?subpath ?locked
               | _ -> false)
            | None -> false)
           &&
-          (* For `opam show`, we need to check does the opam file changed to
-             perform a simulated pin if so *)
-          (not for_view ||
-           match
+          (match
              OpamSwitchState.opam_opt st pinned_pkg,
              OpamFile.OPAM.read_opt nf.pin.pin_file
            with
            | Some opam0, Some opam ->
              let opam = OpamFile.OPAM.with_locked_opt nf.pin.pin_locked opam in
-             OpamFile.OPAM.equal opam0 opam
+             let opam = OpamFile.OPAM.with_name nf.pin_name opam in
+             let opam =
+               match OpamFile.OPAM.version_opt opam with
+               | None ->
+                 OpamFile.OPAM.with_version
+                   (OpamPinCommand.default_version st nf.pin_name) opam
+               | Some _ -> opam
+             in
+             let opam =
+               OpamFile.OPAM.with_url
+                 (OpamFile.URL.create ?subpath:nf.pin.pin_subpath nf.pin.pin_url)
+                 opam
+             in
+             let opam =
+               (* This is required to avoid the case where locked opam files were
+                  stored with the added `available: opam-version >= "2.1"` *)
+               OpamFile.OPAM.with_available (OpamFile.OPAM.available opam0) opam
+             in
+             OpamFile.OPAM.effectively_equal opam0 opam
            | _, _ -> false)
         with Not_found -> false)
       to_pin
@@ -389,14 +426,37 @@ let simulate_local_pinnings ?quiet ?(for_view=false) st to_pin =
            st.switch_global st.switch st.switch_config ~pinned
            ~opams:local_opams)
     );
+    reinstall = lazy (
+      let open OpamPackage.Set.Op in
+      let installed_pinned = st.pinned %% st.installed in
+      OpamPackage.Set.fold (fun pinned_pkg reinstall ->
+          match
+            OpamPackage.Set.find_opt
+              (fun pkg ->
+                 OpamPackage.Name.equal
+                   (OpamPackage.name pinned_pkg)
+                   (OpamPackage.name pkg))
+              local_packages
+          with
+          | None -> reinstall
+          | Some local_pkg ->
+            let old_opam = OpamPackage.Map.find pinned_pkg st.installed_opams in
+            let new_opam = OpamPackage.Map.find local_pkg local_opams in
+            if OpamFile.OPAM.effectively_equal old_opam new_opam then
+              reinstall
+            else
+              OpamPackage.Set.add local_pkg
+                (OpamPackage.Set.remove pinned_pkg reinstall))
+        installed_pinned (Lazy.force st.reinstall)
+    );
     pinned;
   } in
   st, local_packages
 
 let simulate_autopin st ?quiet ?(for_view=false) ?locked ?recurse ?subpath
     atom_or_local_list =
   let atoms, to_pin, obsolete_pins, already_pinned_set =
-    autopin_aux st ?quiet ~for_view ?recurse ?subpath ?locked atom_or_local_list
+    autopin_aux st ?quiet ?recurse ?subpath ?locked atom_or_local_list
   in
   if to_pin = [] then st, atoms else
   let st =

src/client/opamClient.ml

@@ -2207,9 +2207,6 @@ let install_t t ?ask ?(ignore_conflicts=false) ?(depext_only=false)
   in
   let pkg_reinstall =
     if assume_built then OpamPackage.Set.of_list pkg_skip
-    else if deps_only then OpamPackage.Set.empty
-    (* NOTE: As we only install dependency packages, there are no intersections
-       between t.reinstall and pkg_skip *)
     else Lazy.force t.reinstall %% OpamPackage.Set.of_list pkg_skip
   in
   (* Add the packages to the list of package roots and display a