Fix git commit id calculation

5 files changed, 93 insertions, 65 deletions

diff --git a/build2/cc/msvc.cxx b/build2/cc/msvc.cxx
index aa4095e..a4c4317 100644
--- a/build2/cc/msvc.cxx
+++ b/build2/cc/msvc.cxx
@@ -216,7 +216,7 @@ namespace build2
         // that.
       }
 
-      if (!run_finish (args, false, pr, s))
+      if (!run_finish (args, pr, false, s))
         return otype::e;
 
       if (obj && dll)
diff --git a/build2/utility.cxx b/build2/utility.cxx
index 4945514..d11b46d 100644
--- a/build2/utility.cxx
+++ b/build2/utility.cxx
@@ -234,7 +234,7 @@ namespace build2
   }
 
   bool
-  run_finish (const char* args[], bool err, process& pr, const string& l)
+  run_finish (const char* args[], process& pr, bool err, const string& l)
   try
   {
     if (pr.wait ())
diff --git a/build2/utility.hxx b/build2/utility.hxx
index 7d1ffc6..8ba4fe1 100644
--- a/build2/utility.hxx
+++ b/build2/utility.hxx
@@ -180,16 +180,19 @@ namespace build2
   run_search (const path&, bool init, const dir_path& fallback = dir_path ());
 
   process
-  run_start (const process_path&, const char* args[], bool error);
+  run_start (const process_path&, const char* args[], bool error = true);
 
   inline process
-  run_start (const char* args[], bool error)
+  run_start (const char* args[], bool error = true)
   {
     return run_start (run_search (args[0]), args, error);
   }
 
   bool
-  run_finish (const char* args[], bool error, process&, const string&);
+  run_finish (const char* args[],
+              process&,
+              bool error = true,
+              const string& = string ());
 
   // Start the process as above and then call the specified function on each
   // trimmed line of the output until it returns a non-empty object T (tested
@@ -203,7 +206,7 @@ namespace build2
   // is false and the program exits with the non-zero status, then an empty T
   // instance is returned).
   //
-  // If checksum is not NULL, then feed it the content of each tripped line
+  // If checksum is not NULL, then feed it the content of each trimmed line
   // (including those that come after the callback returns non-empty object).
   //
   template <typename T, typename F>
diff --git a/build2/utility.txx b/build2/utility.txx
index 19a9819..d195ea6 100644
--- a/build2/utility.txx
+++ b/build2/utility.txx
@@ -98,7 +98,7 @@ namespace build2
       // Presumably the child process failed. Let run_finish() deal with that.
     }
 
-    if (!(run_finish (args, err, pr, l) || ignore_exit))
+    if (!(run_finish (args, pr, err, l) || ignore_exit))
       r = T ();
 
     return r;
diff --git a/build2/version/snapshot-git.cxx b/build2/version/snapshot-git.cxx
index d91eb62..9ff1cc9 100644
--- a/build2/version/snapshot-git.cxx
+++ b/build2/version/snapshot-git.cxx
@@ -2,9 +2,12 @@
 // copyright : Copyright (c) 2014-2017 Code Synthesis Ltd
 // license   : MIT; see accompanying LICENSE file
 
+#include <libbutl/sha1.mxx>
+
 #include <build2/version/snapshot.hxx>
 
 using namespace std;
+using namespace butl;
 
 namespace build2
 {
@@ -31,75 +34,97 @@ namespace build2
           return r;
       }
 
-      // Now extract the commit id and date.
+      // Now extract the commit id and date. One might think that would be
+      // easy... Commit id is a SHA1 hash of the commit object. And commit
+      // object looks like this:
+      //
+      // commit <len>\0
+      // <data>
+      //
+      // Where <len> is the size of <data> and <data> is the output of:
       //
-      auto extract = [&r] (string& s) -> snapshot
+      // git cat-file commit ...
+      //
+      string data;
+
+      const char* args[] {
+        "git", "-C", d, "cat-file", "commit", "HEAD", nullptr};
+      process pr (run_start (args));
+
+      try
       {
-        if (s.compare (0, 5, "tree ") == 0)
-        {
-          // The 16-characters abbreviated commit id.
-          //
-          r.id.assign (s, 5, 16);
+        ifdstream is (move (pr.in_ofd), ifdstream::badbit);
 
-          if (r.id.size () != 16)
-            fail << "unable to extract git commit id from '" << s << "'";
-        }
-        else if (s.compare (0, 10, "committer ") == 0)
-        try
+        for (string l; !eof (getline (is, l)); )
         {
-          // The line format is:
-          //
-          // committer <noise> <timestamp> <timezone>
-          //
-          // For example:
-          //
-          // committer John Doe <john@example.org> 1493117819 +0200
-          //
-          // The timestamp is in seconds since UNIX epoch. The timezone
-          // appears to be always numeric (+0000 for UTC).
-          //
-          size_t p1 (s.rfind (' ')); // Can't be npos.
-          string tz (s, p1 + 1);
-
-          size_t p2 (s.rfind (' ', p1 - 1));
-          if (p2 == string::npos)
-            throw invalid_argument ("missing timestamp");
-
-          string ts (s, p2 + 1, p1 - p2 - 1);
-          r.sn = stoull (ts);
-
-          if (tz.size () != 5)
-            throw invalid_argument ("invalid timezone");
-
-          unsigned long h (stoul (string (tz, 1, 2)));
-          unsigned long m (stoul (string (tz, 3, 2)));
-          unsigned long s (h * 3600 + m * 60);
-
-          // The timezone indicates where the timestamp was generated so
-          // to convert to UTC we need to invert the sign.
-          //
-          switch (tz[0])
+          data += l;
+          data += '\n'; // We assume there is always a newline.
+
+          if (r.sn == 0 && l.compare (0, 10, "committer ") == 0)
+          try
           {
-          case '+': r.sn -= s; break;
-          case '-': r.sn += s; break;
-          default: throw invalid_argument ("invalid timezone sign");
+            // The line format is:
+            //
+            // committer <noise> <timestamp> <timezone>
+            //
+            // For example:
+            //
+            // committer John Doe <john@example.org> 1493117819 +0200
+            //
+            // The timestamp is in seconds since UNIX epoch. The timezone
+            // appears to be always numeric (+0000 for UTC).
+            //
+            size_t p1 (l.rfind (' ')); // Can't be npos.
+            string tz (l, p1 + 1);
+
+            size_t p2 (l.rfind (' ', p1 - 1));
+            if (p2 == string::npos)
+              throw invalid_argument ("missing timestamp");
+
+            string ts (l, p2 + 1, p1 - p2 - 1);
+            r.sn = stoull (ts);
+
+            if (tz.size () != 5)
+              throw invalid_argument ("invalid timezone");
+
+            unsigned long h (stoul (string (tz, 1, 2)));
+            unsigned long m (stoul (string (tz, 3, 2)));
+            unsigned long s (h * 3600 + m * 60);
+
+            // The timezone indicates where the timestamp was generated so to
+            // convert to UTC we need to invert the sign.
+            //
+            switch (tz[0])
+            {
+            case '+': r.sn -= s; break;
+            case '-': r.sn += s; break;
+            default: throw invalid_argument ("invalid timezone sign");
+            }
+          }
+          catch (const invalid_argument& e)
+          {
+            fail << "unable to extract git commit date from '" << l << "': "
+                 << e;
           }
-        }
-        catch (const invalid_argument& e)
-        {
-          fail << "unable to extract git commit date from '" << s << "': " << e;
         }
 
-        return (r.id.empty () || r.sn == 0) ? snapshot () : move (r);
-      };
-
-      const char* args[] {
-        "git", "-C", d, "cat-file", "commit", "HEAD", nullptr};
-      r = run<snapshot> (args, extract);
+        is.close ();
+      }
+      catch (const io_error&)
+      {
+        // Presumably the child process failed. Let run_finish() deal with
+        // that.
+      }
 
-      if (r.empty ())
+      if (!run_finish (args, pr) || r.sn == 0)
         fail << "unable to extract git commit id/date for " << src_root;
 
+      sha1 cs;
+      cs.append ("commit " + to_string (data.size ())); // Includes '\0'.
+      cs.append (data.c_str (), data.size ());
+
+      r.id.assign (cs.string (), 16); // 16-characters abbreviated commit id.
+
       return r;
     }
   }