From 8ea35ee2db5330955a048d11690850f97987f4f7 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Fri, 10 Feb 2023 08:13:52 +0200 Subject: WIP --- bpkg/system-package-manager-debian.cxx | 87 +++++++++++++++++++++++++++++----- bpkg/system-package-manager.cxx | 20 ++++---- bpkg/system-package-manager.hxx | 6 +-- 3 files changed, 88 insertions(+), 25 deletions(-) diff --git a/bpkg/system-package-manager-debian.cxx b/bpkg/system-package-manager-debian.cxx index d3e63b6..47df65d 100644 --- a/bpkg/system-package-manager-debian.cxx +++ b/bpkg/system-package-manager-debian.cxx @@ -1527,18 +1527,42 @@ namespace bpkg const dir_path&, optional) { - // @@ What are we doing with extras, in both deps and the package being - // generated? - // Map non-system bpkg package to system package name(s) and version. // - auto map_package = [this] (const selected_package& sp, + // This is used both to map the package being generated and its + // dependencies. What should we do with extras returned in package_status? + // We can't really generate any of them (which files would we place in + // them?) and we can't list them as dependencies (we don't know their + // system versions). So it feels like the only sensible choice is to + // ignore extras. + // + // In a sense, we have a parallel arrangement going on here: binary + // packages that we generate don't have extras (i.e., they include + // everything necessary in the "standard" packages from the main group) + // and when we punch a system dependency based on a non-system bpkg + // package, we assume it was generated by us and thus doesn't have any + // extras. Or, to put it another way, if you want the system dependency to + // refer to a "native" system package with extras you need to configure it + // as a system bpkg package. + // + // In fact, this extends to package names. For example, unless custom + // mapping is specified, we will generate libsqlite3 and libsqlite3-dev + // while native names are libsqlite3-0 and libsqlite3-dev. While this + // duality is not ideal, presumably we will normally only be producing + // our binary packages if there are no suitable native packages. And for a + // few exception (e.g., our package is "better" in some way, such as + // configured differently or fixes a critical bug), we will just have to + // provide appropriate manual mapping that makes sure the names match (the + // extras is still a potential problem though -- we will only have them as + // dependencies if we build against a native system package). + // + auto map_package = [this] (const shared_ptr& sp, const available_packages& aps) -> package_status { // We should only have one available package corresponding to the // selected package. // - assert (sp.substate != package_substate::system && aps.size () == 1); + assert (sp->substate != package_substate::system && aps.size () == 1); strings ns (system_package_names (aps, os_release.name_id, @@ -1551,7 +1575,7 @@ namespace bpkg // case above. Except here we don't attempt to deduce main from -dev, // naturally. // - const string& pn (sp.name.string ()); + const string& pn (sp->name.string ()); // The best we can do in trying to detect whether this is a library is // to check for the lib prefix. Libraries without the lib prefix and @@ -1568,15 +1592,54 @@ namespace bpkg else { // Even though we only pass one available package, we may still end up - // with multiple mappings. In this case we take the first per the + // with multiple mappings. In this case we take the first, per the // documentation. // - r = parse_name_value (sp.name, + r = parse_name_value (sp->name, ns.front (), false /* need_doc */, false /* need_dbg */); } + // Map the version. + // + // @@ Should we fail if either is a pre-release (we cannot represent + // it in Debian version). But then won't be able to test on + // pre-release packages. maybe just strip/ignore? Can we translate + // it to something appropriate in Debian version (something in the + // revision)? + // + // @@ Should we include our revision as Debian revistion even if the + // version is mapped? Feels natural. + // + const shared_ptr& ap (aps.front ().first); + const lazy_shared_ptr& rf (aps.front ().second); + + + if (optional v = system_package_version (ap, + rf, + os_release.name_id, + os_release.version_id, + os_release.like_ids)) + { + r.system_version = move (*v); + } + else if (ap->upstream_version) + { + r.system_version = *ap->upstream_version; + } + else + { + // Strip or keep epoch? On one hand it won't necessarily match Debian + // but on the other it will allow packages form different epochs to + // co-exist. + // + // @@ Also need to make sure have explicit epoch/revision if upstream + // contains `:` or `-`. Can our upstream contain them? + // + r.system_version = sp->version.upstream; + } + return r; }; @@ -1586,7 +1649,7 @@ namespace bpkg // Note that there should be no duplicate dependencies and we can sidestep // the status cache. // - const selected_package& sp (*pkgs.front ().first); + const shared_ptr& sp (pkgs.front ().first); const available_packages& aps (pkgs.front ().second); package_status s (map_package (sp, aps)); @@ -1594,13 +1657,13 @@ namespace bpkg sdeps.reserve (deps.size ()); for (const pair, available_packages>& p: deps) { - const selected_package& sp (*p.first); + const shared_ptr& sp (p.first); const available_packages& aps (p.second); package_status s; - if (sp.substate == package_substate::system) + if (sp->substate == package_substate::system) { - optional os (status (sp.name, aps)); + optional os (status (sp->name, aps)); if (!os) fail << "bad boy"; diff --git a/bpkg/system-package-manager.cxx b/bpkg/system-package-manager.cxx index 8e3bf3b..899d390 100644 --- a/bpkg/system-package-manager.cxx +++ b/bpkg/system-package-manager.cxx @@ -219,7 +219,7 @@ namespace bpkg static pair parse_distribution (string&& d, const string& value_name, - const available_package& ap, + const shared_ptr& ap, const lazy_shared_ptr& af) { string dn (move (d)); // [_] @@ -270,8 +270,8 @@ namespace bpkg diag_record dr (fail); dr << "invalid distribution version '" << string (dn, p + 1) - << "' in value " << value_name << " for package " << ap.id.name - << ' ' << ap.version; + << "' in value " << value_name << " for package " << ap->id.name + << ' ' << ap->version; if (db != nullptr) dr << *db; @@ -321,7 +321,7 @@ namespace bpkg if (optional d = dv.distribution ("-name")) { pair dnv ( - parse_distribution (move (*d), dv.name, *ap, a.second)); + parse_distribution (move (*d), dv.name, ap, a.second)); semantic_version& dvr (dnv.second); @@ -379,7 +379,7 @@ namespace bpkg } optional system_package_manager:: - system_package_version (const available_package& ap, + system_package_version (const shared_ptr& ap, const lazy_shared_ptr& af, const string& name_id, const string& version_id, @@ -395,8 +395,8 @@ namespace bpkg // the distribution version is equal to the specified one. Otherwise (the // version is less), continue iterating while preferring system version // candidates for greater distribution versions. Note that here we are - // trying to pick the system version which distribution version closest - // (but never greater) to the specified distribution version, similar to + // trying to pick the system version with distribution version closest to + // (but never greater than) the specified distribution version, similar to // what we do in downstream_package_version() (see its // downstream_version() lambda for details). // @@ -407,7 +407,7 @@ namespace bpkg optional r; semantic_version rv; - for (const distribution_name_value& dv: ap.distribution_values) + for (const distribution_name_value& dv: ap->distribution_values) { if (optional d = dv.distribution ("-version")) { @@ -484,7 +484,7 @@ namespace bpkg // specified one. Otherwise (the version is less), continue iterating // while preferring downstream version candidates for greater distribution // versions. Note that here we are trying to use a version mapping for the - // distribution version closest (but never greater) to the specified + // distribution version closest to (but never greater than) the specified // distribution version. So, for example, if both following values contain // a matching mapping, then for debian 11 we prefer the downstream version // produced by the debian_10-to-downstream-version value: @@ -508,7 +508,7 @@ namespace bpkg if (optional d = nv.distribution ("-to-downstream-version")) { pair dnv ( - parse_distribution (move (*d), nv.name, *ap, a.second)); + parse_distribution (move (*d), nv.name, ap, a.second)); semantic_version& dvr (dnv.second); diff --git a/bpkg/system-package-manager.hxx b/bpkg/system-package-manager.hxx index 7e4f89c..305a00e 100644 --- a/bpkg/system-package-manager.hxx +++ b/bpkg/system-package-manager.hxx @@ -280,15 +280,15 @@ namespace bpkg // -version values corresponding to 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 upstream package version or do - // something more elaborate). + // caller may choose to fallback to the upstream/bpkg package version or + // do something more elaborate). // // Note that lazy_shared_ptr is used only for // diagnostics and conveys the database the available package object // belongs to. // static optional - system_package_version (const available_package&, + system_package_version (const shared_ptr&, const lazy_shared_ptr&, const string& name_id, const string& version_id, -- cgit v1.1