Fix up package manifest version for pkg-unpack

9 files changed, 175 insertions, 118 deletions

diff --git a/bpkg/manifest-utility.cxx b/bpkg/manifest-utility.cxx
index 50ee4e8..b35af4a 100644
--- a/bpkg/manifest-utility.cxx
+++ b/bpkg/manifest-utility.cxx
@@ -6,8 +6,11 @@
 
 #include <libbutl/url.mxx>
 #include <libbutl/sha256.mxx>
+#include <libbutl/process.mxx>
+#include <libbutl/process-io.mxx> // operator<<(ostream, process_path)
 
 #include <bpkg/diagnostics.hxx>
+#include <bpkg/common-options.hxx>
 
 using namespace std;
 using namespace butl;
@@ -180,4 +183,96 @@ namespace bpkg
       return false;
     }
   }
+
+  optional<version>
+  package_version (const common_options& o, const dir_path& d)
+  {
+    const char* b (name_b (o));
+
+    try
+    {
+      process_path pp (process::path_search (b, exec_dir));
+
+      fdpipe pipe (open_pipe ());
+
+      process pr (
+        process_start_callback (
+          [] (const char* const args[], size_t n)
+          {
+            if (verb >= 2)
+              print_process (args, n);
+          },
+          0 /* stdin */, pipe /* stdout */, 2 /* stderr */,
+          pp,
+
+          verb < 2
+          ? strings ({"-q"})
+          : verb == 2
+          ? strings ({"-v"})
+          : strings ({"--verbose", to_string (verb)}),
+
+          o.build_option (),
+          "info:",
+          d.representation ()));
+
+      // Shouldn't throw, unless something is severely damaged.
+      //
+      pipe.out.close ();
+
+      try
+      {
+        optional<version> r;
+
+        ifdstream is (move (pipe.in),
+                      fdstream_mode::skip,
+                      ifdstream::badbit);
+
+        for (string l; !eof (getline (is, l)); )
+        {
+          if (l.compare (0, 9, "version: ") == 0)
+          try
+          {
+            string v (l, 9);
+
+            // An empty version indicates that the version module is not
+            // enabled for the project.
+            //
+            if (!v.empty ())
+              r = version (v);
+
+            break;
+          }
+          catch (const invalid_argument&)
+          {
+            fail << "no package version in '" << l << "'" <<
+              info << "produced by '" << pp << "'; use --build to override";
+          }
+        }
+
+        is.close ();
+
+        if (pr.wait ())
+          return r;
+
+        // Fall through.
+      }
+      catch (const io_error&)
+      {
+        if (pr.wait ())
+          fail << "unable to read '" << b << "' output";
+
+        // Fall through.
+      }
+
+      // We should only get here if the child exited with an error status.
+      //
+      assert (!pr.wait ());
+
+      fail << "unable to obtain version using '" << b << "'" << endf;
+    }
+    catch (const process_error& e)
+    {
+      fail << "unable to execute '" << b << "': " << e << endf;
+    }
+  }
 }
diff --git a/bpkg/manifest-utility.hxx b/bpkg/manifest-utility.hxx
index f608858..ee73c4a 100644
--- a/bpkg/manifest-utility.hxx
+++ b/bpkg/manifest-utility.hxx
@@ -74,6 +74,21 @@ namespace bpkg
   //
   bool
   repository_name (const string&);
+
+  // Return the version of a package as provided by the build2 version module.
+  // Return nullopt if the version module is disabled for the package (or the
+  // build2 project directory doesn't contain the manifest file). Fail if the
+  // directory is not a build2 project.
+  //
+  // Note that if the package directory is under the version control, then the
+  // resulting version may be populated with the snapshot information (see
+  // libbutl/standard-version.mxx for more details). Thus, this function can
+  // be used for fixing up the package manifest version.
+  //
+  class common_options;
+
+  optional<version>
+  package_version (const common_options&, const dir_path&);
 }
 
 #endif // BPKG_MANIFEST_UTILITY_HXX
diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx
index 5395d8d..cc14f1e 100644
--- a/bpkg/pkg-build.cxx
+++ b/bpkg/pkg-build.cxx
@@ -2099,7 +2099,8 @@ namespace bpkg
             assert (pl.repository.object_id () == ""); // Special root.
 
             transaction t (db.begin ());
-            sp = pkg_unpack (c,
+            sp = pkg_unpack (o,
+                             c,
                              t,
                              path_cast<dir_path> (pl.location),
                              true,   // Replace.
diff --git a/bpkg/pkg-unpack.cxx b/bpkg/pkg-unpack.cxx
index 9bd3e03..7472194 100644
--- a/bpkg/pkg-unpack.cxx
+++ b/bpkg/pkg-unpack.cxx
@@ -127,7 +127,8 @@ namespace bpkg
   }
 
   shared_ptr<selected_package>
-  pkg_unpack (const dir_path& c,
+  pkg_unpack (const common_options& o,
+              const dir_path& c,
               transaction& t,
               const dir_path& d,
               bool replace,
@@ -147,6 +148,13 @@ namespace bpkg
     //
     pkg_unpack_check (c, t, m.name, replace);
 
+    // Fix-up the package version.
+    //
+    optional<version> v (package_version (o, d));
+
+    if (v)
+      m.version = move (*v);
+
     // Use the special root repository as the repository of this
     // package.
     //
@@ -411,7 +419,7 @@ namespace bpkg
           info << "run 'bpkg help pkg-unpack' for more information";
 
       p = pkg_unpack (
-        c, t, dir_path (args.next ()), o.replace (), o.purge ());
+        o, c, t, dir_path (args.next ()), o.replace (), o.purge ());
     }
     else
     {
diff --git a/bpkg/pkg-unpack.hxx b/bpkg/pkg-unpack.hxx
index 9fdc848..a04d197 100644
--- a/bpkg/pkg-unpack.hxx
+++ b/bpkg/pkg-unpack.hxx
@@ -21,7 +21,8 @@ namespace bpkg
   // Unpack the package as a source directory and commit the transaction.
   //
   shared_ptr<selected_package>
-  pkg_unpack (const dir_path& configuration,
+  pkg_unpack (const common_options&,
+              const dir_path& configuration,
               transaction&,
               const dir_path&,
               bool replace,
diff --git a/bpkg/rep-fetch.cxx b/bpkg/rep-fetch.cxx
index 2e884c4..8f1528c 100644
--- a/bpkg/rep-fetch.cxx
+++ b/bpkg/rep-fetch.cxx
@@ -6,8 +6,6 @@
 
 #include <set>
 
-#include <libbutl/process.mxx>
-#include <libbutl/process-io.mxx>      // operator<<(ostream, process_path)
 #include <libbutl/manifest-parser.mxx>
 
 #include <bpkg/auth.hxx>
@@ -231,103 +229,14 @@ namespace bpkg
         package_info (dr);
       }
 
-      // Fix-up the package version. Note that the package may have the
-      // version module enable and the directory repository may well be a git
-      // repository.
+      // Fix-up the package version.
       //
-      const char* b (name_b (co));
+      optional<version> v (package_version (co, d));
 
-      try
-      {
-        process_path pp (process::path_search (b, exec_dir));
-
-        fdpipe pipe (open_pipe ());
-
-        process pr (
-          process_start_callback (
-            [] (const char* const args[], size_t n)
-            {
-              if (verb >= 2)
-                print_process (args, n);
-            },
-            0 /* stdin */, pipe /* stdout */, 2 /* stderr */,
-            pp,
-
-            verb < 2
-            ? strings ({"-q"})
-            : verb == 2
-              ? strings ({"-v"})
-              : strings ({"--verbose", to_string (verb)}),
-
-            co.build_option (),
-            "info:",
-            d.representation ()));
-
-        // Shouldn't throw, unless something is severely damaged.
-        //
-        pipe.out.close ();
-
-        try
-        {
-          ifdstream is (move (pipe.in),
-                        fdstream_mode::skip,
-                        ifdstream::badbit);
-
-          for (string l; !eof (getline (is, l)); )
-          {
-            if (l.compare (0, 9, "version: ") == 0)
-            try
-            {
-              string v (l, 9);
-
-              // An empty version indicates that the version module is not
-              // enabled for the project, and so we don't amend the package
-              // version.
-              //
-              if (!v.empty ())
-                sm.version = version (v);
-
-              break;
-            }
-            catch (const invalid_argument&)
-            {
-              fail << "no package version in '" << l << "'" <<
-                info << "produced by '" << pp << "'; use --build to override";
-            }
-          }
-
-          is.close ();
-
-          // If succeess then save the package manifest together with the
-          // repository state it belongs to and go to the next package.
-          //
-          if (pr.wait ())
-          {
-            fps.emplace_back (rep_fetch_data::package {move (sm),
-                                                       repo_fragment});
-            continue;
-          }
-
-          // Fall through.
-        }
-        catch (const io_error&)
-        {
-          if (pr.wait ())
-            failure ("unable to read information");
-
-          // Fall through.
-        }
-
-        // We should only get here if the child exited with an error status.
-        //
-        assert (!pr.wait ());
+      if (v)
+        sm.version = move (*v);
 
-        failure ("unable to obtain information");
-      }
-      catch (const process_error& e)
-      {
-        fail << "unable to execute " << b << ": " << e;
-      }
+      fps.emplace_back (rep_fetch_data::package {move (sm), repo_fragment});
     }
 
     return fps;
diff --git a/tests/pkg-unpack.test b/tests/pkg-unpack.test
index 0795f6b..e4cff16 100644
--- a/tests/pkg-unpack.test
+++ b/tests/pkg-unpack.test
@@ -2,7 +2,7 @@
 # copyright : Copyright (c) 2014-2017 Code Synthesis Ltd
 # license   : MIT; see accompanying LICENSE file
 
-.include common.test auth.test config.test remote.test
+.include common.test auth.test config.test remote.test remote-git.test
 
 # Source repository:
 #
@@ -15,10 +15,13 @@
 # |   |   `-- bootstrap.build
 # |   |-- buildfile
 # |   `-- manifest
-# `-- t1
-#     |-- libfoo-1.0.0.tar.gz
-#     |-- libfoo-1.1.0.tar.gz
-#     `-- repositories.manifest
+# |-- t1
+# |   |-- libfoo-1.0.0.tar.gz
+# |   |-- libfoo-1.1.0.tar.gz
+# |   `-- repositories.manifest
+# |
+# `-- git
+#     `-- style-basic.git
 
 # Prepare repositories used by tests if running in the local mode.
 #
@@ -36,6 +39,10 @@
   # Create the 't1' repository.
   #
   cp -r $src/t1 $out/t1 && $rep_create $out/t1 &$out/t1/packages.manifest
+
+  # Create git repositories.
+  #
+  $git_extract $src/git/style-basic.tar &$out_git/state0/***
 end
 
 pkg_fetch     += -d cfg 2>!
@@ -193,18 +200,6 @@ $* 2>>EOE != 0
   }
 }
 
-: hello
-:
-{
-  $clone_cfg;
-  $rep_add $rep/hello && $rep_fetch --trust $cert_fp &cfg/.bpkg/certs/**;
-  $pkg_fetch libhello/1.0.0;
-
-  $* libhello 2>'unpacked libhello/1.0.0';
-
-  $pkg_purge libhello 2>'purged libhello/1.0.0'
-}
-
 : dir-rep
 :
 {
@@ -268,3 +263,35 @@ $* 2>>EOE != 0
     $pkg_disfigure libfoo  2>'disfigured libfoo/1.1.0'
   }
 }
+
+: existing
+:
+{
+  : version-fixup
+  :
+  : Test that the version is populated with the snapshot information for the
+  : package directory that is under the version control.
+  :
+  if ($remote != true)
+  {
+    $clone_root_cfg;
+
+    rep = $canonicalize([dir_path] $out_git/state0);
+
+    $* -e $rep/style-basic.git 2>>~%EOE%
+      %using style-basic/1\.1\.0-a\.0\.\d+\..+ \(external\)%
+      EOE
+  }
+}
+
+: hello
+:
+{
+  $clone_cfg;
+  $rep_add $rep/hello && $rep_fetch --trust $cert_fp &cfg/.bpkg/certs/**;
+  $pkg_fetch libhello/1.0.0;
+
+  $* libhello 2>'unpacked libhello/1.0.0';
+
+  $pkg_purge libhello 2>'purged libhello/1.0.0'
+}
diff --git a/tests/pkg-unpack/git/style-basic.tar b/tests/pkg-unpack/git/style-basic.tar
new file mode 120000
index 0000000..2833f83
--- /dev/null
+++ b/tests/pkg-unpack/git/style-basic.tar
@@ -0,0 +1 @@
+../../common/git/state0/style-basic.tar
\ No newline at end of file
diff --git a/tests/rep-info.test b/tests/rep-info.test
index 9a70b49..cdc9e70 100644
--- a/tests/rep-info.test
+++ b/tests/rep-info.test
@@ -157,7 +157,7 @@ else
   $* "$rep/style-basic.git#master" >>~%EOO%
   %git:.+style-basic#master .+style-basic.git#master%
 
-  %style-basic/1\.1\.0-a\.0\..+%
+  %style-basic/1\.1\.0-a\.0\.\d+\..+%
   EOO
 
   : manifest-lists