From d0ec9f6f7b241638b23eb51e8d1f43291c5ac0fa Mon Sep 17 00:00:00 2001 From: Kate Date: Thu, 23 Jan 2025 14:18:42 +0000 Subject: [PATCH 1/4] reftest: add a test showing opam upgrade upgrading unrelated packages when it shouldn't --- master_changes.md | 1 + tests/reftests/upgrade.test | 79 +++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/master_changes.md b/master_changes.md index 0fce4742cb1..ace9c618a0c 100644 --- a/master_changes.md +++ b/master_changes.md @@ -171,6 +171,7 @@ users) * Make sure `download.test` does not fail due to a checksum collision in the download cache [#6378 @kit-ty-kate] * Add a test showing the behaviour of `opam upgrade` with packages flagged with `avoid-version`/`deprecated` [#6273 @kit-ty-kate] * Add a test showing the behaviour when a pin depend is unpinned [#6380 @rjbou] + * Add a test to ensure `opam upgrade ` will not upgrade unrelated things [#6373 @kit-ty-kate] ### Engine diff --git a/tests/reftests/upgrade.test b/tests/reftests/upgrade.test index 261efdea85d..4d4fada84a4 100644 --- a/tests/reftests/upgrade.test +++ b/tests/reftests/upgrade.test @@ -430,3 +430,82 @@ Done. <><> avoid-version.3 installed successfully <><><><><><><><><><><><><><><><><><> => Note: This package is deprecated. +### ::::::::::::::::::::::: +### : Make sure opam upgrade only tries to upgrade and not anything else +### : TODO: broken +### ::::::::::::::::::::::: +### opam upgrade +Already up-to-date. +Nothing to do. +### +opam-version: "2.0" +### +opam-version: "2.0" +### +opam-version: "2.0" +depends: "foo" +### +opam-version: "2.0" +### opam upgrade --show foo +The following actions would be performed: +=== upgrade 2 packages + - upgrade baz 1 to 2 [uses foo] + - upgrade foo 3 to 4 +### opam upgrade --show -a foo +The following actions would be performed: +=== upgrade 3 packages + - upgrade bar 2 to 3 + - upgrade baz 1 to 2 [uses foo] + - upgrade foo 3 to 4 +### opam upgrade --show bar +The following actions would be performed: +=== upgrade 1 package + - upgrade bar 2 to 3 +### opam upgrade --show -a bar +The following actions would be performed: +=== upgrade 3 packages + - upgrade bar 2 to 3 + - upgrade baz 1 to 2 [uses foo] + - upgrade foo 3 to 4 +### opam upgrade --show baz +The following actions would be performed: +=== upgrade 2 packages + - upgrade baz 1 to 2 + - upgrade foo 3 to 4 [required by baz] +### opam upgrade --show -a baz +The following actions would be performed: +=== upgrade 3 packages + - upgrade bar 2 to 3 + - upgrade baz 1 to 2 + - upgrade foo 3 to 4 [required by baz] +### opam upgrade --show uninstalled +uninstalled is not installed. Install it? [Y/n] y +The following actions would be performed: +=== install 1 package + - install uninstalled 1 +### opam upgrade --show -a uninstalled +uninstalled is not installed. Install it? [Y/n] y +The following actions would be performed: +=== install 1 package + - install uninstalled 1 +### opam upgrade --show +The following actions would be performed: +=== upgrade 3 packages + - upgrade bar 2 to 3 + - upgrade baz 1 to 2 + - upgrade foo 3 to 4 +### opam upgrade baz +The following actions will be performed: +=== upgrade 2 packages + - upgrade baz 1 to 2 + - upgrade foo 3 to 4 [required by baz] + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +-> removed baz.1 +-> removed foo.3 +-> installed foo.4 +-> installed baz.2 +Done. +### opam upgrade --show foo +Already up-to-date. +Nothing to do. From 7b29e4eab300073b706b3741a77ac3e68989add9 Mon Sep 17 00:00:00 2001 From: Louis Gesbert Date: Wed, 22 Jan 2025 14:24:24 +0100 Subject: [PATCH 2/4] Fix regression on 'opam upgrade ' upgrading unrelated things Regression introduced in 4db0c139ce184e7f49bdde63c8ffb6dbd1bb87d4, which is dedicated to a different case (functionality of this patch is preserved) --- master_changes.md | 1 + src/client/opamClient.ml | 2 +- tests/reftests/upgrade.test | 19 ++++++++----------- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/master_changes.md b/master_changes.md index ace9c618a0c..6fb01defb37 100644 --- a/master_changes.md +++ b/master_changes.md @@ -59,6 +59,7 @@ users) ## Update / Upgrade * [BUG] Do not show the not-up-to-date message with packages tagged with avoid-version [#6273 @kit-ty-kate - fix #6271] + * [BUG] Fix a regression on `opam upgrade ` upgrading unrelated packages [#6373 @AltGr] ## Tree diff --git a/src/client/opamClient.ml b/src/client/opamClient.ml index 730a0e6e9c7..91d03899616 100644 --- a/src/client/opamClient.ml +++ b/src/client/opamClient.ml @@ -113,7 +113,7 @@ let compute_upgrade_t | Some nv -> not (OpamPackage.Set.mem nv (Lazy.force t.available_packages))) atoms in - let criteria = if to_install = [] then `Upgrade else `Default in + let criteria = if to_install = [] && all then `Upgrade else `Default in if all then names, OpamSolution.resolve t Upgrade diff --git a/tests/reftests/upgrade.test b/tests/reftests/upgrade.test index 4d4fada84a4..7fd6a847d67 100644 --- a/tests/reftests/upgrade.test +++ b/tests/reftests/upgrade.test @@ -432,7 +432,6 @@ Done. => Note: This package is deprecated. ### ::::::::::::::::::::::: ### : Make sure opam upgrade only tries to upgrade and not anything else -### : TODO: broken ### ::::::::::::::::::::::: ### opam upgrade Already up-to-date. @@ -448,8 +447,7 @@ depends: "foo" opam-version: "2.0" ### opam upgrade --show foo The following actions would be performed: -=== upgrade 2 packages - - upgrade baz 1 to 2 [uses foo] +=== upgrade 1 package - upgrade foo 3 to 4 ### opam upgrade --show -a foo The following actions would be performed: @@ -469,9 +467,8 @@ The following actions would be performed: - upgrade foo 3 to 4 ### opam upgrade --show baz The following actions would be performed: -=== upgrade 2 packages +=== upgrade 1 package - upgrade baz 1 to 2 - - upgrade foo 3 to 4 [required by baz] ### opam upgrade --show -a baz The following actions would be performed: === upgrade 3 packages @@ -496,16 +493,16 @@ The following actions would be performed: - upgrade foo 3 to 4 ### opam upgrade baz The following actions will be performed: -=== upgrade 2 packages +=== upgrade 1 package - upgrade baz 1 to 2 - - upgrade foo 3 to 4 [required by baz] <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> -> removed baz.1 --> removed foo.3 --> installed foo.4 -> installed baz.2 Done. ### opam upgrade --show foo -Already up-to-date. -Nothing to do. +The following actions would be performed: +=== recompile 1 package + - recompile baz 2 [uses foo] +=== upgrade 1 package + - upgrade foo 3 to 4 From f595d2522b3e1713d9c0a3d5273be27671feaf75 Mon Sep 17 00:00:00 2001 From: Kate Date: Thu, 23 Jan 2025 02:06:07 +0000 Subject: [PATCH 3/4] Simplify OpamClient.compute_upgrade_t --- src/client/opamClient.ml | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/client/opamClient.ml b/src/client/opamClient.ml index 91d03899616..11a37e05990 100644 --- a/src/client/opamClient.ml +++ b/src/client/opamClient.ml @@ -113,27 +113,16 @@ let compute_upgrade_t | Some nv -> not (OpamPackage.Set.mem nv (Lazy.force t.available_packages))) atoms in - let criteria = if to_install = [] && all then `Upgrade else `Default in - if all then - names, - OpamSolution.resolve t Upgrade - ~requested:packages - ~reinstall:(Lazy.force t.reinstall) - (OpamSolver.request - ~install:to_install - ~upgrade:to_upgrade - ~deprequest:(OpamFormula.to_atom_formula formula) - ~all:[] - ~criteria ()) - else names, OpamSolution.resolve t Upgrade ~requested:packages + ?reinstall:(if all then Some (Lazy.force t.reinstall) else None) (OpamSolver.request ~install:to_install ~upgrade:to_upgrade ~deprequest:(OpamFormula.to_atom_formula formula) - ~criteria + ?all:(if all then Some [] else None) + ~criteria:(if to_install = [] && all then `Upgrade else `Default) ()) let print_requested requested formula = From 15ed387e1d7929a935e8bcb2d690d0d4d0ad737a Mon Sep 17 00:00:00 2001 From: Kate Date: Fri, 24 Jan 2025 21:04:06 +0000 Subject: [PATCH 4/4] Fix a regression on 'opam upgrade --all ' not upgrading the whole switch --- master_changes.md | 1 + src/client/opamClient.ml | 2 +- tests/reftests/upgrade.test | 4 ++++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/master_changes.md b/master_changes.md index 6fb01defb37..bbaee0e09de 100644 --- a/master_changes.md +++ b/master_changes.md @@ -60,6 +60,7 @@ users) ## Update / Upgrade * [BUG] Do not show the not-up-to-date message with packages tagged with avoid-version [#6273 @kit-ty-kate - fix #6271] * [BUG] Fix a regression on `opam upgrade ` upgrading unrelated packages [#6373 @AltGr] + * [BUG] Fix a regression on `opam upgrade --all ` not upgrading the whole switch [#6373 @kit-ty-kate] ## Tree diff --git a/src/client/opamClient.ml b/src/client/opamClient.ml index 11a37e05990..efb390778a4 100644 --- a/src/client/opamClient.ml +++ b/src/client/opamClient.ml @@ -122,7 +122,7 @@ let compute_upgrade_t ~upgrade:to_upgrade ~deprequest:(OpamFormula.to_atom_formula formula) ?all:(if all then Some [] else None) - ~criteria:(if to_install = [] && all then `Upgrade else `Default) + ~criteria:(if all then `Upgrade else `Default) ()) let print_requested requested formula = diff --git a/tests/reftests/upgrade.test b/tests/reftests/upgrade.test index 7fd6a847d67..12636e06c46 100644 --- a/tests/reftests/upgrade.test +++ b/tests/reftests/upgrade.test @@ -483,6 +483,10 @@ The following actions would be performed: ### opam upgrade --show -a uninstalled uninstalled is not installed. Install it? [Y/n] y The following actions would be performed: +=== upgrade 3 packages + - upgrade bar 2 to 3 + - upgrade baz 1 to 2 [uses foo] + - upgrade foo 3 to 4 === install 1 package - install uninstalled 1 ### opam upgrade --show