From 5b5b63886b0555c9697061601f865dfbced4764f Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Tue, 17 Oct 2023 14:34:51 +0300 Subject: Print version constraints recursively in 'unable to satisfy constraints' diagnostics --- bpkg/pkg-build-collect.cxx | 320 ++++++++++++++++++++++++++++++++------------- bpkg/pkg-build-collect.hxx | 60 ++++++++- bpkg/pkg-build.cxx | 2 +- tests/pkg-build.testscript | 6 + 4 files changed, 294 insertions(+), 94 deletions(-) diff --git a/bpkg/pkg-build-collect.cxx b/bpkg/pkg-build-collect.cxx index 47e097f..fc8d071 100644 --- a/bpkg/pkg-build-collect.cxx +++ b/bpkg/pkg-build-collect.cxx @@ -442,7 +442,7 @@ namespace bpkg } void unsatisfied_dependents:: - diag () + diag (const build_packages& pkgs) { assert (!empty ()); @@ -488,6 +488,13 @@ namespace bpkg dr << info << "because package " << dk << " depends on (" << n << " " << c << ")"; + { + // " info: ..." + string indent (" "); + set printed; + pkgs.print_constraints (dr, dk, indent, printed); + } + string rb; if (!p.user_selection ()) { @@ -1355,19 +1362,28 @@ namespace bpkg // if (auto c1 = test (p2, p1)) { - const package_name& n (i->first.name); - const string& d1 (c1->dependent); - const string& d2 (c2->dependent); - - fail << "unable to satisfy constraints on package " << n << - info << d1 << c1->db << " depends on (" << n << " " - << c1->value << ")" << - info << d2 << c2->db << " depends on (" << n << " " - << c2->value << ")" << - info << "available " << p1->available_name_version () << - info << "available " << p2->available_name_version () << - info << "explicitly specify " << n << " version to manually " - << "satisfy both constraints"; + const package_name& n (i->first.name); + + // " info: ..." + string indent (" "); + set printed; + + diag_record dr (fail); + dr << "unable to satisfy constraints on package " << n << + info << c1->dependent << c1->db << " depends on (" << n << ' ' + << c1->value << ")"; + + print_constraints (dr, *c1, indent, printed); + + dr << info << c2->dependent << c2->db << " depends on (" << n + << ' ' << c2->value << ")"; + + print_constraints (dr, *c2, indent, printed); + + dr << info << "available " << p1->available_name_version () << + info << "available " << p2->available_name_version () << + info << "explicitly specify " << n << " version to " + << "manually satisfy both constraints"; } else swap (p1, p2); @@ -2229,14 +2245,26 @@ namespace bpkg assert (!pre_reeval || dr == nullptr); if (dr != nullptr) + { + // " info: ..." + string indent (" "); + set printed; + *dr << error << "unable to satisfy constraints on package " << dn << - info << nm << pdb << " depends on (" << dn - << " " << *dp.constraint << ")" << - info << c.dependent << c.db << " depends on (" << dn - << " " << c.value << ")" << - info << "specify " << dn << " version to satisfy " << nm - << " constraint"; + info << nm << pdb << " depends on (" << dn << ' ' + << *dp.constraint << ")"; + + print_constraints (*dr, pkg, indent, printed); + + *dr << info << c.dependent << c.db << " depends on (" << dn + << ' ' << c.value << ")"; + + print_constraints (*dr, c, indent, printed); + + *dr << info << "specify " << dn << " version to satisfy " + << nm << " constraint"; + } return precollect_result (false /* postpone */); } @@ -2930,30 +2958,40 @@ namespace bpkg { if (!satisfies (v1, c2.value)) { - // We should end up throwing reevaluation_deviated exception - // before the diagnostics run in the pre-reevaluation - // mode. + // We should end up throwing reevaluation_deviated + // exception before the diagnostics run in the + // pre-reevaluation mode. // assert (!pre_reeval || dr == nullptr); if (dr != nullptr) { - const package_name& n (d.name); - const string& d1 (c1.dependent); - const string& d2 (c2.dependent); + const package_name& n (d.name); + + // " info: ..." + string indent (" "); + set printed; *dr << error << "unable to satisfy constraints on " << "package " << n << - info << d2 << c2.db << " depends on (" << n << ' ' - << c2.value << ")" << - info << d1 << c1.db << " depends on (" << n << ' ' - << c1.value << ")" << - info << "available " - << bp.available_name_version () << - info << "available " - << package_string (n, v1, b.system) << - info << "explicitly specify " << n << " version " - << "to manually satisfy both constraints"; + info << c2.dependent << c2.db << " depends on (" + << n << ' ' << c2.value << ")"; + + print_constraints (*dr, c2, indent, printed); + + *dr << info << c1.dependent << c1.db + << " depends on (" << n << ' ' + << c1.value << ")"; + + print_constraints (*dr, c1, indent, printed); + + *dr << info << "available " + << bp.available_name_version () << + info << "available " + << package_string (n, v1, b.system) << + info << "explicitly specify " << n + << " version to manually satisfy both " + << "constraints"; } return precollect_result (reused, move (r)); @@ -4884,28 +4922,28 @@ namespace bpkg { build_package p { build_package::adjust, - db, - sp, - nullptr, - nullptr, - nullopt, // Dependencies. - nullopt, // Dependencies alternatives. - nullopt, // Package skeleton. - nullopt, // Postponed dependency alternatives. - false, // Recursive collection. - nullopt, // Hold package. - nullopt, // Hold version. - {}, // Constraints. - sp->system (), - false, // Keep output directory. - false, // Disfigure (from-scratch reconf). - false, // Configure-only. - nullopt, // Checkout root. - false, // Checkout purge. - strings (), // Configuration variables. - {}, // Required by. - false, // Required by dependents. - build_package::adjust_unhold}; + db, + sp, + nullptr, + nullptr, + nullopt, // Dependencies. + nullopt, // Dependencies alternatives. + nullopt, // Package skeleton. + nullopt, // Postponed dependency alternatives. + false, // Recursive collection. + nullopt, // Hold package. + nullopt, // Hold version. + {}, // Constraints. + sp->system (), + false, // Keep output directory. + false, // Disfigure (from-scratch reconf). + false, // Configure-only. + nullopt, // Checkout root. + false, // Checkout purge. + strings (), // Configuration variables. + {}, // Required by. + false, // Required by dependents. + build_package::adjust_unhold}; p.merge (move (bp)); bp = move (p); @@ -6537,6 +6575,8 @@ namespace bpkg for (auto& pd: query_dependents_cache (ddb, n, pdb)) { package_name& dn (pd.name); + optional& dc (pd.constraint); + auto i (map_.find (ddb, dn)); // Make sure the up/downgraded package still satisfies this @@ -6547,7 +6587,7 @@ namespace bpkg // all their collected prerequisites ordered (including new and old // ones). See collect_build_prerequisites() and order() for details. // - bool check (ud != 0 && pd.constraint); + bool check (ud != 0 && dc); if (i != map_.end () && i->second.position != end ()) { @@ -6584,7 +6624,7 @@ namespace bpkg if (check) { const version& av (p.available_version ()); - version_constraint& c (*pd.constraint); + const version_constraint& c (*dc); // If the new dependency version doesn't satisfy the existing // dependent, then postpone the failure in the hope that this @@ -6617,20 +6657,14 @@ namespace bpkg assert (i == deps.end () || i->second == c); if (i == deps.end ()) - deps.push_back (make_pair (&p, move (c))); + deps.push_back (make_pair (&p, c)); } else { unsatisfied_depts.push_back ( - unsatisfied_dependent {move (dk), {make_pair (&p, move (c))}}); + unsatisfied_dependent {move (dk), {make_pair (&p, c)}}); } } - else - { - // Add this contraint to the list for completeness. - // - p.constraints.emplace_back (ddb, dn.string (), move (c)); - } } auto adjustment = [&dn, &ddb, &n, &pdb] () -> build_package @@ -6643,28 +6677,28 @@ namespace bpkg return build_package { build_package::adjust, - ddb, - move (dsp), - nullptr, // No available pkg/repo fragment. - nullptr, - nullopt, // Dependencies. - nullopt, // Dependencies alternatives. - nullopt, // Package skeleton. - nullopt, // Postponed dependency alternatives. - false, // Recursive collection. - nullopt, // Hold package. - nullopt, // Hold version. - {}, // Constraints. - false, // System. - false, // Keep output directory. - false, // Disfigure (from-scratch reconf). - false, // Configure-only. - nullopt, // Checkout root. - false, // Checkout purge. - strings (), // Configuration variables. - {package_key {pdb, n}}, // Required by (dependency). - false, // Required by dependents. - build_package::adjust_reconfigure}; + ddb, + move (dsp), + nullptr, // No available pkg/repo fragment. + nullptr, + nullopt, // Dependencies. + nullopt, // Dependencies alternatives. + nullopt, // Package skeleton. + nullopt, // Postponed dependency alternatives. + false, // Recursive collection. + nullopt, // Hold package. + nullopt, // Hold version. + {}, // Constraints. + false, // System. + false, // Keep output directory. + false, // Disfigure (from-scratch reconf). + false, // Configure-only. + nullopt, // Checkout root. + false, // Checkout purge. + strings (), // Configuration variables. + {package_key {pdb, n}}, // Required by (dependency). + false, // Required by dependents. + build_package::adjust_reconfigure}; }; // We can have three cases here: the package is already on the list, @@ -6729,6 +6763,12 @@ namespace bpkg i->second.position = insert (pos, i->second.package); } + // Add this dependent's contraint, if present, to the dependency's + // constraints list for completeness. + // + if (dc) + p.constraints.emplace_back (ddb, move (dn).string (), move (*dc)); + // Recursively collect our own dependents inserting them before us. // // Note that we cannot end up with an infinite recursion for @@ -6760,6 +6800,106 @@ namespace bpkg } void build_packages:: + print_constraints (diag_record& dr, + const build_package& p, + string& indent, + set& printed) const + { + using constraint_type = build_package::constraint_type; + + const vector& cs (p.constraints); + + if (!cs.empty ()) + { + package_key pk (p.db, p.name ()); + + if (printed.find (pk) == printed.end ()) + { + printed.insert (pk); + + for (const constraint_type& c: cs) + { + if (c.dependent != "command line") + { + const build_package* d; + + try + { + d = entered_build (package_key (c.db, + package_name (c.dependent))); + + assert (d != nullptr); // Expected to be collected. + } + catch (const invalid_argument&) + { + // Must be a package name, unless it is 'command line'. + // + assert (false); + } + + dr << '\n' << indent << c.dependent << '/'; + + // The dependent can only be collected as a build or adjustment. + // + assert (d->action && + d->action != build_package::drop && + (d->available || d->selected)); + + dr << (d->available != nullptr + ? d->available_version () + : d->selected->version) << c.db << " requires (" << pk + << ' ' << c.value << ")"; + + indent += " "; + print_constraints (dr, *d, indent, printed); + indent.resize (indent.size () - 2); + } + else + dr << '\n' << indent << c.dependent << " requires (" << pk << ' ' + << c.value << ")"; + } + } + else + { + dr << '\n' << indent << "..."; + } + } + } + + void build_packages:: + print_constraints (diag_record& dr, + const package_key& pk, + string& indent, + set& printed) const + { + const build_package* p (entered_build (pk)); + assert (p != nullptr); // Expected to be collected. + print_constraints (dr, *p, indent, printed); + } + + void build_packages:: + print_constraints (diag_record& dr, + const build_package::constraint_type& c, + string& indent, + set& printed) const + { + const string& d (c.dependent); + + if (d != "command line") + try + { + print_constraints (dr, + package_key (c.db, package_name (d)), + indent, + printed); + } + catch (const invalid_argument&) + { + assert (false); // Must be a package name, unless it is 'command line'. + } + } + + void build_packages:: verify_ordering () const { for (const auto& b: map_) diff --git a/bpkg/pkg-build-collect.hxx b/bpkg/pkg-build-collect.hxx index d5a07b5..d1acc33 100644 --- a/bpkg/pkg-build-collect.hxx +++ b/bpkg/pkg-build-collect.hxx @@ -198,8 +198,13 @@ namespace bpkg // Constraint value plus, normally, the dependent package name that placed // this constraint but can also be some other name for the initial - // selection (e.g., package version specified by the user on the command - // line). This why we use the string type, rather than package_name. + // selection. This is why we use the string type, rather than + // package_name. Currently, the only valid non-package name is "command + // line", which is used when the package version is specified by the user + // on the command line. + // + // Note that if the dependent is a package name, then this package is + // expected to be collected (present in the map). // struct constraint_type { @@ -726,6 +731,8 @@ namespace bpkg vector> dependencies; }; + struct build_packages; + class unsatisfied_dependents: public vector { public: @@ -738,7 +745,7 @@ namespace bpkg // and throw failed. // [[noreturn]] void - diag (); + diag (const build_packages&); }; // List of dependency groups whose recursive processing should be postponed @@ -1110,6 +1117,19 @@ namespace bpkg return entered_build (p.db, p.name); } + const build_package* + entered_build (database& db, const package_name& name) const + { + auto i (map_.find (db, name)); + return i != map_.end () ? &i->second.package : nullptr; + } + + const build_package* + entered_build (const package_key& p) const + { + return entered_build (p.db, p.name); + } + // Collect the package being built. Return its pointer if this package // version was, in fact, added to the map and NULL if it was already there // and the existing version was preferred or if the package build has been @@ -1491,6 +1511,34 @@ namespace bpkg void clear_order (); + // Print all the version constraints (one per line) applied to this + // package and its dependents, recursively. The specified package is + // expected to be collected (present in the map). Don't print the version + // constraints for the same package twice, printing "..." instead. Noop if + // there are no constraints for this package. + // + void + print_constraints (diag_record&, + const build_package&, + string& indent, + std::set& printed) const; + + void + print_constraints (diag_record&, + const package_key&, + string& indent, + std::set& printed) const; + + // Wraps the above function for the case when the package is a dependent + // from the dependency's constraints list. Noop if the dependent is not a + // package name (command line, etc; see constraint_type for details). + // + void + print_constraints (diag_record&, + const build_package::constraint_type&, + string& indent, + std::set& printed) const; + // Verify that builds ordering is consistent across all the data // structures and the ordering expectations are fulfilled (real build // actions are all ordered, etc). @@ -1651,6 +1699,12 @@ namespace bpkg return find (package_key {db, pn}); } + const_iterator + find (database& db, const package_name& pn) const + { + return find (package_key {db, pn}); + } + // Try to find a package build in the dependency configurations (see // database::dependency_configs() for details). Return the end iterator // if no build is found and issue diagnostics and fail if multiple diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index f13c114..ad1a695 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -5308,7 +5308,7 @@ namespace bpkg // dependencies. // if (!refine && !unsatisfied_depts.empty ()) - unsatisfied_depts.diag (); + unsatisfied_depts.diag (pkgs); // Re-link the private configurations that were created during the // collection of the package builds with their parent diff --git a/tests/pkg-build.testscript b/tests/pkg-build.testscript index 9be2ad7..600f0a8 100644 --- a/tests/pkg-build.testscript +++ b/tests/pkg-build.testscript @@ -2679,6 +2679,7 @@ test.arguments += --sys-no-query $* libbar/0.0.1 ?libbaz/0.0.2 2>>EOE != 0; error: unable to satisfy constraints on package libbaz info: libbar depends on (libbaz == 0.0.1) + command line requires (libbar == 0.0.1) info: command line depends on (libbaz == 0.0.2) info: specify libbaz version to satisfy libbar constraint info: while satisfying libbar/0.0.1 @@ -2687,6 +2688,7 @@ test.arguments += --sys-no-query $* -- libbar/0.0.1 '?libbaz>=0.0.2' 2>>EOE != 0 error: unable to satisfy constraints on package libbaz info: libbar depends on (libbaz == 0.0.1) + command line requires (libbar == 0.0.1) info: command line depends on (libbaz >= 0.0.2) info: specify libbaz version to satisfy libbar constraint info: while satisfying libbar/0.0.1 @@ -27524,6 +27526,7 @@ else error: unable to satisfy constraints on package libfoo info: command line depends on (libfoo == 1.0.0) info: libbar depends on (libfoo == 1.1.0) + command line requires (libbar == 1.1.0) info: available libfoo/1.0.0 info: available libfoo/1.1.0 info: explicitly specify libfoo version to manually satisfy both constraints @@ -27953,6 +27956,7 @@ else error: unable to satisfy constraints on package libfoo info: command line depends on (libfoo == 1.0.0) info: libbar depends on (libfoo == 1.1.0) + command line requires (libbar == 1.1.0) info: available libfoo/1.0.0 info: available libfoo/1.1.0 info: explicitly specify libfoo version to manually satisfy both constraints @@ -28342,6 +28346,7 @@ else $* $src/libbar-1.1.0.tar.gz "?$src/libfoo-1.0.0.tar.gz" 2>>~%EOE% != 0 error: unable to satisfy constraints on package libfoo info: libbar depends on (libfoo == 1.1.0) + command line requires (libbar == 1.1.0) info: command line depends on (libfoo == 1.0.0) info: specify libfoo version to satisfy libbar constraint info: while satisfying libbar/1.1.0 @@ -28753,6 +28758,7 @@ else $* $d/libbar-1.1.0/ "?$d/libfoo-1.0.0/" 2>>~%EOE% != 0 error: unable to satisfy constraints on package libfoo info: libbar depends on (libfoo == 1.1.0) + command line requires (libbar == 1.1.0) info: command line depends on (libfoo == 1.0.0) info: specify libfoo version to satisfy libbar constraint info: while satisfying libbar/1.1.0 -- cgit v1.1