From d137d3b52c0374d11561ffb9f1f5bae20ae01f98 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Tue, 31 Jan 2023 07:13:15 +0200 Subject: Improve Debian diagnostics --- bpkg/system-package-manager-debian.cxx | 231 ++++++++++++--------- bpkg/system-package-manager-debian.test.testscript | 8 +- 2 files changed, 140 insertions(+), 99 deletions(-) diff --git a/bpkg/system-package-manager-debian.cxx b/bpkg/system-package-manager-debian.cxx index 71d3596..4b4dfa9 100644 --- a/bpkg/system-package-manager-debian.cxx +++ b/bpkg/system-package-manager-debian.cxx @@ -971,6 +971,9 @@ namespace bpkg // The main argument specifies the size of the main group. Only components // from this group are considered for partially_installed determination. // + // @@ TODO: we should probably prioritize partially installed with fully + // installed main group. Add almost_installed next to partially_installed? + // using status_type = package_status::status_type; auto status = [] (const vector& pps, size_t main) @@ -1002,62 +1005,70 @@ namespace bpkg // optional r; - for (package_status& ps: candidates) { - vector& pps (ps.package_policies); + diag_record dr; // Ambiguity diagnostics. - if (!ps.main.empty ()) pps.emplace_back (ps.main); - if (!ps.dev.empty ()) pps.emplace_back (ps.dev); - if (!ps.doc.empty () && need_doc) pps.emplace_back (ps.doc); - if (!ps.dbg.empty () && need_dbg) pps.emplace_back (ps.dbg); - if (!ps.common.empty () && false) pps.emplace_back (ps.common); - ps.package_policies_main = pps.size (); - for (const string& n: ps.extras) pps.emplace_back (n); + for (package_status& ps: candidates) + { + vector& pps (ps.package_policies); - apt_cache_policy (pps); + if (!ps.main.empty ()) pps.emplace_back (ps.main); + if (!ps.dev.empty ()) pps.emplace_back (ps.dev); + if (!ps.doc.empty () && need_doc) pps.emplace_back (ps.doc); + if (!ps.dbg.empty () && need_dbg) pps.emplace_back (ps.dbg); + if (!ps.common.empty () && false) pps.emplace_back (ps.common); + ps.package_policies_main = pps.size (); + for (const string& n: ps.extras) pps.emplace_back (n); - // Handle the unknown main package. - // - if (ps.main.empty ()) - { - const package_policy& dev (pps.front ()); + apt_cache_policy (pps); - // Note that at this stage we can only use the installed dev package - // (since the candidate version may change after fetch). + // Handle the unknown main package. // - if (dev.installed_version.empty ()) - continue; + if (ps.main.empty ()) + { + const package_policy& dev (pps.front ()); - guess_main (ps, dev.installed_version); - pps.emplace (pps.begin (), ps.main); - ps.package_policies_main++; - apt_cache_policy (pps, 1); - } + // Note that at this stage we can only use the installed dev package + // (since the candidate version may change after fetch). + // + if (dev.installed_version.empty ()) + continue; - optional s (status (pps, ps.package_policies_main)); + guess_main (ps, dev.installed_version); + pps.emplace (pps.begin (), ps.main); + ps.package_policies_main++; + apt_cache_policy (pps, 1); + } - if (!s) - continue; + optional s (status (pps, ps.package_policies_main)); + + if (!s || *s != package_status::installed) + continue; - if (*s == package_status::installed) - { const package_policy& main (pps.front ()); ps.status = *s; ps.system_name = main.name; ps.system_version = main.installed_version; - if (r) + if (!r) { - fail << "multiple installed " << os_release_.name_id - << " packages for " << pn << - info << "first package: " << r->main << " " << r->system_version << - info << "second package: " << ps.main << " " << ps.system_version << - info << "consider specifying the desired version manually"; + r = move (ps); + continue; } - r = move (ps); + if (dr.empty ()) + { + dr << fail << "multiple installed " << os_release_.name_id + << " packages for " << pn << + info << "candidate: " << r->main << " " << r->system_version; + } + + dr << info << "candidate: " << ps.main << " " << ps.system_version; } + + if (!dr.empty ()) + dr << info << "consider specifying the desired version manually"; } // Next look for available versions if we are allowed to install. @@ -1074,77 +1085,100 @@ namespace bpkg fetched_ = true; } - for (package_status& ps: candidates) { - vector& pps (ps.package_policies); - - if (requery) - apt_cache_policy (pps); + diag_record dr; // Ambiguity diagnostics. - // Handle the unknown main package. - // - if (ps.main.empty ()) + for (package_status& ps: candidates) { - const package_policy& dev (pps.front ()); + vector& pps (ps.package_policies); + + if (requery) + apt_cache_policy (pps); - // Note that this time we use the candidate version. + // Handle the unknown main package. // - if (dev.candidate_version.empty ()) - continue; // Not installable. + if (ps.main.empty ()) + { + const package_policy& dev (pps.front ()); - guess_main (ps, dev.candidate_version); - pps.emplace (pps.begin (), ps.main); - ps.package_policies_main++; - apt_cache_policy (pps, 1); - } + // Note that this time we use the candidate version. + // + if (dev.candidate_version.empty ()) + continue; // Not installable. - optional s (status (pps, ps.package_policies_main)); + guess_main (ps, dev.candidate_version); + pps.emplace (pps.begin (), ps.main); + ps.package_policies_main++; + apt_cache_policy (pps, 1); + } - if (!s) - { - ps.main.clear (); // Not installable. - continue; - } + optional s (status (pps, ps.package_policies_main)); - assert (*s != package_status::installed); // Sanity check. + if (!s) + { + ps.main.clear (); // Not installable. + continue; + } - const package_policy& main (pps.front ()); + assert (*s != package_status::installed); // Sanity check. - // Note that if we are installing something for this main package, - // then we always go for the candidate version even though it may have - // an installed version that may be good enough (especially if what we - // are installing are extras). The reason is that it may as well not - // be good enough (especially if we are installing the -dev package) - // and there is no straightforward way to change our mind. - // - ps.status = *s; - ps.system_name = main.name; - ps.system_version = main.candidate_version; + const package_policy& main (pps.front ()); - // Prefer partially installed to not installed. This makes detecting - // ambiguity a bit trickier so we handle partially installed here and - // not installed in a separate loop below. - // - if (ps.status == package_status::partially_installed) - { - if (r) + // Note that if we are installing something for this main package, + // then we always go for the candidate version even though it may + // have an installed version that may be good enough (especially if + // what we are installing are extras). The reason is that it may as + // well not be good enough (especially if we are installing the -dev + // package) and there is no straightforward way to change our mind. + // + ps.status = *s; + ps.system_name = main.name; + ps.system_version = main.candidate_version; + + // Prefer partially installed to not installed. This makes detecting + // ambiguity a bit trickier so we handle partially installed here + // and not installed in a separate loop below. + // + if (ps.status != package_status::partially_installed) + continue; + + if (!r) { - // @@ TODO show missing components (like -dev, etc). - - fail << "multiple partially installed " << os_release_.name_id - << " packages for " << pn << - info << "first package: " << r->main << " " << r->system_version << - info << "second package: " << ps.main << " " << ps.system_version << - info << "consider fully installing the desired package manually " - << "and retrying the bpkg command"; + r = move (ps); + continue; } - r = move (ps); + auto print_missing = [&dr] (const package_status& s) + { + for (const package_policy& pp: s.package_policies) + if (pp.installed_version.empty ()) + dr << ' ' << pp.name; + }; + + if (dr.empty ()) + { + dr << fail << "multiple partially installed " + << os_release_.name_id << " packages for " << pn; + + dr << info << "candidate: " << r->main << " " << r->system_version + << ", missing components:"; + print_missing (*r); + } + + dr << info << "candidate: " << ps.main << " " << ps.system_version + << ", missing components:"; + print_missing (ps); } + + if (!dr.empty ()) + dr << info << "consider fully installing the desired package " + << "manually and retrying the bpkg command"; } if (!r) { + diag_record dr; // Ambiguity diagnostics. + for (package_status& ps: candidates) { if (ps.main.empty ()) @@ -1152,18 +1186,25 @@ namespace bpkg assert (ps.status == package_status::not_installed); // Sanity check. - if (r) + if (!r) { - fail << "multiple available " << os_release_.name_id - << " packages for " << pn << - info << "first package: " << r->main << " " << r->system_version << - info << "second package: " << ps.main << " " << ps.system_version << - info << "consider installing the desired package manually " - << "and retrying the bpkg command"; + r = move (ps); + continue; } - r = move (ps); + if (dr.empty ()) + { + dr << fail << "multiple available " << os_release_.name_id + << " packages for " << pn << + info << "candidate: " << r->main << " " << r->system_version; + } + + dr << info << "candidate: " << ps.main << " " << ps.system_version; } + + if (!dr.empty ()) + dr << info << "consider installing the desired package manually and " + << "retrying the bpkg command"; } } diff --git a/bpkg/system-package-manager-debian.test.testscript b/bpkg/system-package-manager-debian.test.testscript index 1a3b8b2..6d3094c 100644 --- a/bpkg/system-package-manager-debian.test.testscript +++ b/bpkg/system-package-manager-debian.test.testscript @@ -870,8 +870,8 @@ LC_ALL=C apt-cache policy --quiet libcurl4 libcurl4-openssl-dev