From 1d8094d56a06209c071f70015caee1765fb8bcf0 Mon Sep 17 00:00:00 2001 From: Raja Boujbel Date: Wed, 20 Jul 2022 11:51:41 +0200 Subject: [PATCH 1/7] reftest: add upgrade switch from 2.0 test --- tests/reftests/dune.inc | 17 +++++++++++ tests/reftests/upgrade-two-point-o.test | 39 +++++++++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 tests/reftests/upgrade-two-point-o.test diff --git a/tests/reftests/dune.inc b/tests/reftests/dune.inc index b5ecd9e1ec6..42ea7d33e56 100644 --- a/tests/reftests/dune.inc +++ b/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 diff --git a/tests/reftests/upgrade-two-point-o.test b/tests/reftests/upgrade-two-point-o.test new file mode 100644 index 00000000000..9b299b7d61c --- /dev/null +++ b/tests/reftests/upgrade-two-point-o.test @@ -0,0 +1,39 @@ +N0REP0 +### +opam-version: "2.0" +flags: compiler +### +opam-version: "2.0" +flags: compiler +### OPAMYES=1 +### +opam-version: "2.0" +repositories: "default" +installed-switches: ["pinned-comp"] +switch: "pinned-comp" +default-compiler: ["i-am-compiler"] +### +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-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"] +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 +The following actions would be performed: +=== upgrade 1 package + - upgrade i-am-compiler 1 to 2 From b6077bc90813a2f2a740b597ccef71d1fcef5d16 Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Mon, 11 Jul 2022 11:07:14 +0100 Subject: [PATCH 2/7] compat: add `Lazy.map` --- src/core/opamCompat.ml | 12 ++++++++++++ src/core/opamCompat.mli | 11 +++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/core/opamCompat.ml b/src/core/opamCompat.ml index 91520c8aa7a..19498569e10 100644 --- a/src/core/opamCompat.ml +++ b/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 diff --git a/src/core/opamCompat.mli b/src/core/opamCompat.mli index bfbbcc415e7..cb8c907c661 100644 --- a/src/core/opamCompat.mli +++ b/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 From bdb3eff1912a0a33c23b9e09578154e39c921dc6 Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Mon, 11 Jul 2022 11:07:14 +0100 Subject: [PATCH 3/7] Fix invariant inference w.r.t. pinned packages When inferring a switch invariant from base packages, opam filtered out alternate versions of any package which is pinned. This causes all pinned packages to appear to be unique versions and so they are then inferred without constraint. Pinned packages are no longer filtered when inferring the invariant - only availability is used. --- src/state/opamSwitchState.ml | 41 +++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/src/state/opamSwitchState.ml b/src/state/opamSwitchState.ml index 08b8975abb1..c3c1da304f6 100644 --- a/src/state/opamSwitchState.ml +++ b/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 From 6855c58b37eb880034e3d5742af204aae6989b34 Mon Sep 17 00:00:00 2001 From: Raja Boujbel Date: Wed, 20 Jul 2022 11:53:20 +0200 Subject: [PATCH 4/7] reftest: update upgrade switch from 2.0 --- tests/reftests/upgrade-two-point-o.test | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/reftests/upgrade-two-point-o.test b/tests/reftests/upgrade-two-point-o.test index 9b299b7d61c..8ebc765ee46 100644 --- a/tests/reftests/upgrade-two-point-o.test +++ b/tests/reftests/upgrade-two-point-o.test @@ -24,7 +24,7 @@ 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"] +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). @@ -34,6 +34,6 @@ Nothing to do. Ok, i-am-compiler is no longer pinned locally (version 1) Nothing to do. ### opam upgrade --show-action -The following actions would be performed: -=== upgrade 1 package - - upgrade i-am-compiler 1 to 2 +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. From 423c2d81d9d0e13518394defb98abb5c6dcaa831 Mon Sep 17 00:00:00 2001 From: Raja Boujbel Date: Wed, 20 Jul 2022 11:53:41 +0200 Subject: [PATCH 5/7] update changes --- master_changes.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/master_changes.md b/master_changes.md index 98f5f554142..bd20ad3718b 100644 --- a/master_changes.md +++ b/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] From 0adbc1fe3cfa1ee759c109b470594f2f3e42ec2d Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Fri, 22 Jul 2022 16:23:45 +0100 Subject: [PATCH 6/7] Bump the src_ext / opam-repository cache --- .github/workflows/ci.ml | 2 +- .github/workflows/main.yml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.ml b/.github/workflows/ci.ml index 781a298478c..c8e5e5b7d69 100644 --- a/.github/workflows/ci.ml +++ b/.github/workflows/ci.ml @@ -517,7 +517,7 @@ let main oc : unit = ("CYGWIN_EPOCH", "2"); ] 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 }}"); diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 8d194dbb0df..a0a1384de5d 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -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') }} From 550b18a0ffd63e9e6e5584e2e77a463056324467 Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Fri, 22 Jul 2022 15:59:09 +0100 Subject: [PATCH 7/7] Bump the caches --- .github/workflows/ci.ml | 2 +- .github/workflows/main.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.ml b/.github/workflows/ci.ml index c8e5e5b7d69..5e1ade4bb86 100644 --- a/.github/workflows/ci.ml +++ b/.github/workflows/ci.ml @@ -514,7 +514,7 @@ 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-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 }}"); diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index a0a1384de5d..a18383faaf4 100644 --- a/.github/workflows/main.yml +++ b/.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: