_Correct_ trying to be too clever when upgrading switches from opam 2.0

.github/workflows/ci.ml

@@ -514,10 +514,10 @@ let main oc : unit =
     ("CYGWIN_MIRROR", "http://mirrors.kernel.org/sourceware/cygwin/");
     ("CYGWIN_ROOT", "D:\\cygwin");
     ("CYGWIN", "winsymlinks:native");
-    ("CYGWIN_EPOCH", "2");
+    ("CYGWIN_EPOCH", "3");
   ] in
   let keys = [
-    ("archives", "archives-${{ hashFiles('src_ext/Makefile.sources', 'src_ext/Makefile', '.github/scripts/common/preamble.sh', '.github/scripts/main/preamble.sh', '.github/scripts/main/archives-cache.sh') }}-${{ env.OPAM_REPO_SHA }}");
+    ("archives", "archives-1-${{ hashFiles('src_ext/Makefile.sources', 'src_ext/Makefile', '.github/scripts/common/preamble.sh', '.github/scripts/main/preamble.sh', '.github/scripts/main/archives-cache.sh') }}-${{ env.OPAM_REPO_SHA }}");
     ("ocaml-secondary-compiler", "legacy-${{ env.OPAM_REPO_SHA }}");
     ("ocaml-cache", "${{ hashFiles('.github/scripts/main/ocaml-cache.sh', '.github/scripts/main/preamble.sh') }}");
     ("cygwin", "${{ hashFiles('.github/scripts/cygwin.cmd') }}-${{ env.CYGWIN_EPOCH }}");

.github/workflows/main.yml

@@ -46,7 +46,7 @@ env:
   CYGWIN_MIRROR: http://mirrors.kernel.org/sourceware/cygwin/
   CYGWIN_ROOT: D:\cygwin
   CYGWIN: winsymlinks:native
-  CYGWIN_EPOCH: 2
+  CYGWIN_EPOCH: 3
 
 defaults:
   run:
@@ -71,8 +71,8 @@ jobs:
     - name: Determine cache keys
       id: keys
       run: |
-        echo archives=archives-${{ hashFiles('src_ext/Makefile.sources', 'src_ext/Makefile', '.github/scripts/common/preamble.sh', '.github/scripts/main/preamble.sh', '.github/scripts/main/archives-cache.sh') }}-${{ env.OPAM_REPO_SHA }}
-        echo ::set-output name=archives::archives-${{ hashFiles('src_ext/Makefile.sources', 'src_ext/Makefile', '.github/scripts/common/preamble.sh', '.github/scripts/main/preamble.sh', '.github/scripts/main/archives-cache.sh') }}-${{ env.OPAM_REPO_SHA }}
+        echo archives=archives-1-${{ hashFiles('src_ext/Makefile.sources', 'src_ext/Makefile', '.github/scripts/common/preamble.sh', '.github/scripts/main/preamble.sh', '.github/scripts/main/archives-cache.sh') }}-${{ env.OPAM_REPO_SHA }}
+        echo ::set-output name=archives::archives-1-${{ hashFiles('src_ext/Makefile.sources', 'src_ext/Makefile', '.github/scripts/common/preamble.sh', '.github/scripts/main/preamble.sh', '.github/scripts/main/archives-cache.sh') }}-${{ env.OPAM_REPO_SHA }}
         echo ocaml-secondary-compiler=legacy-${{ env.OPAM_REPO_SHA }}
         echo ::set-output name=ocaml-secondary-compiler::legacy-${{ env.OPAM_REPO_SHA }}
         echo ocaml-cache=${{ hashFiles('.github/scripts/main/ocaml-cache.sh', '.github/scripts/main/preamble.sh') }}

master_changes.md

@@ -65,6 +65,7 @@ users)
   * [BUG] Fix `set-invariant: default repos were loaded instead of switch repos [#4866 @rjbou]
   * Add support for `opam switch -` (go to previous non-local switch) [#4910 @kit-ty-kate - fix 4866]
   * On loading, check for executable external files if they are in `PATH`, and warn if not the case [#4932 @rjbou - fix #4923]
+  * When inferring a 2.1+ switch invariant from 2.0 base packages, don't filter out pinned packages as that causes very wide invariants for pinned compiler packages [#5176 @dra27 - fix #4501]
 
 ## Pin
   * Switch the default version when undefined from ~dev to dev [#4949 @kit-ty-kate]
@@ -293,6 +294,7 @@ users)
   * Update opam root version test do escape `OPAMROOTVERSION` sed, it matches generated hexa temporary directory names [#5007 @AltGr]
   * Add json output test [#5143 @rjbou]
   * Add test for opam file write with format preserved bug in #4936, fixed in #4941 [#4159 @rjbou]
+  * Add test for switch upgrade from 2.0 root, with pinned compiler [#5176 @rjbou @kit-ty-kate]
 ### Engine
   * Add `opam-cat` to normalise opam file printing [#4763 @rjbou @dra27] [2.1.0~rc2 #4715]
   * Fix meld reftest: open only with failing ones [#4913 @rjbou]
@@ -449,3 +451,4 @@ users)
   * `OpamJson`: use `Jsonm` and add an `of_string` function [#5142 @rjbou]
   * `OpamStd.Config.E`: add `value_t` to allow getting environment variable value dynamically [#5111 @rjbou]
   * `OpamCompat.Unix`: add `realpath` for ocaml < 4.13, and use it in `OpamSystem` [#5152 @rjbou]
+  * `OpamCompat`: add `Lazy` module and `Lazy.map` function [#5176 @dra27]

src/core/opamCompat.ml

@@ -130,3 +130,15 @@ module Stdlib = Pervasives
 #else
 module Stdlib = Stdlib
 #endif
+
+module Lazy =
+#if OCAML_VERSION >= (4, 13, 0)
+  Lazy
+#else
+struct
+  include Lazy
+
+  let map f x =
+    lazy (f (force x))
+end
+#endif

src/core/opamCompat.mli

@@ -97,3 +97,14 @@ module Stdlib = Pervasives
 #else
 module Stdlib = Stdlib
 #endif
+
+module Lazy
+#if OCAML_VERSION >= (4, 13, 0)
+= Lazy
+#else
+: sig
+  include module type of struct include Lazy end
+
+  val map : ('a -> 'b) -> 'a t -> 'b t
+end
+#endif

src/state/opamSwitchState.ml

@@ -34,24 +34,28 @@ let load_switch_config ?lock_kind gt switch =
        (OpamSwitch.to_string switch);
      OpamFile.Switch_config.empty)
 
-let compute_available_packages gt switch switch_config ~pinned ~opams =
+let filter_available_packages gt switch switch_config ~opams =
+  OpamPackage.keys @@
+    OpamPackage.Map.filter (fun package opam ->
+        OpamFilter.eval_to_bool ~default:false
+          (OpamPackageVar.resolve_switch_raw ~package gt switch switch_config)
+          (OpamFile.OPAM.available opam))
+      opams
+
+let compute_available_and_pinned_packages gt switch switch_config ~pinned ~opams =
   (* remove all versions of pinned packages, but the pinned-to version *)
   let pinned_names = OpamPackage.names_of_packages pinned in
-  let opams =
-    OpamPackage.Map.filter
+  let (opams, pinned) =
+    OpamPackage.Map.partition
       (fun nv _ ->
          not (OpamPackage.Name.Set.mem nv.name pinned_names) ||
          OpamPackage.Set.mem nv pinned)
       opams
   in
-  let avail_map =
-    OpamPackage.Map.filter (fun package opam ->
-        OpamFilter.eval_to_bool ~default:false
-          (OpamPackageVar.resolve_switch_raw ~package gt switch switch_config)
-          (OpamFile.OPAM.available opam))
-      opams
-  in
-  OpamPackage.keys avail_map
+  (filter_available_packages gt switch switch_config ~opams, pinned)
+
+let compute_available_packages gt switch switch_config ~pinned ~opams =
+  fst @@ compute_available_and_pinned_packages gt switch switch_config ~pinned ~opams
 
 let repos_list_raw rt switch_config =
   let global, repos =
@@ -115,7 +119,7 @@ let infer_switch_invariant_raw
             deps dmap
         in
         dmap)
-      (OpamPackage.packages_of_names (Lazy.force available_packages) @@
+      (OpamPackage.packages_of_names available_packages @@
        OpamPackage.names_of_packages @@
        compiler)
       OpamPackage.Map.empty
@@ -133,7 +137,7 @@ let infer_switch_invariant_raw
   match List.sort (fun (_, c1) (_, c2) -> compare c1 c2) counts with
   | (nv, _) :: _ ->
     let versions =
-      OpamPackage.packages_of_name (Lazy.force available_packages) nv.name
+      OpamPackage.packages_of_name available_packages nv.name
     in
     let n = OpamPackage.Set.cardinal versions in
     if n <= 1 then
@@ -153,9 +157,10 @@ let infer_switch_invariant st =
         st.installed
     else st.compiler_packages
   in
+  let lazy available_packages = st.available_packages in
   infer_switch_invariant_raw
     st.switch_global st.switch st.switch_config st.opams
-    st.packages compiler_packages st.installed_roots st.available_packages
+    st.packages compiler_packages st.installed_roots available_packages
 
 let depexts_raw ~env nv opams =
   try
@@ -330,7 +335,7 @@ let load lock_kind gt rt switch =
     OpamPackage.Map.union (fun _ x -> x) repos_package_index pinned_opams
   in
   let available_packages =
-    lazy (compute_available_packages gt switch switch_config
+    lazy (compute_available_and_pinned_packages gt switch switch_config
             ~pinned ~opams)
   in
   let opams =
@@ -402,6 +407,11 @@ let load lock_kind gt rt switch =
     match switch_config.invariant with
     | Some invariant -> switch_config, invariant
     | None ->
+      let available_packages =
+        let lazy (available_packages, pinned) = available_packages in
+        OpamPackage.Set.union available_packages @@
+          filter_available_packages gt switch switch_config ~opams:pinned
+      in
       let invariant =
         infer_switch_invariant_raw
           gt switch switch_config opams
@@ -499,6 +509,7 @@ let load lock_kind gt rt switch =
       OpamPackage.Set.empty
   ) in
   (* depext check *)
+  let available_packages = OpamCompat.Lazy.map fst available_packages in
   let sys_packages =
     if not (OpamFile.Config.depext gt.config)
     || OpamStateConfig.(!r.no_depexts) then

tests/reftests/dune.inc

@@ -804,6 +804,23 @@
    %{targets}
    (run ./run.exe %{bin:opam} %{dep:upgrade-format.test} %{read-lines:testing-env}))))
 
+(rule
+ (alias reftest-upgrade-two-point-o)
+ (action
+  (diff upgrade-two-point-o.test upgrade-two-point-o.out)))
+
+(alias
+ (name reftest)
+ (deps (alias reftest-upgrade-two-point-o)))
+
+(rule
+ (targets upgrade-two-point-o.out)
+ (deps root-N0REP0)
+ (action
+  (with-stdout-to
+   %{targets}
+   (run ./run.exe %{bin:opam} %{dep:upgrade-two-point-o.test} %{read-lines:testing-env}))))
+
 (rule
  (alias reftest-upgrade)
  (action

tests/reftests/upgrade-two-point-o.test

@@ -0,0 +1,39 @@
+N0REP0
+### <pkg:i-am-compiler.1>
+opam-version: "2.0"
+flags: compiler
+### <pkg:i-am-compiler.2>
+opam-version: "2.0"
+flags: compiler
+### OPAMYES=1
+### <OPAM/config>
+opam-version: "2.0"
+repositories: "default"
+installed-switches: ["pinned-comp"]
+switch: "pinned-comp"
+default-compiler: ["i-am-compiler"]
+### <OPAM/pinned-comp/.opam-switch/switch-state>
+opam-version: "2.0"
+compiler: ["i-am-compiler.1"]
+roots: ["i-am-compiler.1"]
+installed: ["i-am-compiler.1"]
+pinned: ["i-am-compiler.1"]
+### <OPAM/pinned-comp/.opam-switch/switch-config>
+opam-version: "2.0"
+synopsis: "switch with pinned compiler"
+### OPAMDEBUGSECTIONS=STATE opam upgrade --show-action --debug-level=-1
+STATE                           LOAD-SWITCH-STATE @ pinned-comp
+STATE                           Definition missing for installed package i-am-compiler.1, copying from repo
+STATE                           Inferred invariant: from base packages { i-am-compiler.1 }, (roots { i-am-compiler.1 }) => ["i-am-compiler" {= "1"}]
+STATE                           Switch state loaded in 0.000s
+STATE                           Detected changed packages (marked for reinstall): {}
+Everything as up-to-date as possible (run with --verbose to show unavailable upgrades).
+However, you may "opam upgrade" these packages explicitly, which will ask permission to downgrade or uninstall the conflicting packages.
+Nothing to do.
+### opam pin remove i-am-compiler
+Ok, i-am-compiler is no longer pinned locally (version 1)
+Nothing to do.
+### opam upgrade --show-action
+Everything as up-to-date as possible (run with --verbose to show unavailable upgrades).
+However, you may "opam upgrade" these packages explicitly, which will ask permission to downgrade or uninstall the conflicting packages.
+Nothing to do.