Skip to content

Commit

Permalink
Fix "opam install --deps/--show <local_dir>" not updating pinned pack…
Browse files Browse the repository at this point in the history
…ages' metadata

The reinstall field from the switch state wasn't updated according to
the implicit pin and thus opam didn't think there was anything to do.
  • Loading branch information
kit-ty-kate committed Oct 22, 2024
1 parent b060e1d commit 3a911c1
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 10 deletions.
1 change: 1 addition & 0 deletions master_changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ users)

## 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)

Expand Down
23 changes: 23 additions & 0 deletions src/client/opamAuxCommands.ml
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,29 @@ 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 to_pin ->
let old_opam = OpamPackage.Map.find pinned_pkg st.installed_opams in
let new_opam = OpamPackage.Map.find to_pin local_opams in
if OpamFile.OPAM.effectively_equal old_opam new_opam then
reinstall
else
OpamPackage.Set.add to_pin
(OpamPackage.Set.remove pinned_pkg reinstall))
installed_pinned (Lazy.force st.reinstall)
);
pinned;
} in
st, local_packages
Expand Down
3 changes: 0 additions & 3 deletions src/client/opamClient.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions tests/reftests/autopin.test
Original file line number Diff line number Diff line change
Expand Up @@ -286,16 +286,16 @@ opam believes some required external dependencies are missing. opam can:

Nothing to do.
### opam install ./foo --show
[NOTE] Package foo-core is already installed (current version is 2.0).
The following actions would be performed:
=== recompile 1 package
- recompile foo-core 2.0 (pinned)
=== install 2 packages
- install foo-format 2.0 (pinned)
- install foo-format-bis 2.0 (pinned)
- install foo-format 2.0 (pinned)
- install foo-format-bis 2.0 (pinned)

The following system packages will first need to be installed:
some-depext
### opam install ./foo --depext-only --show
[NOTE] Package foo-core is already installed (current version is 2.0).

The following system packages will first need to be installed:
some-depext
Expand Down
17 changes: 15 additions & 2 deletions tests/reftests/pin.test
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,19 @@ depends: "dependency" {= "2"}
### opam install --deps-only ./pin-change
[NOTE] Ignoring uncommitted changes in ${BASEDIR}/pin-change (`--working-dir' not specified or specified with no argument).
[pin-change.dev] synchronised (no changes)
The following actions will be performed:
=== recompile 1 package
- recompile pin-change dev (pinned) [uses dependency]
=== upgrade 1 package
- upgrade dependency 1 to 2 [required by pin-change]

<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
-> removed dependency.1
-> installed dependency.2
-> retrieved pin-change.dev (no changes)
-> removed pin-change.dev
-> installed pin-change.dev
Done.
### opam remove pin-change
The following actions will be performed:
=== remove 1 package
Expand All @@ -480,10 +493,10 @@ depends: "dependency" {= "3"}
[pin-change.dev] synchronised (no changes)
The following actions will be performed:
=== upgrade 1 package
- upgrade dependency 1 to 3 [required by pin-change]
- upgrade dependency 2 to 3 [required by pin-change]

<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
-> removed dependency.1
-> removed dependency.2
-> installed dependency.3
Done.
### opam pin
Expand Down
6 changes: 5 additions & 1 deletion tests/reftests/working-dir.test
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,11 @@ Done.
opam-version: "2.0"
depends: ["qux" {= "1"}]
### opam install ./ongoing --working-dir --show-action
[NOTE] Package ongoing is already installed (current version is dev).
The following actions would be performed:
=== downgrade 1 package
- downgrade qux 4 to 1 [required by ongoing]
=== recompile 1 package
- recompile ongoing dev (pinned)
### opam reinstall ./ongoing --working-dir --show-action

<><> Synchronising pinned packages ><><><><><><><><><><><><><><><><><><><><><><>
Expand Down

0 comments on commit 3a911c1

Please sign in to comment.