treemanifest: add ManifestNode type and use it in NewTreeIter

Summary:
As part of enabling deltas in our tree pack files, we need NewTreeIter to return
the p1 and p2 Manifests as well. To do this we need to return a Manifest/Node
tuple. Since this is becoming a common structure, let's define a type for it and
change NewTreeIter to use it for it's return values.

Test Plan:
Ran the tests. With a future diff I built a pack file and verified it
was small because of deltas.

Reviewers: #mercurial

Differential Revision: https://phabricator.intern.facebook.com/D4202080

ctreemanifest/manifest.h

@@ -205,4 +205,10 @@ class SortedManifestIterator {
     bool isfinished() const;
 };
 
+class ManifestNode {
+  public:
+    Manifest *manifest;
+    char node[BIN_NODE_SIZE];
+};
+
 #endif //REMOTEFILELOG_MANIFEST_H

ctreemanifest/py-treemanifest.cpp

@@ -962,18 +962,18 @@ void writestore(Manifest *mainManifest, const std::vector<char*> &cmpNodes,
   NewTreeIterator iterator(mainManifest, cmpNodes, cmpManifests, fetcher);
 
   std::string *path = NULL;
-  Manifest *result = NULL;
-  std::string *node = NULL;
+  ManifestNode *result = NULL;
+  ManifestNode *p1 = NULL;
+  ManifestNode *p2 = NULL;
   std::string raw;
-  while (iterator.next(&path, &result, &node)) {
+  while (iterator.next(&path, &result, &p1, &p2)) {
     // TODO: find an appropriate delta base and compute the delta
-    result->serialize(raw);
+    result->manifest->serialize(raw);
     PythonObj args = Py_BuildValue("(s#s#s#s#)",
                                    path->c_str(), (Py_ssize_t)path->size(),
-                                   node->c_str(), (Py_ssize_t)BIN_NODE_SIZE,
+                                   result->node, (Py_ssize_t)BIN_NODE_SIZE,
                                    NULLID, (Py_ssize_t)BIN_NODE_SIZE,
                                    raw.c_str(), (Py_ssize_t)raw.size());
-
     pack.callmethod("add", args);
   }
 }

ctreemanifest/treemanifest.cpp

@@ -539,16 +539,16 @@ NewTreeIterator::NewTreeIterator(Manifest *mainRoot,
   }
 }
 
-bool NewTreeIterator::popResult(std::string **path, Manifest **result, std::string **node) {
+bool NewTreeIterator::popResult(std::string **path, ManifestNode **result,
+                                ManifestNode **p1, ManifestNode **p2) {
   stackframe &mainFrame = this->mainStack.back();
   Manifest *mainManifest = mainFrame.manifest;
   std::string mainSerialized;
 
   // When we loop over the cmpStacks, record the cmp nodes that are parents
   // of the level we're about to return.
-  char parentNodes[2][BIN_NODE_SIZE];
-  memcpy(parentNodes[0], NULLID, BIN_NODE_SIZE);
-  memcpy(parentNodes[1], NULLID, BIN_NODE_SIZE);
+  memcpy(this->parents[0].node, NULLID, BIN_NODE_SIZE);
+  memcpy(this->parents[1].node, NULLID, BIN_NODE_SIZE);
 
   bool alreadyExists = false;
 
@@ -579,10 +579,14 @@ bool NewTreeIterator::popResult(std::string **path, Manifest **result, std::stri
       if (cmpStack.size() > 1) {
         stackframe &priorCmpFrame = cmpStack[cmpStack.size() - 2];
         ManifestEntry *priorCmpEntry = priorCmpFrame.currentvalue();
-        memcpy(parentNodes[i], binfromhex(priorCmpEntry->node).c_str(), BIN_NODE_SIZE);
+        this->parents[i].manifest = cmpManifest;
+        memcpy(this->parents[i].node, binfromhex(priorCmpEntry->node).c_str(),
+               BIN_NODE_SIZE);
       } else {
         // Use the original passed in parent nodes
-        memcpy(parentNodes[i], binfromhex(this->cmpNodes[i]).c_str(), BIN_NODE_SIZE);
+        this->parents[i].manifest = cmpManifest;
+        memcpy(this->parents[i].node, binfromhex(this->cmpNodes[i]).c_str(),
+               BIN_NODE_SIZE);
       }
     }
   }
@@ -596,7 +600,8 @@ bool NewTreeIterator::popResult(std::string **path, Manifest **result, std::stri
   }
 
   char tempnode[BIN_NODE_SIZE];
-  mainManifest->computeNode(parentNodes[0], parentNodes[1], tempnode);
+  mainManifest->computeNode(this->parents[0].node, this->parents[1].node,
+                            tempnode);
 
   // Update the node on the manifest entry
   if (mainStack.size() > 0) {
@@ -620,11 +625,13 @@ bool NewTreeIterator::popResult(std::string **path, Manifest **result, std::stri
     return false;
   }
 
-  this->node.assign(tempnode, 20);
+  this->result.manifest = mainManifest;
+  memcpy(this->result.node, tempnode, BIN_NODE_SIZE);
 
   *path = &this->path;
-  *result = mainManifest;
-  *node = &this->node;
+  *result = &this->result;
+  *p1 = &this->parents[0];
+  *p2 = &this->parents[1];
   return true;
 }
 
@@ -660,7 +667,7 @@ bool NewTreeIterator::processDirectory(ManifestEntry *mainEntry) {
         if (cmp == 0) {
           // And the nodes match...
           if (!alreadyExists &&
-              (mainEntry->node && strncmp(mainEntry->node, cmpEntry->node, 40) == 0)) {
+              (mainEntry->node && memcmp(mainEntry->node, cmpEntry->node, HEX_NODE_SIZE) == 0)) {
             // Skip this entry
             alreadyExists = true;
           }
@@ -697,7 +704,8 @@ bool NewTreeIterator::processDirectory(ManifestEntry *mainEntry) {
   return true;
 }
 
-bool NewTreeIterator::next(std::string **path, Manifest **result, std::string **node) {
+bool NewTreeIterator::next(std::string **path, ManifestNode **result,
+                           ManifestNode **p1, ManifestNode **p2) {
   // Pop the last returned directory off the path
   size_t slashoffset = this->path.find_last_of('/', this->path.size() - 2);
   if (slashoffset == std::string::npos) {
@@ -718,7 +726,7 @@ bool NewTreeIterator::next(std::string **path, Manifest **result, std::string **
     if (mainFrame.isfinished()) {
       // This can return false if this manifest ended up being equivalent to
       // a cmp parent manifest, which means we should skip it.
-      if (this->popResult(path, result, node)) {
+      if (this->popResult(path, result, p1, p2)) {
         if (this->mainStack.size() > 0) {
           this->mainStack.back().next();
         }

ctreemanifest/treemanifest.h

@@ -251,7 +251,8 @@ class NewTreeIterator {
     std::vector<char*> cmpNodes;
     std::vector<std::vector<stackframe> > cmpStacks;
     std::string path;
-    std::string node;
+    ManifestNode result;
+    ManifestNode parents[2];
     ManifestFetcher fetcher;
   public:
     NewTreeIterator(Manifest *mainRoot,
@@ -265,13 +266,15 @@ class NewTreeIterator {
      * Return true if a manifest was returned, or false if we've reached the
      * end.
      */
-    bool next(std::string **path, Manifest **result, std::string **node);
+    bool next(std::string **path, ManifestNode **result,
+              ManifestNode **p1, ManifestNode **p2);
   private:
     /**
      * Pops the current Manifest, populating the output values and returning true
      * if the current Manifest is different from all comparison manifests.
      */
-    bool popResult(std::string **path, Manifest **result, std::string **node);
+    bool popResult(std::string **path, ManifestNode **result,
+                   ManifestNode **p1, ManifestNode **p2);
 
     /** Pushes the given Manifest onto the stacks. If the given Manifest equals
      * one of the comparison Manifests, the function does nothing.