From 5d605a5fd2e94cd5ecb62b860da01eb7fa884062 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Fri, 29 Apr 2022 12:10:57 +0300 Subject: Fix collect_order_dependents() not to verify constraints of being dropped dependent --- bpkg/pkg-build.cxx | 51 ++++++++----------- tests/common/satisfy/t12b/baz-0.1.0.tar.gz | Bin 0 -> 359 bytes tests/common/satisfy/t12b/baz-1.0.0.tar.gz | Bin 0 -> 366 bytes tests/common/satisfy/t12b/foo-0.1.0.tar.gz | Bin 0 -> 357 bytes tests/pkg-build.testscript | 77 +++++++++++++++++++++++++++++ 5 files changed, 97 insertions(+), 31 deletions(-) create mode 100644 tests/common/satisfy/t12b/baz-0.1.0.tar.gz create mode 100644 tests/common/satisfy/t12b/baz-1.0.0.tar.gz create mode 100644 tests/common/satisfy/t12b/foo-0.1.0.tar.gz diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index b58a25d..9ca992a 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -2146,10 +2146,8 @@ namespace bpkg const map* rpt_prereq_flags (nullptr); // Bail out if this is a configured non-system package and no - // up/down-grade nor collecting prerequisite replacements are required. - // - // NOTE: remember to update the check condition in - // collect_order_dependents() if changing anything here. + // up/down-grade, reconfiguration, nor collecting prerequisite + // replacements are required. // bool src_conf (sp != nullptr && sp->state == package_state::configured && @@ -4720,8 +4718,8 @@ namespace bpkg auto i (map_.find (ddb, dn)); // Make sure the up/downgraded package still satisfies this - // dependent. But first "prune" if this is a replaced prerequisite - // of the repointed dependent. + // dependent. But first "prune" if the dependent is being dropped or + // this is a replaced prerequisite of the repointed dependent. // // Note that the repointed dependents are always collected and have // all their collected prerequisites ordered (including new and old @@ -4731,6 +4729,13 @@ namespace bpkg if (i != map_.end () && i->second.position != end ()) { + build_package& dp (i->second.package); + + // Skip the droped dependent. + // + if (dp.action && *dp.action == build_package::drop) + continue; + repointed_dependents::const_iterator j ( rpt_depts.find (config_package {ddb, dn})); @@ -4744,24 +4749,14 @@ namespace bpkg continue; } - build_package& dp (i->second.package); - const shared_ptr& dsp (dp.selected); - // There is one tricky aspect: the dependent could be in the - // process of being up/downgraded as well. In this case all we - // need to do is detect this situation and skip the test since all - // the (new) contraints of this package have been satisfied in - // collect_build(). + // process of being reconfigured or up/downgraded as well. In this + // case all we need to do is detect this situation and skip the + // test since all the (new) constraints of this package have been + // satisfied in collect_build(). // if (check) - { - check = dp.available == nullptr || - (dsp->system () == dp.system && - dsp->version == dp.available_version () && - (dp.system || - dp.config_vars.empty () || - !has_buildfile_clause (dp.available->dependencies))); - } + check = !dp.dependencies; } if (check) @@ -4845,11 +4840,10 @@ namespace bpkg // list, the package is in the map (but not on the list) and it // is in neither. // - // If the existing entry is a drop, then we skip it. If it is - // pre-entered, is an adjustment, or is a build that is not supposed - // to be built (not in the list), then we merge it into the new - // adjustment entry. Otherwise (is a build in the list), we just add - // the reconfigure adjustment flag to it. + // If the existing entry is pre-entered, is an adjustment, or is a + // build that is not supposed to be built (not in the list), then we + // merge it into the new adjustment entry. Otherwise (is a build in + // the list), we just add the reconfigure adjustment flag to it. // if (i != map_.end ()) { @@ -4860,11 +4854,6 @@ namespace bpkg *dp.action != build_package::build || // Non-build. dpos == end ()) // Build not in the list. { - // Skip the droped package. - // - if (dp.action && *dp.action == build_package::drop) - continue; - build_package bp (adjustment ()); bp.merge (move (dp)); dp = move (bp); diff --git a/tests/common/satisfy/t12b/baz-0.1.0.tar.gz b/tests/common/satisfy/t12b/baz-0.1.0.tar.gz new file mode 100644 index 0000000..2676c52 Binary files /dev/null and b/tests/common/satisfy/t12b/baz-0.1.0.tar.gz differ diff --git a/tests/common/satisfy/t12b/baz-1.0.0.tar.gz b/tests/common/satisfy/t12b/baz-1.0.0.tar.gz new file mode 100644 index 0000000..1aec461 Binary files /dev/null and b/tests/common/satisfy/t12b/baz-1.0.0.tar.gz differ diff --git a/tests/common/satisfy/t12b/foo-0.1.0.tar.gz b/tests/common/satisfy/t12b/foo-0.1.0.tar.gz new file mode 100644 index 0000000..a282f20 Binary files /dev/null and b/tests/common/satisfy/t12b/foo-0.1.0.tar.gz differ diff --git a/tests/pkg-build.testscript b/tests/pkg-build.testscript index 79cce38..1aa3a95 100644 --- a/tests/pkg-build.testscript +++ b/tests/pkg-build.testscript @@ -227,8 +227,11 @@ # |-- t12b -> t12b (prerequisite repository) # | |-- libbaz-0.1.0.tar.gz # | |-- libbar-1.0.0.tar.gz -> libbaz == 0.1.0 +# | |-- foo-0.1.0.tar.gz # | |-- foo-1.0.0.tar.gz -> libbar # | |-- bar-1.0.0.tar.gz -> libbar == 0.1.0 +# | |-- baz-0.1.0.tar.gz -> libbaz +# | |-- baz-1.0.0.tar.gz -> libbaz == 1.0.0 # | `-- repositories.manifest # | # `-- git @@ -4630,6 +4633,80 @@ test.options += --no-progress } } + : drop-dependent + : + { + +$clone_root_cfg && $rep_add $rep/t12b && $rep_fetch + + test.arguments += --yes + + : unhold + : + : Test that the being dropped dependent does not constrain a dependency + : anymore. + : + { + $clone_cfg; + + $* libbar 2>!; + + $pkg_status -r >>EOO; + !libbar configured 1.0.0 + libbaz configured 0.1.0 available [1.0.0] + EOO + + $* baz/0.1.0 ?libbar ?libbaz 2>!; + + $pkg_status -r >>EOO; + !baz configured !0.1.0 available 1.0.0 + libbaz configured 1.0.0 + EOO + + $pkg_drop baz + } + + : unuse + : + : Unlike the previous test, at the time we check the constraint applied + : by libbar on libbaz (== 0.1.0) there is no evidence that libbar will be + : dropped, which will happen some later execution plan refinement + : iteration. + : + : @@ This scenario is not supported yet and fails with: + : + : error: unable to upgrade package libbaz/0.1.0 to 1.0.0 + : info: because package libbar depends on (libbaz == 0.1.0) + : info: package libbaz/1.0.0 required by baz + : info: explicitly request up/downgrade of package libbar + : info: or explicitly specify package libbaz version to manually satisfy these constraints + : + : We could probably fix this postponing the constraints check in + : collect_order_dependents() until the final execution plan is produced + : (after all that refinement iterations). We could have an additional + : iteration after all the refinements which would enable the constraint + : check in collect_order_dependents(). + : + if false + { + $clone_cfg; + + $* foo 2>!; + + $pkg_status -r >>EOO; + !foo configured 1.0.0 + libbar configured 1.0.0 + libbaz configured 0.1.0 available [1.0.0] + EOO + + $* baz foo/0.1.0 2>|; + + $pkg_status -r >>EOO; + EOO + + $pkg_drop foo + } + } + : configuration-negotiation : { -- cgit v1.1