From b583457aa83df2c80d94584cd3cba78f0dbce6e9 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Fri, 20 Jan 2023 17:21:53 +0300 Subject: Review-inspired changes --- bpkg/package.hxx | 2 +- bpkg/pkg-build-collect.cxx | 24 ++++++++------ bpkg/pkg-build.cxx | 82 +++++++++++++++++++++++++++------------------- 3 files changed, 63 insertions(+), 45 deletions(-) diff --git a/bpkg/package.hxx b/bpkg/package.hxx index 6afc624..44c3fee 100644 --- a/bpkg/package.hxx +++ b/bpkg/package.hxx @@ -841,7 +841,7 @@ namespace bpkg // #pragma db member(tests) id_column("") value_column("test_") - // distributions + // distribution_values // #pragma db member(distribution_values) id_column("") value_column("dist_") diff --git a/bpkg/pkg-build-collect.cxx b/bpkg/pkg-build-collect.cxx index 11d1848..8442b15 100644 --- a/bpkg/pkg-build-collect.cxx +++ b/bpkg/pkg-build-collect.cxx @@ -30,7 +30,7 @@ namespace bpkg // build_package // const system_package_status* build_package:: - system_install () const + system_status () const { assert (action); @@ -40,20 +40,24 @@ namespace bpkg assert (sys_rep); if (const system_package* sys_pkg = sys_rep->find (name ())) - { - const system_package_status* s (sys_pkg->system_status); - - return s != nullptr && - (s->status == system_package_status::partially_installed || - s->status == system_package_status::not_installed) - ? s - : nullptr; - } + return sys_pkg->system_status; } return nullptr; } + const system_package_status* build_package:: + system_install () const + { + if (const system_package_status* s = system_status ()) + return s->status == system_package_status::partially_installed || + s->status == system_package_status::not_installed + ? s + : nullptr; + + return nullptr; + } + bool build_package:: user_selection () const { diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index f1cd77a..1af6c7a 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -4383,10 +4383,11 @@ namespace bpkg bool update_dependents (false); // We need the plan and to ask for the user's confirmation only if some - // implicit action (such as building prerequisite or reconfiguring - // dependent package) is to be taken or there is a selected package which - // version must be changed. But if the user explicitly requested it with - // --plan, then we print it as long as it is not empty. + // implicit action (such as building prerequisite, reconfiguring dependent + // package, or installing system/distribution packages) is to be taken or + // there is a selected package which version must be changed. But if the + // user explicitly requested it with --plan, then we print it as long as + // it is not empty. // string plan; sha256 csum; @@ -4401,18 +4402,25 @@ namespace bpkg // to the system packages which caused their installation (see // build_package::system_install() for details). // - // @@ Use main package for key. - // @@ Switch set to vector. - // @@ Terminology binary distribution package -> system/dis... - // - using system_map = map>; + using package_names = vector>; + using system_map = map; system_map sys_map; - for (const build_package& p: pkgs) + // Iterate in the reverse order as we will do for printing the action + // lines. This way a sys-install action line will be printed right + // before the bpkg action line of a package which appears first in the + // sys-install action's 'required by' list. + // + for (const build_package& p: reverse_iterate (pkgs)) { if (const system_package_status* s = p.system_install ()) - sys_installs[s].insert (p.name ()); + { + package_names& ps (sys_map[s->system_name]); + + if (find (ps.begin (), ps.end (), p.name ()) == ps.end ()) + ps.push_back (p.name ()); + } } // Start the transaction since we may query available packages for @@ -4441,10 +4449,10 @@ namespace bpkg string act; const system_package_status* s; - sys_installs_map::iterator j; + system_map::iterator j; if ((s = p.system_install ()) != nullptr && - (j = sys_installs.find (s)) != sys_installs.end ()) + (j = sys_map.find (s->system_name)) != sys_map.end ()) { act = "system-install "; act += s->system_name; @@ -4468,9 +4476,9 @@ namespace bpkg need_prompt = true; - // Make sure that we print this system_install action just once. + // Make sure that we print this system-install action just once. // - sys_installs.erase (j); + sys_map.erase (j); } else { @@ -4727,13 +4735,14 @@ namespace bpkg // Ok, we have "all systems go". The overall action plan is as follows. // - // 1. disfigure up/down-graded, reconfigured [left to right] - // 2. purge up/down-graded [right to left] - // 3.a fetch/unpack new, up/down-graded - // 3.b checkout new, up/down-graded - // 4. configure all - // 5. unhold unheld - // 6. build user selection [right to left] + // 1. system-install not installed system/distribution + // 2. disfigure up/down-graded, reconfigured [left to right] + // 3. purge up/down-graded [right to left] + // 4.a fetch/unpack new, up/down-graded + // 4.b checkout new, up/down-graded + // 5. configure all + // 6. unhold unheld + // 7. build user selection [right to left] // // Note that for some actions, e.g., purge or fetch, the order is not // really important. We will, however, do it right to left since that @@ -4841,37 +4850,42 @@ namespace bpkg size_t prog_i, prog_n, prog_percent; - // system_install + // system-install // - // Install the binary distribution packages required by the respective + // Install the system/distribution packages required by the respective // system packages (see build_package::system_install() for details). // if (!simulate) { - // Collect the names of the system packages which require installation - // of the binary distribution packages, suppressing duplicates. + // Collect the names of all the system packages being managed by the + // system package manager (as opposed to user/fallback), suppressing + // duplicates. Check if any of the system/distribution packages need to + // be installed. // - vector install_pkgs; + vector ips; + bool install (false); for (build_package& p: build_pkgs) { - if (p.system_install ()) + if (p.system_status () && + find (ips.begin (), ips.end (), p.name ()) == ips.end ()) { - if (find (install_pkgs.begin (), install_pkgs.end (), p.name ()) != - install_pkgs.end ()) - install_pkgs.push_back (p.name ()); + ips.push_back (p.name ()); + + if (!install && p.system_install ()) + install = true; } } - // Install the binary distribution packages, if any. + // Install the system/distribution packages. // - if (!install_pkgs.empty ()) + if (install) { // Otherwise, we wouldn't get any package statuses. // assert (sys_pkg_mgr && *sys_pkg_mgr != nullptr); - (*sys_pkg_mgr)->pkg_install (install_pkgs); + (*sys_pkg_mgr)->pkg_install (ips); } } -- cgit v1.1