From f17e68be1fa405c38411f13f4429c62432c612d5 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Tue, 31 Jan 2023 10:06:09 +0200 Subject: Various tweaks and fixes --- bpkg/pkg-build.cli | 26 ++++----- bpkg/pkg-build.cxx | 4 +- bpkg/system-package-manager-debian.cxx | 45 +++++++++------ bpkg/system-package-manager-debian.hxx | 14 ++--- bpkg/system-package-manager-debian.test.cxx | 8 ++- bpkg/system-package-manager-debian.test.testscript | 66 ++++++++++++++++++++-- bpkg/system-package-manager.cxx | 2 +- bpkg/system-package-manager.hxx | 41 +++++++------- 8 files changed, 137 insertions(+), 69 deletions(-) diff --git a/bpkg/pkg-build.cli b/bpkg/pkg-build.cli index d2844c2..740764b 100644 --- a/bpkg/pkg-build.cli +++ b/bpkg/pkg-build.cli @@ -103,11 +103,11 @@ namespace bpkg unspecified, then \cb{pkg-build} will attempt to query the system package manager for the installed version unless the system package manager is unsupported or this functionality is disabled with \cb{--sys-no-query}, - in which case '\cb{/*}' is assumed. If the system package + in which case the '\cb{/*}' is assumed. If the system package manager is supported, then the automatic installation of an available - package can be requested with the \cb{--sys-install} option. If the - version is not explicitly specified, then at least a stub package must be - available from one of the repositories. + package can be requested with the \cb{--sys-install} option. Note that if + the version is not explicitly specified, then at least a stub package + must be available from one of the repositories. Finally, a package can be specified as either the path to the package archive () or to the package directory (\cb{/}; note that it @@ -424,24 +424,24 @@ namespace bpkg bool --sys-install { - "Automatically instruct the system package manager to install - available versions of packages specified with the \cb{sys} scheme - that are not already installed. See also the \cb{--sys-no-fetch}, - \cb{--sys-yes}, and \cb{--sys-sudo} options." + "Instruct the system package manager to install available versions of + packages specified with the \cb{sys} scheme that are not already + installed. See also the \cb{--sys-no-fetch}, \cb{--sys-yes}, and + \cb{--sys-sudo} options." } bool --sys-no-fetch { - "Do not automatically fetch the system package manager metadata - before querying for available version. This option only makes - sense together with \cb{--sys-install}." + "Do not fetch the system package manager metadata before querying for + available versions of packages specified with the \cb{sys} scheme. + This option only makes sense together with \cb{--sys-install}." } bool --sys-yes { "Assume the answer to the system package manager prompts is \cb{yes}. Note that system package manager interactions may break your system - and you should normally only use this option on throw-away systems + and you should normally only use this option on throw-away setups (test virtual machines, etc)." } @@ -455,7 +455,7 @@ namespace bpkg by default. Pass empty or the special \cb{false} value to disable the use of the \cb{sudo} program. Note that the \cb{sudo} program is normally only needed if the system package installation is enabled - (\cb{--sys-install})." + with the \cb{--sys-install} option." } dir_paths --directory|-d diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index 07e23be..4d90df2 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -44,7 +44,7 @@ using namespace butl; namespace bpkg { // System package manager. Resolved lazily if and when needed. Present NULL - // value means no system package manager is available. + // value means no system package manager is available for this host. // static optional> sys_pkg_mgr; @@ -1731,7 +1731,7 @@ namespace bpkg << "installed with system package manager"; dr << info << "specify --sys-no-query to disable system " - << "package manager interaction"; + << "package manager interactions"; }); if (!os) diff --git a/bpkg/system-package-manager-debian.cxx b/bpkg/system-package-manager-debian.cxx index 4b4dfa9..06b5060 100644 --- a/bpkg/system-package-manager-debian.cxx +++ b/bpkg/system-package-manager-debian.cxx @@ -19,7 +19,8 @@ namespace bpkg // extra_{doc,dbg} arguments. // package_status system_package_manager_debian:: - parse_name_value (const string& nv, + parse_name_value (const package_name& pn, + const string& nv, bool extra_doc, bool extra_dbg) { @@ -38,7 +39,8 @@ namespace bpkg return nn > sn && n.compare (nn - sn, sn, s) == 0; }; - auto parse_group = [&split, &suffix] (const string& g) + auto parse_group = [&split, &suffix] (const string& g, + const package_name* pn) { strings ns (split (g, ' ')); @@ -47,17 +49,20 @@ namespace bpkg package_status r; - // Handle the dev instead of main special case for libraries. + // Handle the "dev instead of main" special case for libraries. + // + // Note: the lib prefix check is based on the bpkg package name. // // Check that the following name does not end with -dev. This will be // the only way to disambiguate the case where the library name happens - // to end with -dev (e.g., libops-dev libops-dev-dev). + // to end with -dev (e.g., libfoo-dev libfoo-dev-dev). // { string& m (ns[0]); - if (m.compare (0, 3, "lib") == 0 && - suffix (m, "-dev") && + if (pn != nullptr && + pn->string ().compare (0, 3, "lib") == 0 && + suffix (m, "-dev") && !(ns.size () > 1 && suffix (ns[1], "-dev"))) { r = package_status ("", move (m)); @@ -98,10 +103,10 @@ namespace bpkg for (size_t i (0); i != gs.size (); ++i) { if (i == 0) // Main group. - r = parse_group (gs[i]); + r = parse_group (gs[i], &pn); else { - package_status g (parse_group (gs[i])); + package_status g (parse_group (gs[i], nullptr)); if (!g.main.empty ()) r.extras.push_back (move (g.main)); if (!g.dev.empty ()) r.extras.push_back (move (g.dev)); @@ -202,7 +207,7 @@ namespace bpkg assert (n != 0 && n <= pps.size ()); - // In particular, --quiet makes sure we don't get a noice (N) printed to + // The --quiet option makes sure we don't get a noice (N) printed to // stderr if the package is unknown. It does not appear to affect error // diagnostics (try temporarily renaming /var/lib/dpkg/status). // @@ -666,7 +671,7 @@ namespace bpkg // // 1 -- shows URL being downloaded but no percentage progress is shown. // - // 2 -- only shows diagnostics (implies --assume-yes and which cannot be + // 2 -- only shows diagnostics (implies --assume-yes which cannot be // overriden with --assume-no). // // It also appears to automatically use level 1 if stderr is not a @@ -856,7 +861,8 @@ namespace bpkg { // For now we ignore -doc and -dbg package components (but we may want to // have options controlling this later). Note also that we assume -common - // is pulled automatically by the main package so we ignore it as well. + // is pulled automatically by the main package so we ignore it as well + // (see equivalent logic in parse_name_value()). // bool need_doc (false); bool need_dbg (false); @@ -917,7 +923,7 @@ namespace bpkg // for (const string& n: ns) { - package_status s (parse_name_value (n, need_doc, need_dbg)); + package_status s (parse_name_value (pn, n, need_doc, need_dbg)); // Suppress duplicates for good measure based on the main package // name (and falling back to -dev if empty). @@ -925,7 +931,11 @@ namespace bpkg auto i (find_if (candidates.begin (), candidates.end (), [&s] (const package_status& x) { - return s.main.empty () + // Note that it's possible for one mapping to be + // specified as -dev only while the other as main + // and -dev. + // + return s.main.empty () || x.main.empty () ? s.dev == x.dev : s.main == x.main; })); @@ -940,13 +950,14 @@ namespace bpkg // debian_9-name: libcurl4 libcurl4-dev // // Note that for this to work we must get debian_10 values before - // debian_9, which is the semantics of parse_name_value(). + // debian_9, which is the semantics guaranteed by + // system_package_names(). } } } } - // Guess unknown main package given the dev package and its version. + // Guess unknown main package given the -dev package and its version. // auto guess_main = [this, &pn] (package_status& s, const string& ver) { @@ -1028,8 +1039,8 @@ namespace bpkg { const package_policy& dev (pps.front ()); - // Note that at this stage we can only use the installed dev package - // (since the candidate version may change after fetch). + // 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; diff --git a/bpkg/system-package-manager-debian.hxx b/bpkg/system-package-manager-debian.hxx index b945a54..8783b51 100644 --- a/bpkg/system-package-manager-debian.hxx +++ b/bpkg/system-package-manager-debian.hxx @@ -29,8 +29,8 @@ namespace bpkg // libssl1.1 libssl-dev libssl-doc // libssl3 libssl-dev libssl-doc // - // libcurl4 libcurl4-doc libcurl4-openssl-dev - // libcurl3-gnutls libcurl4-gnutls-dev (yes, 3 and 4) + // libcurl4 libcurl4-openssl-dev libcurl4-doc + // libcurl3-gnutls libcurl4-gnutls-dev libcurl4-doc (yes, 3 and 4) // // Based on that, it seems our best bet when trying to automatically map our // library package name to Debian package names is to go for the -dev @@ -42,7 +42,7 @@ namespace bpkg // For executable packages there is normally no -dev packages but -dbg, // -doc, and -common are plausible. // - // The format of the debian-name (or alike) manifest value value is a comma- + // The format of the debian-name (or alike) manifest value is a comma- // separated list of one or more package groups: // // [, ...] @@ -77,7 +77,7 @@ namespace bpkg // is specified with the debian-to-downstream-version manifest values (or // alike), then we fallback to using the part as bpkg version. If // explicit mapping is specified, then we match it against the - // : parts ignoring . + // [:] parts ignoring . // struct system_package_status_debian: system_package_status { @@ -123,8 +123,8 @@ namespace bpkg pkg_install (const vector&) override; public: - // Note: expects os_release::name_id to be "debian" or os_release::like_id - // to contain "debian". + // Expects os_release::name_id to be "debian" or os_release::like_ids to + // contain "debian". // using system_package_manager::system_package_manager; @@ -151,7 +151,7 @@ namespace bpkg apt_get_common (const char*); static package_status - parse_name_value (const string&, bool, bool); + parse_name_value (const package_name&, const string&, bool, bool); static string main_from_dev (const string&, const string&, const string&); diff --git a/bpkg/system-package-manager-debian.test.cxx b/bpkg/system-package-manager-debian.test.cxx index 562344c..a033400 100644 --- a/bpkg/system-package-manager-debian.test.cxx +++ b/bpkg/system-package-manager-debian.test.cxx @@ -32,7 +32,7 @@ namespace bpkg // // apt-cache-show result comes from stdin // - // parse_name_value debian-name value from from stdin + // parse-name-value debian-name value from stdin // // main-from-dev depends comes from stdin // @@ -128,13 +128,15 @@ namespace bpkg } else if (cmd == "parse-name-value") { - assert (argc == 2); + assert (argc == 3); // + + package_name pn (argv[2]); string v; getline (cin, v); package_status s ( - system_package_manager_debian::parse_name_value (v, false, false)); + system_package_manager_debian::parse_name_value (pn, v, false, false)); if (!s.main.empty ()) cout << "main: " << s.main << '\n'; if (!s.dev.empty ()) cout << "dev: " << s.dev << '\n'; diff --git a/bpkg/system-package-manager-debian.test.testscript b/bpkg/system-package-manager-debian.test.testscript index 6d3094c..b1a0030 100644 --- a/bpkg/system-package-manager-debian.test.testscript +++ b/bpkg/system-package-manager-debian.test.testscript @@ -145,7 +145,7 @@ : basics : - $* <>EOO + $* libssl <>EOO libssl3 libssl-common libssl-doc libssl-dev libssl-dbg libssl-extras, libc6 libc-dev libc-common libc-doc, libz-dev EOI main: libssl3 @@ -158,7 +158,7 @@ : non-lib : - $* <>EOO + $* sqlite3 <>EOO sqlite3 sqlite3-common sqlite3-doc EOI main: sqlite3 @@ -168,7 +168,7 @@ : lib-dev : - $* <>EOO + $* libssl <>EOO libssl-dev EOI dev: libssl-dev @@ -176,7 +176,7 @@ : non-lib-dev : - $* <>EOO + $* ssl-dev <>EOO ssl-dev EOI main: ssl-dev @@ -184,7 +184,7 @@ : lib-custom-dev : - $* <>EOO + $* libfoo-dev <>EOO libfoo-dev libfoo-dev-dev EOI main: libfoo-dev @@ -372,6 +372,62 @@ EOO + # Note that the semantics is unrealistic (maybe background apt-get update + # happenned in between). + # + : part-installed-upgrade-version-change + : + cat <=libsqlite3-dev.policy; + libsqlite3-dev: + Installed: (none) + Candidate: 3.39.4-1 + Version table: + 3.39.4-1 500 + 500 http://deb.debian.org/debian bookworm/main amd64 Packages + EOI + cat <=libsqlite3-dev.show; + Package: libsqlite3-dev + Version: 3.39.4-1 + Depends: libsqlite3-0 (= 3.39.4-1), libc-dev + EOI + cat <=libsqlite3-0.policy; + libsqlite3-0: + Installed: 3.39.4-1 + Candidate: 3.39.4-1 + Version table: + *** 3.39.4-1 500 + 500 http://deb.debian.org/debian bookworm/main amd64 Packages + 100 /var/lib/dpkg/status + EOI + cat <=libsqlite3-0.policy-installed; + libsqlite3-0: + Installed: 3.40.1-1 + Candidate: 3.40.1-1 + Version table: + *** 3.40.1-1 500 + 500 http://deb.debian.org/debian bookworm/main amd64 Packages + 100 /var/lib/dpkg/status + EOI + $* libsqlite3 --install --no-fetch libsqlite3 <>EOE >>EOO != 0 + apt-cache-policy: libsqlite3-dev libsqlite3-dev.policy + apt-cache-show: libsqlite3-dev 3.39.4-1 libsqlite3-dev.show + apt-cache-policy: libsqlite3-0 libsqlite3-0.policy + apt-cache-policy-installed: libsqlite3-0 libsqlite3-0.policy-installed + EOI + LC_ALL=C apt-cache policy --quiet libsqlite3-dev =libsqlite3-dev.policy; diff --git a/bpkg/system-package-manager.cxx b/bpkg/system-package-manager.cxx index 77265b3..9f5dad0 100644 --- a/bpkg/system-package-manager.cxx +++ b/bpkg/system-package-manager.cxx @@ -103,7 +103,7 @@ namespace bpkg catch (const invalid_argument& e) { fail << "invalid version '" << version_id << "' for " << name_id - << " operating system: " << e << endf; + << " host: " << e << endf; } } diff --git a/bpkg/system-package-manager.hxx b/bpkg/system-package-manager.hxx index a6b8299..9a9c443 100644 --- a/bpkg/system-package-manager.hxx +++ b/bpkg/system-package-manager.hxx @@ -41,9 +41,9 @@ namespace bpkg // managers (e.g., Debian, Fedora) are able to install multiple libraries in // parallel, they normally can only install a single set of headers, static // libraries, pkg-config files, etc., (e.g., -dev/-devel package) at a time - // due to them being installed into the same location (e.g., - // /usr/include). The same holds for executables, which are installed into - // the same location (e.g., /usr/bin). + // due to them being installed into the same location (e.g., /usr/include). + // The same holds for executables, which are installed into the same + // location (e.g., /usr/bin). // // It is possible that a certain library has made arrangements for // multiple of its versions to co-exist. For example, hypothetically, our @@ -85,12 +85,11 @@ namespace bpkg // bpkg::version version; - // System (as in, distribution package) package name and version for - // diagnostics. + // System (as in, distribution) package name and version for diagnostics. // // Note that this status may represent multiple system packages (for - // example, libssl3 and libssl3-dev) and here we have the main package - // name (for example, libssl3). + // example, libfoo and libfoo-dev) and here we have only the + // main/representative package name (for example, libfoo). // string system_name; string system_version; @@ -122,7 +121,8 @@ namespace bpkg // package installation is not enabled (see the constructor below). // // Note also that the implementation is expected to issue appropriate - // progress and diagnostics if fetching package metadata. + // progress and diagnostics if fetching package metadata (again see the + // constructor below). // virtual optional pkg_status (const package_name&, const available_packages*) = 0; @@ -132,7 +132,7 @@ namespace bpkg // below). // // Note that this function should be called only once after the final set - // of the necessary system packages has been determined. And the specified + // of the required system packages has been determined. And the specified // subset should contain all the selected packages, including the already // fully installed. This allows the implementation to merge and de- // duplicate the system package set to be installed (since some bpkg @@ -149,9 +149,6 @@ namespace bpkg pkg_install (const vector&) = 0; public: - virtual - ~system_package_manager (); - // If install is true, then enable package installation. // // If fetch is false, then do not re-fetch the system package repository @@ -174,6 +171,9 @@ namespace bpkg yes_ (yes), sudo_ (sudo != "false" ? move (sudo) : string ()) {} + virtual + ~system_package_manager (); + // Implementation details. // public: @@ -188,17 +188,17 @@ namespace bpkg // First consider -name values corresponding to name_id. // Assume has the [_] form, where // is a semver-like version (e.g, 10, 10.15, or 10.15.1) and return all - // the values that are equal of less than the specified version_id + // the values that are equal or less than the specified version_id // (include the value with the absent ). In a sense, absent - // can be treated as 0 semver-like versions. + // can be treated as a 0 semver-like version. // // If no value is found then repeat the above process for every like_ids // entry (from left to right) instead of name_id with version_id equal 0. // // If still no value is found, then return empty list (in which case the // caller may choose to fallback to the downstream package name or do - // something more elaborate, like translate version_id to the like_id's - // version and try that). + // something more elaborate, like translate version_id to one of the + // like_id's version and try that). // // Note that multiple -name values per same distribution can be returned // as, for example, for the following distribution values: @@ -206,9 +206,9 @@ namespace bpkg // debian_10-name: libcurl4 libcurl4-doc libcurl4-openssl-dev // debian_10-name: libcurl3-gnutls libcurl4-gnutls-dev (yes, 3 and 4) // - // Note also that the value are returned in the "override order", that is + // Note also that the values are returned in the "override order", that is // from the newest package version to oldest and then from the highest - // distribution version to oldest. + // distribution version to lowest. // static strings system_package_names (const available_packages&, @@ -226,8 +226,7 @@ namespace bpkg // name_id. If none match, then repeat the above process for every // like_ids entry with version_id equal 0. If still no match, then return // nullopt (in which case the caller may choose to fallback to the system - // package version or do something more elaborate, like translate - // version_id to the like_id's version and try that). + // package version or do something more elaborate). // static optional downstream_package_version (const string& system_version, @@ -255,7 +254,7 @@ namespace bpkg // debian -- Debian and alike (Ubuntu, etc) using the APT frontend. // fedora -- Fedora and alike (RHEL, Centos, etc) using the DNF frontend. // - // Note: the name will be used to select an alternative package manager + // Note: the name can be used to select an alternative package manager // implementation on platforms that support multiple. // unique_ptr -- cgit v1.1