WIP

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<recursive_mode>)
   {
-    // @@ 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<selected_package>& 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<available_package>&        ap (aps.front ().first);
+      const lazy_shared_ptr<repository_fragment>& rf (aps.front ().second);
+
+
+      if (optional<string> 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<selected_package>& 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<shared_ptr<selected_package>, available_packages>& p: deps)
     {
-      const selected_package& sp (*p.first);
+      const shared_ptr<selected_package>& 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<package_status> os (status (sp.name, aps));
+        optional<package_status> 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<string, semantic_version>
   parse_distribution (string&& d,
                       const string& value_name,
-                      const available_package& ap,
+                      const shared_ptr<available_package>& ap,
                       const lazy_shared_ptr<repository_fragment>& af)
   {
     string dn (move (d));      // <name>[_<version>]
@@ -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<string> d = dv.distribution ("-name"))
           {
             pair<string, semantic_version> 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<string> system_package_manager::
-  system_package_version (const available_package& ap,
+  system_package_version (const shared_ptr<available_package>& ap,
                           const lazy_shared_ptr<repository_fragment>& 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<string> r;
       semantic_version rv;
 
-      for (const distribution_name_value& dv: ap.distribution_values)
+      for (const distribution_name_value& dv: ap->distribution_values)
       {
         if (optional<string> 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<string> d = nv.distribution ("-to-downstream-version"))
           {
             pair<string, semantic_version> 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
     // <distribution>-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<repository_fragment> is used only for
     // diagnostics and conveys the database the available package object
     // belongs to.
     //
     static optional<string>
-    system_package_version (const available_package&,
+    system_package_version (const shared_ptr<available_package>&,
                             const lazy_shared_ptr<repository_fragment>&,
                             const string& name_id,
                             const string& version_id,