diff --git a/master_changes.md b/master_changes.md index fc791de1029..0c29a5d423b 100644 --- a/master_changes.md +++ b/master_changes.md @@ -22,6 +22,7 @@ users) ## Config report ## Actions + * Fix the install cache storing the wrong version of the opam file after a build failure [#6213 @kit-ty-kate] ## Install @@ -112,6 +113,7 @@ users) * Move pin test to pin-legacy [#6135 @rjbou] * More exhaustive test for pin command: test different behaviour and cli options [#6135 @rjbou] * pin: add a test for erroneous first fetch done as local path on local VCS pinned packages [#6221 @rjbou] + * Add cache test for installed packages cache update after an action failure [#6213 @kit-ty-kate @rjbou] ### Engine * Update print file function [#6233 @rjbou] diff --git a/src/client/opamSolution.ml b/src/client/opamSolution.ml index 905d53cd82d..60280adf916 100644 --- a/src/client/opamSolution.ml +++ b/src/client/opamSolution.ml @@ -824,15 +824,46 @@ let parallel_apply t (* 2/ Display errors and finalize *) - OpamSwitchState.Installed_cache.save - (OpamPath.Switch.installed_opams_cache t.switch_global.root t.switch) - (OpamPackage.Set.fold (fun nv opams -> - let opam = - OpamSwitchState.opam t nv |> - OpamFile.OPAM.with_metadata_dir None - in - OpamPackage.Map.add nv opam opams) - t.installed OpamPackage.Map.empty); + let save_installed_cache failed = + OpamSwitchState.Installed_cache.save + (OpamPath.Switch.installed_opams_cache t.switch_global.root t.switch) + (OpamPackage.Set.fold (fun nv opams -> + (* NOTE: We need to know whether an action was successful + or not to know which version of the opam file to store + in the case: the previous one if it failed, or the new + one if it succeeded. *) + let pkg_failed = + List.exists (function + | `Fetch ps -> List.for_all (OpamPackage.equal nv) ps + | `Build p | `Change (_, _, p) | `Install p + | `Reinstall p | `Remove p -> OpamPackage.equal nv p) + failed + in + let add_to_opams opam = + let opam = OpamFile.OPAM.with_metadata_dir None opam in + OpamPackage.Map.add nv opam opams + in + if pkg_failed then + match OpamPackage.Map.find_opt nv t.installed_opams with + | None -> opams + | Some opam -> add_to_opams opam + else + add_to_opams (OpamSwitchState.opam t nv)) + t.installed OpamPackage.Map.empty); + in + begin match action_results with + | `Exception _ | `Error Aborted -> () + | `Error (Nothing_to_do | OK _) -> assert false + | `Error (Partial_error res) -> + let { actions_successes = _; + actions_errors; + actions_aborted; + } = res + in + save_installed_cache (List.map fst actions_errors @ actions_aborted) + | `Successful _ -> + save_installed_cache [] + end; let cleanup_artefacts graph = PackageActionGraph.iter_vertex (function diff --git a/tests/reftests/cache.test b/tests/reftests/cache.test new file mode 100644 index 00000000000..8e8012a3852 --- /dev/null +++ b/tests/reftests/cache.test @@ -0,0 +1,319 @@ +N0REP0 +### :: Installed Cache update on action failure +### : Origin +### : upgrades from 2.2 to 2.3 can trigger issues where a repository forgot to +### : add the missing extra-files field, opam tells the user they should +### : upgrade as the opam file changed (as extra-files was added automatically) +### : then the build fails and upon fixing the error opam still says the +### : package is to be rebuilt because the previous version of the opam file +### : was stored in the install cache +### : The bug is seen on specific case : a package is already installed, and +### : its update that doesn't remove it (fetch or build failure) update the +### : cache with the wrong opam file, the one from repo state. +### opam switch create action-failure --empty +### OPAMYES=1 +### +opam-version: "2.0" +### opam install always-here +The following actions will be performed: +=== install 1 package + - install always-here 1 + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +-> installed always-here.1 +Done. +### ::: Build +### +opam-version: "2.0" +build: "true" +### opam install upgrade.1 +The following actions will be performed: +=== install 1 package + - install upgrade 1 + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +-> installed upgrade.1 +Done. +### +opam-version: "2.0" +build: "false" +### opam upgrade +The following actions will be performed: +=== recompile 1 package + - recompile upgrade 1 [upstream or system changes] + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +[ERROR] The compilation of upgrade.1 failed at "false". + + + + +<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><> ++- The following actions failed +| - build upgrade 1 ++- +- No changes have been performed +# Return code 31 # +### +opam-version: "2.0" +build: "true" +### opam show --raw upgrade.1 +opam-version: "2.0" +name: "upgrade" +version: "1" +build: "true" +### cat ${BASEDIR}/OPAM/action-failure/.opam-switch/packages/upgrade.1/opam +opam-version: "2.0" +build: "true" +### opam-cache repo upgrade +=> default:upgrade.1 <= +opam-version: "2.0" +name: "upgrade" +version: "1" +build: "true" +### opam-cache installed action-failure +=> upgrade.1 <= +opam-version: "2.0" +name: "upgrade" +version: "1" +build: "true" +=> always-here.1 <= +opam-version: "2.0" +name: "always-here" +version: "1" +### opam upgrade +Already up-to-date. +Nothing to do. +### opam-cache installed action-failure +=> upgrade.1 <= +opam-version: "2.0" +name: "upgrade" +version: "1" +build: "true" +=> always-here.1 <= +opam-version: "2.0" +name: "always-here" +version: "1" +### ::: Install +### opam remove upgrade +The following actions will be performed: +=== remove 1 package + - remove upgrade 1 + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +-> removed upgrade.1 +Done. +### +opam-version: "2.0" +install: "true" +### opam install upgrade +The following actions will be performed: +=== install 1 package + - install upgrade 1 + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +-> installed upgrade.1 +Done. +### opam-cache installed action-failure +=> upgrade.1 <= +opam-version: "2.0" +name: "upgrade" +version: "1" +install: "true" +=> always-here.1 <= +opam-version: "2.0" +name: "always-here" +version: "1" +### +opam-version: "2.0" +install: "false" +### opam install upgrade | grep -v import +The following actions will be performed: +=== recompile 1 package + - recompile upgrade 1 + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +-> removed upgrade.1 +[ERROR] The installation of upgrade failed at "false". + + + + +<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><> ++- The following actions failed +| - install upgrade 1 ++- ++- The following changes have been performed +| - remove upgrade 1 ++- + +The former state can be restored with: +Or you can retry to install your package selection with: + ${OPAM} install --restore +# Return code 31 # +### test -f ${BASEDIR}/OPAM/action-failure/.opam-switch/packages/upgrade.1/opam +# Return code 1 # +### opam-cache installed action-failure +No cache +### ::: Remove standalone +### +opam-version: "2.0" +remove: "false" +### opam install upgrade +The following actions will be performed: +=== install 1 package + - install upgrade 1 + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +-> installed upgrade.1 +Done. +### opam-cache installed action-failure +=> upgrade.1 <= +opam-version: "2.0" +name: "upgrade" +version: "1" +remove: "false" +=> always-here.1 <= +opam-version: "2.0" +name: "always-here" +version: "1" +### opam remove upgrade | grep -v " #" | '^\[WARNING] package uninstall script failed at .*' -> '[WARNING] package uninstall script failed' +The following actions will be performed: +=== remove 1 package + - remove upgrade 1 + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +[WARNING] package uninstall script failed + +Done. +### test -f ${BASEDIR}/OPAM/action-failure/.opam-switch/packages/upgrade.1/opam +# Return code 1 # +### opam-cache installed action-failure +No cache +### ::: Remove on reinstall +### +opam-version: "2.0" +### opam install upgrade +The following actions will be performed: +=== install 1 package + - install upgrade 1 + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +-> installed upgrade.1 +Done. +### opam-cache installed action-failure +=> upgrade.1 <= +opam-version: "2.0" +name: "upgrade" +version: "1" +=> always-here.1 <= +opam-version: "2.0" +name: "always-here" +version: "1" +### +opam-version: "2.0" +remove: "false" +### opam upgrade +The following actions will be performed: +=== recompile 1 package + - recompile upgrade 1 [upstream or system changes] + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +-> removed upgrade.1 +-> installed upgrade.1 +Done. +### cat ${BASEDIR}/OPAM/action-failure/.opam-switch/packages/upgrade.1/opam +opam-version: "2.0" +remove: "false" +### opam-cache installed action-failure +=> upgrade.1 <= +opam-version: "2.0" +name: "upgrade" +version: "1" +remove: "false" +=> always-here.1 <= +opam-version: "2.0" +name: "always-here" +version: "1" +### +opam-version: "2.0" +### opam upgrade | grep -v " #" | '^\[WARNING] package uninstall script failed at .*' -> '[WARNING] package uninstall script failed' +The following actions will be performed: +=== recompile 1 package + - recompile upgrade 1 [upstream or system changes] + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +[WARNING] package uninstall script failed + +-> installed upgrade.1 +Done. +### cat ${BASEDIR}/OPAM/action-failure/.opam-switch/packages/upgrade.1/opam +opam-version: "2.0" +### opam-cache installed action-failure +=> upgrade.1 <= +opam-version: "2.0" +name: "upgrade" +version: "1" +=> always-here.1 <= +opam-version: "2.0" +name: "always-here" +version: "1" +### ::: Fetch +### opam remove upgrade +The following actions will be performed: +=== remove 1 package + - remove upgrade 1 + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +-> removed upgrade.1 +Done. +### +opam-version: "2.0" +### opam install upgrade +The following actions will be performed: +=== install 1 package + - install upgrade 1 + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +-> installed upgrade.1 +Done. +### opam-cache installed action-failure +=> upgrade.1 <= +opam-version: "2.0" +name: "upgrade" +version: "1" +=> always-here.1 <= +opam-version: "2.0" +name: "always-here" +version: "1" +### +opam-version: "2.0" +url { src:"/inexistent/path" } +### opam install upgrade | '"C:/' -> '"/' | ' C:/' -> ' /' +The following actions will be performed: +=== recompile 1 package + - recompile upgrade 1 + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +[ERROR] Failed to get sources of upgrade.1: /inexistent/path + +OpamSolution.Fetch_fail("/inexistent/path") + + +<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><> ++- The following actions failed +| - fetch upgrade 1 ++- +- No changes have been performed +# Return code 40 # +### cat ${BASEDIR}/OPAM/action-failure/.opam-switch/packages/upgrade.1/opam +opam-version: "2.0" +### opam-cache installed action-failure +=> upgrade.1 <= +opam-version: "2.0" +name: "upgrade" +version: "1" +=> always-here.1 <= +opam-version: "2.0" +name: "always-here" +version: "1" diff --git a/tests/reftests/dune.inc b/tests/reftests/dune.inc index 0b1cc01b36e..643bbb93218 100644 --- a/tests/reftests/dune.inc +++ b/tests/reftests/dune.inc @@ -146,6 +146,27 @@ %{targets} (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:best-effort.test} %{read-lines:testing-env})))) +(rule + (alias reftest-cache) + (enabled_if (and (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1)))) + (action + (diff cache.test cache.out))) + +(alias + (name reftest) + (enabled_if (and (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1)))) + (deps (alias reftest-cache))) + +(rule + (targets cache.out) + (deps root-N0REP0) + (enabled_if (and (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1)))) + (package opam) + (action + (with-stdout-to + %{targets} + (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:cache.test} %{read-lines:testing-env})))) + (rule (alias reftest-clean) (enabled_if (and (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1))))