From 0072c2109599d2d46dbde4f84a5957487d6158e7 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Sat, 23 Apr 2016 16:14:59 +0200 Subject: Delay checking dependents until we know which of them are up/downgraded --- bpkg/pkg-build.cxx | 126 ++++++++++++++++++++++++++++++++--------------------- tests/test.sh | 12 ++--- 2 files changed, 82 insertions(+), 56 deletions(-) diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index d516441..f4d4699 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -174,9 +174,9 @@ namespace bpkg // "upgrade" or "downgrade" a package that is already in a map as a // result of another package depending on it and, for example, requiring // a different version. One notable side-effect of this process is that - // we may end up with a lot more packages in the map than we will have - // on the list. This is because some of the prerequisites of "upgraded" - // or "downgraded" packages may no longer need to be built. + // we may end up with a lot more packages in the map (but not in the list) + // than we will have on the list. This is because some of the prerequisites + // of "upgraded" or "downgraded" packages may no longer need to be built. // // Note also that we don't try to do exhaustive constraint satisfaction // (i.e., there is no backtracking). Specifically, if we have two @@ -373,51 +373,12 @@ namespace bpkg } else { + // This is the first time we are adding this package name to the map. + // string n (pkg.available->id.name); // Note: copy; see emplace() below. l4 ([&]{trace << "add " << n << " " << pkg.available->version;}); - // This is the first time we are adding this package name to the - // map. If it is already selected, then we need to make sure that - // packages that already depend on it (called dependents) are ok - // with the up/downgrade. We will also have to keep doing this - // every time we choose a new available package above. So what - // we are going to do is copy the dependents' constrains over to - // our constraint list; this way they will be automatically taken - // into account by the rest of the logic. - // - const shared_ptr& sp (pkg.selected); - const shared_ptr& ap (pkg.available); - - int r; - if (sp != nullptr && - sp->state == package_state::configured && - (r = sp->version.compare (ap->version)) != 0) - { - using query = query; - - for (const auto& pd: db.query (query::name == n)) - { - if (!pd.constraint) - continue; - - const version& v (ap->version); - const dependency_constraint& c (*pd.constraint); - - if (satisfies (v, c)) - { - pkg.constraints.emplace_back (pd.name, c); - continue; - } - - fail << "unable to " << (r < 0 ? "up" : "down") << "grade " - << "package " << n << " " << sp->version << " to " << v << - info << pd.name << " depends on (" << n << " " << c << ")" << - info << "explicitly specify " << n << " version to manually " - << "satisfy this constraint"; - } - } - i = map_.emplace (move (n), data_type {end (), move (pkg)}).first; } @@ -659,7 +620,8 @@ namespace bpkg // If a configured package is being up/down-graded then that means // all its dependents could be affected and we have to reconfigure // them. This function examines every package that is already on - // the list and collects and orders all its dependents. + // the list and collects and orders all its dependents. We also need + // to make sure the dependents are ok with the up/downgrade. // // Should we reconfigure just the direct depends or also include // indirect, recursively? Consider this plauisible scenario as an @@ -697,26 +659,90 @@ namespace bpkg { tracer trace ("collect_order_dependents"); - const build_package& p (*pos); - const string& n (p.selected->name); + build_package& p (*pos); + const shared_ptr& sp (p.selected); + const shared_ptr& ap (p.available); + + const string& n (sp->name); + + // See if we are up/downgrading this package. In particular, the + // available package could be NULL meaning we are just reconfiguring. + // + int ud (ap != nullptr ? sp->version.compare (ap->version) : 0); using query = query; for (auto& pd: db.query (query::name == n)) { string& dn (pd.name); + auto i (map_.find (dn)); + + // First make sure the up/downgraded package still satisfies this + // dependent. + // + bool check (ud != 0 && pd.constraint); + + // 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(). + // + if (check && i != map_.end () && i->second.position != end ()) + { + build_package& dp (i->second.package); + + check = dp.available == nullptr || + dp.selected->version == dp.available->version; + } + + if (check) + { + const version& v (ap->version); + const dependency_constraint& c (*pd.constraint); + + if (!satisfies (v, c)) + { + diag_record dr; + + dr << fail << "unable to " << (ud < 0 ? "up" : "down") << "grade " + << "package " << n << " " << sp->version << " to " << v; + + dr << info << "because package " << dn << " depends on (" << n + << " " << c << ")"; + + string rb; + if (p.required_by.find ("") == p.required_by.end ()) + { + for (const string& n: p.required_by) + rb += ' ' + n; + } + + if (!rb.empty ()) + dr << info << "package " << n << " " << v << " required by" + << rb; + + dr << info << "explicitly request up/downgrade of package " << dn; + + dr << info << "or explicitly specify package " << n << " version " + << "to manually satisfy these constraints"; + } + + // Add this contraint to the list for completeness. + // + p.constraints.emplace_back (dn, c); + } // We can have three cases here: the package is already on the // list, the package is in the map (but not on the list) and it // is in neither. // - auto i (map_.find (dn)); - if (i != map_.end ()) { + // Now add to the list. + // build_package& dp (i->second.package); - dp.required_by.insert (n); + p.required_by.insert (dn); // Force reconfiguration in both cases. // diff --git a/tests/test.sh b/tests/test.sh index 9345d35..b4905c0 100755 --- a/tests/test.sh +++ b/tests/test.sh @@ -1122,32 +1122,32 @@ test cfg-add $rep/satisfy/t4b test cfg-fetch test pkg-build -p libbar <