From 0c5181917a71499e69b6a26fb882af4ac1b5ea91 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Thu, 19 Jan 2023 08:27:57 +0200 Subject: Switch back to single available system package version --- bpkg/system-package-manager-debian.cxx | 66 +++++------------- bpkg/system-package-manager-debian.hxx | 13 ++-- bpkg/system-package-manager.hxx | 122 +++++++++++++++++++++++---------- 3 files changed, 113 insertions(+), 88 deletions(-) diff --git a/bpkg/system-package-manager-debian.cxx b/bpkg/system-package-manager-debian.cxx index f30cfd6..cf15a46 100644 --- a/bpkg/system-package-manager-debian.cxx +++ b/bpkg/system-package-manager-debian.cxx @@ -203,6 +203,8 @@ namespace bpkg { unique_ptr g (parse_group (gs[i])); + // @@ Shouldn't we filter some based on what we are installing? + if (!g->main.empty ()) r->extras.push_back (move (g->main)); if (!g->dev.empty ()) r->extras.push_back (move (g->dev)); if (!g->doc.empty ()) r->extras.push_back (move (g->doc)); @@ -218,7 +220,7 @@ namespace bpkg return r; } - const vector>* + optional system_package_manager_debian:: pkg_status (const package_name& pn, const available_packages* aps, @@ -231,13 +233,13 @@ namespace bpkg auto i (status_cache_.find (pn)); if (i != status_cache_.end ()) - return &i->second; + return i->second.get (); if (aps == nullptr) return nullptr; } - vector> r; + vector> rs; // Translate our package name to the Debian package names. // @@ -276,7 +278,7 @@ namespace bpkg else s.reset (new package_status_debian (n)); - r.push_back (move (s)); + rs.push_back (move (s)); } else { @@ -289,16 +291,15 @@ namespace bpkg // Suppress duplicates for good measure based on the main package // name (and falling back to -dev if empty). // - auto i (find_if (r.begin (), r.end (), - [&s] (const unique_ptr& x) + auto i (find_if (rs.begin (), rs.end (), + [&s] (const unique_ptr& x) { - const package_status_debian& d (as_debian (x)); return s->main.empty () - ? s->dev == d.dev - : s->main == d.main; + ? s->dev == x->dev + : s->main == x->main; })); - if (i == r.end ()) - r.push_back (move (s)); + if (i == rs.end ()) + rs.push_back (move (s)); else { // @@ Should we verify the rest matches for good measure? @@ -309,42 +310,12 @@ namespace bpkg // First look for an already installed package. // + unique_ptr r; // Next look for available versions if we are allowed to install. // - // We only do this if we don't have a package already installed. This is - // because while Debian generally supports installing multiple versions in - // parallel, this is unlikely to be supported for the packages we are - // interested in due to the underlying limitations. - // - // Specifically, the packages that we are primarily interested in are - // libraries with headers and executables (tools). While Debian is able to - // install multiple libraries in parallel, it normally can only install a - // single set of headers, static libraries, pkg-config files, etc., (i.e., - // the -dev 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). - // - // It is possible that a certain library has made arrangements for - // multiple of its versions to co-exist. For example, hypothetically, our - // libssl package could be mapped to both libssl1.1 libssl1.1-dev and - // libssl3 libssl3-dev which could be installed at the same time (note - // that it is not the case in reality; there is only libssl-dev). However, - // in this case, we should probably also have two packages with separate - // names (e.g., libssl and libssl3) that can also co-exist. An example of - // this would be libQt5Core and libQt6Core. (Note that strictly speaking - // there could be different degrees of co-existence: for the system - // package manager it is sufficient for different versions not to clobber - // each other's files while for us we may also need the ability to use - // different versions in the base build). - // - // Note also that the above reasoning is quite C/C++-centric and it's - // possible that multiple versions of libraries (or equivalent) for other - // languages (e.g., Rust) can always co-exist. In this case we may need to - // revise this decision. - // - if (install && r.empty ()) + if (r == nullptr && install) { if (fetch && !fetched_) { @@ -356,12 +327,13 @@ namespace bpkg // Cache. // - return &status_cache_.emplace (pn, move (r)).first->second; + return status_cache_.emplace (pn, move (r)).first->second.get (); } - bool system_package_manager_debian:: - pkg_install (const package_name&, const version&) + void system_package_manager_debian:: + pkg_install (const vector&) { - return false; + assert (!installed_); + installed_ = true; } } diff --git a/bpkg/system-package-manager-debian.hxx b/bpkg/system-package-manager-debian.hxx index a2f56a2..74d9867 100644 --- a/bpkg/system-package-manager-debian.hxx +++ b/bpkg/system-package-manager-debian.hxx @@ -4,6 +4,8 @@ #ifndef BPKG_SYSTEM_PACKAGE_MANAGER_DEBIAN_HXX #define BPKG_SYSTEM_PACKAGE_MANAGER_DEBIAN_HXX +#include + #include #include @@ -17,14 +19,14 @@ namespace bpkg class system_package_manager_debian: public system_package_manager { public: - virtual const vector>* + virtual optional pkg_status (const package_name&, const available_packages*, bool install, bool fetch) override; - virtual bool - pkg_install (const package_name&, const version&) override; + virtual void + pkg_install (const vector&) override; public: // Note: expects os_release::name_id to be "debian" or os_release::like_id @@ -35,7 +37,10 @@ namespace bpkg : system_package_manager (move (osr)) {} protected: - bool fetched_ = false; // True if already fetched metadata. + bool fetched_ = false; // True if already fetched metadata. + bool installed_ = false; // True if already installed. + + std::map> status_cache_; }; } diff --git a/bpkg/system-package-manager.hxx b/bpkg/system-package-manager.hxx index 7d960f4..2d782b3 100644 --- a/bpkg/system-package-manager.hxx +++ b/bpkg/system-package-manager.hxx @@ -4,8 +4,6 @@ #ifndef BPKG_SYSTEM_PACKAGE_MANAGER_HXX #define BPKG_SYSTEM_PACKAGE_MANAGER_HXX -#include - #include // version #include @@ -20,6 +18,65 @@ namespace bpkg // The system package manager interface. Used by both pkg-build (to query // and install system packages) and by pkg-bindist (to build them). // + // Note that currently the result of a query is a single available version. + // While some package managers may support having multiple available + // versions and may even allow installing multiple versions in parallel, + // supporting this on our side will complicate things quite a bit. While we + // can probably plug multiple available versions into our constraint + // satisfaction machinery, the rabbit hole goes deeper than that since, for + // example, different bpkg packages can be mapped to the same system + // package, as is the case for libcrypto/libssl which are both mapped to + // libssl on Debian. This means we will need to somehow coordinate (and + // likely backtrack) version selection between unrelated bpkg packages + // because only one underlying system version can be selected. (One + // simplified way to handle this would be to detect that different versions + // we selected and fail asking the user to resolve this manually.) + // + // Additionally, parallel installation is unlikely to be suppored for the + // packages we are interested in due to the underlying limitations. + // Specifically, the packages that we are primarily interested in are + // libraries with headers and executables (tools). While most package + // 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). + // + // It is possible that a certain library has made arrangements for + // multiple of its versions to co-exist. For example, hypothetically, our + // libssl package could be mapped to both libssl1.1 libssl1.1-dev and + // libssl3 libssl3-dev which could be installed at the same time (note + // that it is not the case in reality; there is only libssl-dev). However, + // in this case, we should probably also have two packages with separate + // names (e.g., libssl and libssl3) that can also co-exist. An example of + // this would be libQt5Core and libQt6Core. (Note that strictly speaking + // there could be different degrees of co-existence: for the system + // package manager it is sufficient for different versions not to clobber + // each other's files while for us we may also need the ability to use + // different versions in the base build). + // + // Note also that the above reasoning is quite C/C++-centric and it's + // possible that multiple versions of libraries (or equivalent) for other + // languages (e.g., Rust) can always co-exist. Plus, even in the case of + // C/C++ libraries, there is still the plausible case of picking one of + // the multiple available version. + // + // On the other hand, the ultimate goal of system package managers, at least + // traditional ones like Debian and Fedora, is to end up with a single, + // usually the latest available, version of the package that is used by + // everyone. In fact, if one looks at a stable distributions of Debian and + // Fedora, they normally provide only a single version of each package. This + // decision will also likely simplify the implementation. For example, on + // Debian, it's straightforward to get the installed and candidate versions + // (e.g., from apt-cache policy). But getting all the possible versions that + // can be installed without having to specify the release explicitly is a + // lot less straightforward (see the apt-cache command documentation in The + // Debian Administrator's Handbook for background). + // + // So for now we keep it simple and pick a single available version but can + // probably revise this decision later. + // class system_package_status { public: @@ -27,27 +84,18 @@ namespace bpkg // bpkg::version version; + // System (as in, distribution package) version. + // + string system_version; + // The system package can be either "available already installed", // "available partially installed" (for example, libfoo but not // libfoo-dev is installed) or "available not yet installed". // - // Whether not_installed versions can be returned along with installed - // or partially_installed depends on whether the packager manager can - // install multiple versions side-by-side. - // enum status_type {installed, partially_installed, not_installed}; status_type status = not_installed; - // System (as in, distribution package) name and version. - // - // @@ But these could be multiple. Do we really need this? - // @@ Can now probably provide as virtual functions. - /* - string system_name; - string system_version; - */ - public: virtual ~system_package_status (); @@ -63,41 +111,43 @@ namespace bpkg // This function has two modes: cache-only (available_packages is NULL) // and full (available_packages is not NULL). In the cache-only mode this // function returns the status of this package if it has already been - // queried and NULL otherwise. This allows the caller to only collect all - // the available packages (for the name/version mapping information) if - // really necessary. + // queried and nullopt otherwise. This allows the caller to only collect + // all the available packages (for the name/version mapping information) + // if really necessary. // - // The returned list should be arranged in the preference order with the - // first entry having the highest preference. Normally this will be in the - // descending version order but can also be something more elaborate, such - // as the already installed or partially installed version coming first - // with the descending version order after that. - // - // The returned list can be empty, which indicates that no such package + // The returned status can be NULL, which indicates that no such package // is available from the system package manager. Note that empty is also // returned if no fully installed package is available from the system and // the install argument is false. // // If fetch is false, then do not re-fetch the system package repository - // metadata (that is, available packages/versions) before querying for - // the available version of the not yet installed or partially installed + // metadata (that is, available packages/versions) before querying for the + // available version of the not yet installed or partially installed // packages. // - virtual const vector>* + virtual optional pkg_status (const package_name&, const available_packages*, bool install, bool fetch) = 0; - // Install the previously-queried package that is not installed or - // partially installed. + // Install the specified subset of the previously-queried packages. + // + // Note that this function should be called only once after the final set + // of the necessary 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 + // packages may be mapped to the same system package), perform post- + // installation verifications (such as making sure the versions of already + // installed packages have not changed due to upgrades), implement version + // holds, etc. // - // Return false if the installation was aborted by the user (for example, - // the user answered 'N' to the prompt). @@ Do we really need this? We - // may not always be able to distinguish. + // Note also that the implementation is expected to issue appropriate + // progress and diagnostics. // - virtual bool - pkg_install (const package_name&, const version&) = 0; + virtual void + pkg_install (const vector&) = 0; public: virtual @@ -162,8 +212,6 @@ namespace bpkg const vector& like_ids); protected: os_release os_release_; - std::map>> status_cache_; }; // Create a package manager instance corresponding to the specified host -- cgit v1.1