Also consider subproject directory names when calculating manifest checksums

10 files changed, 224 insertions, 82 deletions

diff --git a/bpkg/manifest-utility.cxx b/bpkg/manifest-utility.cxx
index a4cee94..9b8cbca 100644
--- a/bpkg/manifest-utility.cxx
+++ b/bpkg/manifest-utility.cxx
@@ -22,6 +22,41 @@ namespace bpkg
   const path signature_file    ("signature.manifest");
   const path manifest_file     ("manifest");
 
+  vector<package_info>
+  package_b_info (const common_options& o, const dir_paths& ds, bool ext_mods)
+  {
+    path b (name_b (o));
+
+    vector<package_info> r;
+    try
+    {
+      b_info (r,
+              ds,
+              ext_mods,
+              verb,
+              [] (const char* const args[], size_t n)
+              {
+                if (verb >= 2)
+                  print_process (args, n);
+              },
+              b,
+              exec_dir,
+              o.build_option ());
+      return r;
+    }
+    catch (const b_error& e)
+    {
+      if (e.normal ())
+        throw failed (); // Assume the build2 process issued diagnostics.
+
+      diag_record dr (fail);
+      dr << "unable to parse project ";
+      if (r.size () < ds.size ()) dr << ds[r.size ()] << ' ';
+      dr << "info: " << e <<
+        info << "produced by '" << b << "'; use --build to override" << endf;
+    }
+  }
+
   package_scheme
   parse_package_scheme (const char*& s)
   {
@@ -272,52 +307,54 @@ namespace bpkg
     }
   }
 
-  vector<optional<version>>
+  package_version_infos
   package_versions (const common_options& o, const dir_paths& ds)
   {
-    path b (name_b (o));
+    vector<b_project_info> pis (package_b_info (o, ds, false /* ext_mods */));
+
+    package_version_infos r;
+    r.reserve (pis.size ());
+
+    for (const b_project_info& pi: pis)
+    {
+      // An empty version indicates that the version module is not enabled for
+      // the project.
+      //
+      optional<version> v (!pi.version.empty ()
+                           ? version (pi.version.string ())
+                           : optional<version> ());
+
+      r.push_back (package_version_info {move (v), move (pi)});
+    }
+
+    return r;
+  }
+
+  string
+  package_checksum (const common_options& o,
+                    const dir_path& d,
+                    const package_info* pi)
+  {
+    path f (d / manifest_file);
 
-    vector<b_project_info> pis;
     try
     {
-      b_info (pis,
-              ds,
-              false /* ext_mods */,
-              verb,
-              [] (const char* const args[], size_t n)
-              {
-                if (verb >= 2)
-                  print_process (args, n);
-              },
-              b,
-              exec_dir,
-              o.build_option ());
+      ifdstream is (f, fdopen_mode::binary);
+      sha256 cs (is);
 
-      vector<optional<version>> r;
-      r.reserve (pis.size ());
+      const vector<package_info::subproject>& sps (
+        pi != nullptr
+        ? pi->subprojects
+        : package_b_info (o, d, false /* ext_mods */).subprojects);
 
-      for (const b_project_info& pi: pis)
-      {
-        // An empty version indicates that the version module is not enabled
-        // for the project.
-        //
-        r.push_back (!pi.version.empty ()
-                     ? version (pi.version.string ())
-                     : optional<version> ());
-      }
+      for (const package_info::subproject& sp: sps)
+        cs.append (sp.path.string ());
 
-      return r;
+      return cs.string ();
     }
-    catch (const b_error& e)
+    catch (const io_error& e)
     {
-      if (e.normal ())
-        throw failed (); // Assume the build2 process issued diagnostics.
-
-      diag_record dr (fail);
-      dr << "unable to parse project ";
-      if (pis.size () < ds.size ()) dr << ds[pis.size ()] << ' ';
-      dr << "info: " << e <<
-        info << "produced by '" << b << "'; use --build to override" << endf;
+      fail << "unable to read from " << f << ": " << e << endf;
     }
   }
 }
diff --git a/bpkg/manifest-utility.hxx b/bpkg/manifest-utility.hxx
index b7d9b07..8ef517e 100644
--- a/bpkg/manifest-utility.hxx
+++ b/bpkg/manifest-utility.hxx
@@ -17,6 +17,20 @@ namespace bpkg
   extern const path signature_file;    // signature.manifest
   extern const path manifest_file;     // manifest
 
+  // Obtain build2 projects info for package source or output directories.
+  //
+  vector<package_info>
+  package_b_info (const common_options&, const dir_paths&, bool ext_mods);
+
+  // As above but return the info for a single package directory.
+  //
+  inline package_info
+  package_b_info (const common_options& o, const dir_path& d, bool ext_mods)
+  {
+    vector<package_info> r (package_b_info (o, dir_paths ({d}), ext_mods));
+    return move (r[0]);
+  }
+
   // Package naming schemes.
   //
   enum class package_scheme
@@ -95,10 +109,12 @@ namespace bpkg
   bool
   repository_name (const string&);
 
-  // Return the versions of packages as provided by the build2 version module.
-  // Return nullopt for a package if the version module is disabled for it (or
-  // the build2 project directory doesn't contain the manifest file). Fail if
-  // any of the specified directories is not a build2 project.
+  // Return the versions of packages as provided by the build2 version module
+  // together with the build2 project info the versions originate from (in
+  // case the caller may want to reuse it). Return nullopt as a package
+  // version if the version module is disabled for the package (or the build2
+  // project directory doesn't contain the manifest file). Fail if any of the
+  // specified directories is not a build2 project.
   //
   // Note that if a package directory is under the version control, then the
   // resulting version may be populated with the snapshot information (see
@@ -107,17 +123,37 @@ namespace bpkg
   //
   class common_options;
 
-  vector<optional<version>>
+  struct package_version_info
+  {
+    optional<bpkg::version> version;
+    package_info            info;
+  };
+
+  using package_version_infos = vector<package_version_info>;
+
+  package_version_infos
   package_versions (const common_options&, const dir_paths&);
 
   // As above but return the version of a single package.
   //
-  inline optional<version>
+  inline package_version_info
   package_version (const common_options& o, const dir_path& d)
   {
-    vector<optional<version>> r (package_versions (o, dir_paths ({d})));
+    package_version_infos r (package_versions (o, dir_paths ({d})));
     return move (r[0]);
   }
+
+  // Caclulate the checksum of the manifest file located in the package source
+  // directory and the subproject set (see package::manifest_checksum).
+  //
+  // Pass the build2 project info for the package, if available, to speed up
+  // the call and NULL otherwise (in which case it will be queried by the
+  // implementation).
+  //
+  string
+  package_checksum (const common_options&,
+                    const dir_path& src_dir,
+                    const package_info*);
 }
 
 #endif // BPKG_MANIFEST_UTILITY_HXX
diff --git a/bpkg/package.cxx b/bpkg/package.cxx
index 8ad178d..e0101fb 100644
--- a/bpkg/package.cxx
+++ b/bpkg/package.cxx
@@ -539,6 +539,7 @@ namespace bpkg
                      const dir_path& d,
                      const package_name& n,
                      const version& v,
+                     const package_info* pi,
                      bool check_external)
   {
     tracer trace ("package_iteration");
@@ -576,7 +577,7 @@ namespace bpkg
                             false /* iteration */))
       return nullopt;
 
-    string mc (sha256 (o, d / manifest_file));
+    string mc (package_checksum (o, d, pi));
 
     // The selected package must not be "simulated" (see pkg-build for
     // details).
diff --git a/bpkg/package.hxx b/bpkg/package.hxx
index da5c64a..cb1a401 100644
--- a/bpkg/package.hxx
+++ b/bpkg/package.hxx
@@ -1005,13 +1005,23 @@ namespace bpkg
     optional<dir_path> src_root;
     bool purge_src;
 
-    // The checksum of the manifest file located in the source directory.
+    // The checksum of the manifest file located in the source directory and
+    // the subproject set. Changes to this information should trigger the
+    // package version revision increment. In particular, new subprojects
+    // should trigger the package reconfiguration.
     //
     // Must be present if the source directory is present, unless the object
     // is created/updated during the package build simulation (see pkg-build
     // for details). Note that during the simulation the manifest may not be
     // available.
     //
+    // @@ Currently we don't consider subprojects recursively (would most
+    //    likely require extension to b info, also could be a performance
+    //    concern).
+    //
+    // @@ We should probably rename it if/when ODB add support for that for
+    //    SQlite.
+    //
     optional<std::string> manifest_checksum;
 
     // Path to the output directory of this package, if any. It is
@@ -1153,6 +1163,10 @@ namespace bpkg
   // considered its iteration. Return the version of this iteration if that's
   // the case and nullopt otherwise.
   //
+  // Pass the build2 project info for the package, if available, to speed up
+  // the call and NULL otherwise (in which case it will be queried by the
+  // implementation).
+  //
   // Notes:
   //
   // - The package directory is considered an iteration of the package if this
@@ -1190,6 +1204,7 @@ namespace bpkg
                      const dir_path&,
                      const package_name&,
                      const version&,
+                     const package_info*,
                      bool check_external);
 
   // certificate
diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx
index f59c2c0..60bdb08 100644
--- a/bpkg/pkg-build.cxx
+++ b/bpkg/pkg-build.cxx
@@ -4171,14 +4171,21 @@ namespace bpkg
                   info << "'" << package << "' does not appear to be a valid "
                        << "package directory: ";
 
+                // For better diagnostics, let's obtain the package info after
+                // pkg_verify() verifies that this is a package directory.
+                //
+                package_version_info pvi;
+
                 package_manifest m (
                   pkg_verify (
                     d,
                     true /* ignore_unknown */,
-                    [&o, &d] (version& v)
+                    [&o, &d, &pvi] (version& v)
                     {
-                      if (optional<version> pv = package_version (o, d))
-                        v = move (*pv);
+                      pvi = package_version (o, d);
+
+                      if (pvi.version)
+                        v = move (*pvi.version);
                     },
                     diag));
 
@@ -4208,6 +4215,7 @@ namespace bpkg
                                        d,
                                        m.name,
                                        m.version,
+                                       &pvi.info,
                                        true /* check_external */))
                   m.version = move (*v);
 
diff --git a/bpkg/pkg-checkout.cxx b/bpkg/pkg-checkout.cxx
index 5b69d40..94dd19b 100644
--- a/bpkg/pkg-checkout.cxx
+++ b/bpkg/pkg-checkout.cxx
@@ -301,7 +301,7 @@ namespace bpkg
              "config.dist.root='" + ord.representation () + "'",
              bspec);
 
-      mc = sha256 (o, d / manifest_file);
+      mc = package_checksum (o, d, nullptr /* package_info */);
     }
 
     if (p != nullptr)
diff --git a/bpkg/pkg-unpack.cxx b/bpkg/pkg-unpack.cxx
index 368a71c..04250f8 100644
--- a/bpkg/pkg-unpack.cxx
+++ b/bpkg/pkg-unpack.cxx
@@ -63,6 +63,7 @@ namespace bpkg
               transaction& t,
               package_name n,
               version v,
+              const package_info* pi,
               dir_path d,
               repository_location rl,
               bool purge,
@@ -75,7 +76,7 @@ namespace bpkg
     optional<string> mc;
 
     if (!simulate)
-      mc = sha256 (o, d / manifest_file);
+      mc = package_checksum (o, d, pi);
 
     // Make the package path absolute and normalized. If the package is inside
     // the configuration, use the relative path. This way we can move the
@@ -160,15 +161,22 @@ namespace bpkg
     if (!exists (d))
       fail << "package directory " << d << " does not exist";
 
+    // For better diagnostics, let's obtain the package info after
+    // pkg_verify() verifies that this is a package directory.
+    //
+    package_version_info pvi;
+
     // Verify the directory is a package and get its manifest.
     //
     package_manifest m (
       pkg_verify (d,
                   true /* ignore_unknown */,
-                  [&o, &d] (version& v)
+                  [&o, &d, &pvi] (version& v)
                   {
-                    if (optional<version> pv = package_version (o, d))
-                      v = move (*pv);
+                    pvi = package_version (o, d);
+
+                    if (pvi.version)
+                      v = move (*pvi.version);
                   }));
 
     l4 ([&]{trace << d << ": " << m.name << " " << m.version;});
@@ -179,8 +187,14 @@ namespace bpkg
 
     // Fix-up the package version.
     //
-    if (optional<version> v = package_iteration (
-          o, db, t, d, m.name, m.version, true /* check_external */))
+    if (optional<version> v = package_iteration (o,
+                                                 db,
+                                                 t,
+                                                 d,
+                                                 m.name,
+                                                 m.version,
+                                                 &pvi.info,
+                                                 true /* check_external */))
       m.version = move (*v);
 
     // Use the special root repository fragment as the repository fragment of
@@ -191,6 +205,7 @@ namespace bpkg
                        t,
                        move (m.name),
                        move (m.version),
+                       &pvi.info,
                        d,
                        repository_location (),
                        purge,
@@ -256,9 +271,10 @@ namespace bpkg
                        t,
                        move (n),
                        move (v),
+                       nullptr   /* package_info */,
                        path_cast<dir_path> (rl.path () / pl->location),
                        rl,
-                       false /* purge */,
+                       false     /* purge */,
                        simulate);
   }
 
@@ -329,7 +345,7 @@ namespace bpkg
         fail << "unable to extract " << a << " to " << c << ": " << e;
       }
 
-      mc = sha256 (co, d / manifest_file);
+      mc = package_checksum (co, d, nullptr /* package_info */);
     }
 
     p->src_root = d.leaf (); // For now assuming to be in configuration.
diff --git a/bpkg/rep-fetch.cxx b/bpkg/rep-fetch.cxx
index 80598c5..de6e3b7 100644
--- a/bpkg/rep-fetch.cxx
+++ b/bpkg/rep-fetch.cxx
@@ -192,7 +192,7 @@ namespace bpkg
 
   // Parse package manifests referenced by the package directory manifests.
   //
-  static vector<package_manifest>
+  static pair<vector<package_manifest>, vector<package_info>>
   parse_package_manifests (const common_options& co,
                            const dir_path& repo_dir,
                            vector<package_manifest>&& pms,
@@ -200,8 +200,8 @@ namespace bpkg
                            const repository_location& rl,
                            const optional<string>& fragment) // For diagnostics.
   {
-    auto package_info = [&rl, &fragment] (const package_manifest& pm,
-                                          diag_record& dr)
+    auto add_package_info = [&rl, &fragment] (const package_manifest& pm,
+                                              diag_record& dr)
     {
       dr << "package ";
 
@@ -218,8 +218,8 @@ namespace bpkg
     // files and retrieve the package versions via the single `b info` call.
     // While at it cache the manifest paths for the future use.
     //
-    paths                     mfs;
-    vector<optional<version>> pvs;
+    paths                 mfs;
+    package_version_infos pvs;
     {
       mfs.reserve (pms.size ());
 
@@ -238,7 +238,7 @@ namespace bpkg
         {
           diag_record dr (fail);
           dr << "no manifest file for ";
-          package_info (pm, dr);
+          add_package_info (pm, dr);
         }
 
         mfs.push_back (move (f));
@@ -250,8 +250,9 @@ namespace bpkg
 
     // Parse package manifests, fixing up their versions.
     //
-    vector<package_manifest> r;
-    r.reserve (pms.size ());
+    pair<vector<package_manifest>, vector<package_info>> r;
+    r.first.reserve  (pms.size ());
+    r.second.reserve (pms.size ());
 
     for (size_t i (0); i != pms.size (); ++i)
     {
@@ -266,7 +267,7 @@ namespace bpkg
         ifdstream ifs (f);
         manifest_parser mp (ifs, f.string ());
 
-        optional<version>& pv (pvs[i]);
+        optional<version>& pv (pvs[i].version);
 
         package_manifest m (
           mp,
@@ -286,14 +287,15 @@ namespace bpkg
       {
         diag_record dr (fail (e.name, e.line, e.column));
         dr << e.description << info;
-        package_info (pm, dr);
+        add_package_info (pm, dr);
       }
       catch (const io_error& e)
       {
         fail << "unable to read from " << f << ": " << e;
       }
 
-      r.emplace_back (move (pm));
+      r.first.push_back  (move (pm));
+      r.second.push_back (move (pvs[i].info));
     }
 
     return r;
@@ -360,12 +362,16 @@ namespace bpkg
         rl,
         string () /* fragment */));
 
-    fr.packages = parse_package_manifests (co,
-                                           rd,
-                                           move (pms),
-                                           iu,
-                                           rl,
-                                           empty_string /* fragment */);
+    pair<vector<package_manifest>, vector<package_info>> pmi (
+      parse_package_manifests (co,
+                               rd,
+                               move (pms),
+                               iu,
+                               rl,
+                               empty_string /* fragment */));
+
+    fr.packages      = move (pmi.first);
+    fr.package_infos = move (pmi.second);
 
     // Expand file-referencing package manifest values.
     //
@@ -520,12 +526,16 @@ namespace bpkg
 
       // Parse package manifests.
       //
-      fr.packages = parse_package_manifests (co,
-                                             td,
-                                             move (pms),
-                                             iu,
-                                             rl,
-                                             fr.friendly_name);
+      pair<vector<package_manifest>, vector<package_info>> pmi (
+        parse_package_manifests (co,
+                                 td,
+                                 move (pms),
+                                 iu,
+                                 rl,
+                                 fr.friendly_name));
+
+      fr.packages      = move (pmi.first);
+      fr.package_infos = move (pmi.second);
 
       // Expand file-referencing package manifest values checking out
       // submodules, if required.
@@ -890,12 +900,19 @@ namespace bpkg
     if (exists && !full_fetch)
       rep_remove_package_locations (db, t, rf->name);
 
-    for (package_manifest& pm: fr.packages)
+    vector<package_manifest>&   pms (fr.packages);
+    const vector<package_info>& pis (fr.package_infos);
+
+    for (size_t i (0); i != pms.size (); ++i)
     {
+      package_manifest& pm (pms[i]);
+
       // Fix-up the external package version iteration number.
       //
       if (rl.directory_based ())
       {
+        assert (!pis.empty ());
+
         // Note that we can't check if the external package of this upstream
         // version and revision is already available in the configuration
         // until we fetch all the repositories, as some of the available
@@ -909,7 +926,8 @@ namespace bpkg
             path_cast<dir_path> (rl.path () / *pm.location),
             pm.name,
             pm.version,
-            false /* check_external */));
+            &pis[i],
+            false   /* check_external */));
 
         if (v)
           pm.version = move (*v);
diff --git a/bpkg/rep-fetch.hxx b/bpkg/rep-fetch.hxx
index 7905e85..2445129 100644
--- a/bpkg/rep-fetch.hxx
+++ b/bpkg/rep-fetch.hxx
@@ -35,6 +35,12 @@ namespace bpkg
 
       vector<repository_manifest> repositories;
       vector<package_manifest>    packages;
+
+      // Empty if the build2 project info is not available for the packages.
+      // Currently we only retrieve it for the directory and version control
+      // based repositories.
+      //
+      vector<package_info>        package_infos;
     };
 
     vector<fragment> fragments;
diff --git a/bpkg/types.hxx b/bpkg/types.hxx
index 8e00666..6065dd8 100644
--- a/bpkg/types.hxx
+++ b/bpkg/types.hxx
@@ -21,6 +21,7 @@
 
 #include <odb/lazy-ptr.hxx>
 
+#include <libbutl/b.mxx>
 #include <libbutl/url.mxx>
 #include <libbutl/path.mxx>
 #include <libbutl/uuid.hxx>
@@ -125,6 +126,10 @@ namespace bpkg
   using butl::default_options_entry;
   using butl::default_options;
 
+  // <libbutl/b.mxx>
+  //
+  using package_info = butl::b_project_info;
+
   // Derive from ODB smart pointers to return derived database (note that the
   // database() functions are defined in database.hxx).
   //