Make it possible to generate hashes for uploaded content during upload

Summary:
Previously, we assumed that all content hashes came from Mercurial;
this is not going to remain true, as we will want to be able to upload manifests
that have been synthesised from Bonsai Changesets. Turn the previous boolean
into a tri-state, and fix up all callers to get the behaviour they expect.

Reviewed By: StanislavGlebik

Differential Revision: D8014911

fbshipit-source-id: 9156b9fab4542ceb269626ad005e1b28392b5329

blobrepo/src/lib.rs

@@ -63,7 +63,8 @@ pub use errors::*;
 pub use changeset::{BlobChangeset, ChangesetContent};
 pub use file::HgBlobEntry;
 pub use manifest::BlobManifest;
-pub use repo::{BlobRepo, ContentBlobInfo, ContentBlobMeta, CreateChangeset, UploadHgEntry};
+pub use repo::{BlobRepo, ContentBlobInfo, ContentBlobMeta, CreateChangeset, UploadHgEntry,
+               UploadHgNodeHash};
 pub use repo_commit::ChangesetHandle;
 // TODO: This is exported for testing - is this the right place for it?
 pub use repo_commit::compute_changed_files;

blobrepo/src/repo.rs

@@ -419,19 +419,26 @@ impl BlobRepo {
     }
 }
 
-/// Context for uploading a Mercurial entry.
-pub struct UploadHgEntry {
+/// Node hash handling for upload entries
+pub enum UploadHgNodeHash {
+    /// Generate the hash from the uploaded content
+    Generate,
     /// This hash is used as the blobstore key, even if it doesn't match the hash of the
     /// parents and raw content. This is done because in some cases like root tree manifests
     /// in hybrid mode, Mercurial sends fake hashes.
-    pub nodeid: HgNodeHash,
+    Supplied(HgNodeHash),
+    /// As Supplied, but Verify the supplied hash - if it's wrong, you will get an error.
+    Checked(HgNodeHash),
+}
+
+/// Context for uploading a Mercurial entry.
+pub struct UploadHgEntry {
+    pub upload_nodeid: UploadHgNodeHash,
     pub raw_content: HgBlob,
     pub content_type: manifest::Type,
     pub p1: Option<HgNodeHash>,
     pub p2: Option<HgNodeHash>,
     pub path: RepoPath,
-    /// Pass `true` here to verify that the hash actually matches the inputs.
-    pub check_nodeid: bool,
 }
 
 impl UploadHgEntry {
@@ -445,33 +452,47 @@ impl UploadHgEntry {
     pub fn upload(
         self,
         repo: &BlobRepo,
+    ) -> Result<(HgNodeHash, BoxFuture<(HgBlobEntry, RepoPath), Error>)> {
+        self.upload_to_blobstore(&repo.blobstore, &repo.logger)
+    }
+
+    pub(crate) fn upload_to_blobstore(
+        self,
+        blobstore: &Arc<Blobstore>,
+        logger: &Logger,
     ) -> Result<(HgNodeHash, BoxFuture<(HgBlobEntry, RepoPath), Error>)> {
         let UploadHgEntry {
-            nodeid,
+            upload_nodeid,
             raw_content,
             content_type,
             p1,
             p2,
             path,
-            check_nodeid,
         } = self;
 
         let p1 = p1.as_ref();
         let p2 = p2.as_ref();
         let raw_content = raw_content.clean();
 
-        if check_nodeid {
-            let computed_nodeid = HgBlobNode::new(raw_content.clone(), p1, p2)
+        let nodeid: HgNodeHash = match upload_nodeid {
+            UploadHgNodeHash::Generate => HgBlobNode::new(raw_content.clone(), p1, p2)
                 .nodeid()
-                .expect("raw_content must have data available");
-            if nodeid != computed_nodeid {
-                bail_err!(ErrorKind::InconsistentEntryHash(
-                    path,
-                    nodeid,
-                    computed_nodeid
-                ));
+                .expect("raw_content must have data available"),
+            UploadHgNodeHash::Supplied(nodeid) => nodeid,
+            UploadHgNodeHash::Checked(nodeid) => {
+                let computed_nodeid = HgBlobNode::new(raw_content.clone(), p1, p2)
+                    .nodeid()
+                    .expect("raw_content must have data available");
+                if nodeid != computed_nodeid {
+                    bail_err!(ErrorKind::InconsistentEntryHash(
+                        path,
+                        nodeid,
+                        computed_nodeid
+                    ));
+                }
+                nodeid
             }
-        }
+        };
 
         let parents = HgParents::new(p1, p2);
 
@@ -485,7 +506,7 @@ impl UploadHgEntry {
         };
 
         let blob_entry = HgBlobEntry::new(
-            repo.blobstore.clone(),
+            blobstore.clone(),
             path.mpath()
                 .and_then(|m| m.into_iter().last())
                 .map(|m| m.clone()),
@@ -513,10 +534,10 @@ impl UploadHgEntry {
         }
 
         // Ensure that content is in the blobstore
-        let content_upload = repo.blobstore
+        let content_upload = blobstore
             .put(format!("sha1-{}", blob_hash.sha1()), raw_content.into())
             .timed({
-                let logger = repo.logger.clone();
+                let logger = logger.clone();
                 let path = path.clone();
                 let nodeid = nodeid.clone();
                 move |stats, result| {
@@ -527,7 +548,7 @@ impl UploadHgEntry {
                 }
             });
         // Upload the new node
-        let node_upload = repo.blobstore.put(
+        let node_upload = blobstore.put(
             get_node_key(nodeid.into_mononoke()),
             raw_node.serialize(&nodeid)?.into(),
         );
@@ -541,7 +562,7 @@ impl UploadHgEntry {
                     |_| (blob_entry, path)
                 })
                 .timed({
-                    let logger = repo.logger.clone();
+                    let logger = logger.clone();
                     let path = path.clone();
                     let nodeid = nodeid.clone();
                     move |stats, result| {

blobrepo/test/utils.rs

@@ -14,9 +14,10 @@ use futures::future::Future;
 use futures::stream::futures_unordered;
 use futures_ext::{BoxFuture, StreamExt};
 
-use blobrepo::{BlobRepo, ChangesetHandle, CreateChangeset, HgBlobEntry, UploadHgEntry};
+use blobrepo::{BlobRepo, ChangesetHandle, CreateChangeset, HgBlobEntry, UploadHgEntry,
+               UploadHgNodeHash};
 use memblob::{EagerMemblob, LazyMemblob};
-use mercurial::{HgBlobNode, HgNodeHash};
+use mercurial::HgNodeHash;
 use mercurial_types::{manifest, DNodeHash, FileType, HgBlob, RepoPath};
 use mononoke_types::DateTime;
 use std::sync::Arc;
@@ -149,19 +150,14 @@ fn upload_hg_entry(
     p2: Option<HgNodeHash>,
 ) -> (HgNodeHash, BoxFuture<(HgBlobEntry, RepoPath), Error>) {
     let raw_content = HgBlob::from(data);
-    // compute the nodeid from the content
-    let nodeid = HgBlobNode::new(raw_content.clone(), p1.as_ref(), p2.as_ref())
-        .nodeid()
-        .expect("raw_content must have data available");
 
     let upload = UploadHgEntry {
-        nodeid,
+        upload_nodeid: UploadHgNodeHash::Generate,
         raw_content,
         content_type,
         p1,
         p2,
         path,
-        check_nodeid: true,
     };
     upload.upload(repo).unwrap()
 }

bundle2-resolver/src/changegroup/filelog.rs

@@ -16,7 +16,8 @@ use futures_ext::{BoxFuture, BoxStream, FutureExt, StreamExt};
 use heapsize::HeapSizeOf;
 use quickcheck::{Arbitrary, Gen};
 
-use blobrepo::{BlobRepo, ContentBlobInfo, ContentBlobMeta, HgBlobEntry, UploadHgEntry};
+use blobrepo::{BlobRepo, ContentBlobInfo, ContentBlobMeta, HgBlobEntry, UploadHgEntry,
+               UploadHgNodeHash};
 use mercurial::{self, file, HgNodeHash, HgNodeKey};
 use mercurial_bundles::changegroup::CgDeltaChunk;
 use mercurial_types::{delta, manifest, Delta, FileType, HgBlob, MPath, RepoPath};
@@ -51,13 +52,12 @@ impl UploadableHgBlob for Filelog {
     fn upload(self, repo: &BlobRepo) -> Result<(HgNodeKey, Self::Value)> {
         let node_key = self.node_key;
         let upload = UploadHgEntry {
-            nodeid: node_key.hash,
+            upload_nodeid: UploadHgNodeHash::Checked(node_key.hash),
             raw_content: HgBlob::from(self.data),
             content_type: manifest::Type::File(FileType::Regular),
             p1: self.p1,
             p2: self.p2,
             path: node_key.path.clone(),
-            check_nodeid: true,
         };
         upload
             .upload(repo)

bundle2-resolver/src/wirepackparser.rs

@@ -13,7 +13,7 @@ use futures::{Future, Poll, Stream};
 use futures::future::Shared;
 use futures_ext::{BoxFuture, FutureExt};
 
-use blobrepo::{BlobRepo, HgBlobEntry, UploadHgEntry};
+use blobrepo::{BlobRepo, HgBlobEntry, UploadHgEntry, UploadHgNodeHash};
 use mercurial::{self, HgNodeHash, HgNodeKey};
 use mercurial::manifest::ManifestContent;
 use mercurial_bundles::wirepack::{DataEntry, HistoryEntry, Part};
@@ -93,16 +93,20 @@ impl UploadableHgBlob for TreemanifestEntry {
     fn upload(self, repo: &BlobRepo) -> Result<(HgNodeKey, Self::Value)> {
         let node_key = self.node_key;
         let manifest_content = self.manifest_content;
+        // The root tree manifest is expected to have the wrong hash in hybrid mode.
+        // XXX possibly remove this once hybrid mode is gone
+        let upload_nodeid = if node_key.path.is_root() {
+            UploadHgNodeHash::Supplied(node_key.hash)
+        } else {
+            UploadHgNodeHash::Checked(node_key.hash)
+        };
         let upload = UploadHgEntry {
-            nodeid: node_key.hash,
+            upload_nodeid,
             raw_content: HgBlob::from(self.data),
             content_type: manifest::Type::Tree,
             p1: self.p1,
             p2: self.p2,
             path: node_key.path.clone(),
-            // The root tree manifest is expected to have the wrong hash in hybrid mode.
-            // XXX possibly remove this once hybrid mode is gone
-            check_nodeid: !node_key.path.is_root(),
         };
         upload.upload(repo).map(move |(_node, value)| {
             (

cmds/new_blobimport/changeset.rs

@@ -17,7 +17,7 @@ use futures_cpupool::CpuPool;
 use futures_ext::{BoxFuture, BoxStream, FutureExt, StreamExt};
 
 use blobrepo::{BlobChangeset, BlobRepo, ChangesetHandle, CreateChangeset, HgBlobEntry,
-               UploadHgEntry};
+               UploadHgEntry, UploadHgNodeHash};
 use mercurial::{manifest, HgChangesetId, HgManifestId, HgNodeHash, RevlogChangeset, RevlogEntry,
                 RevlogRepo, NULL_HASH};
 use mercurial_types::{HgBlob, MPath, RepoPath, Type};
@@ -170,13 +170,12 @@ fn upload_entry(
         .and_then(move |(content, parents)| {
             let (p1, p2) = parents.get_nodes();
             let upload = UploadHgEntry {
-                nodeid: entry.get_hash().into_nodehash(),
+                upload_nodeid: UploadHgNodeHash::Checked(entry.get_hash().into_nodehash()),
                 raw_content: content,
                 content_type: ty,
                 p1: p1.cloned(),
                 p2: p2.cloned(),
                 path,
-                check_nodeid: true,
             };
             upload.upload(&blobrepo)
         })
@@ -210,16 +209,17 @@ pub fn upload_changesets(
                             None => future::ok(None).boxify(),
                             Some((manifest_id, blob, p1, p2)) => {
                                 let upload = UploadHgEntry {
-                                    nodeid: manifest_id.into_nodehash(),
+                                    // The root tree manifest is expected to have the wrong hash in
+                                    // hybrid mode. This will probably never go away for
+                                    // compatibility with old repositories.
+                                    upload_nodeid: UploadHgNodeHash::Supplied(
+                                        manifest_id.into_nodehash(),
+                                    ),
                                     raw_content: blob,
                                     content_type: Type::Tree,
                                     p1,
                                     p2,
                                     path: RepoPath::root(),
-                                    // The root tree manifest is expected to have the wrong hash in
-                                    // hybrid mode. This will probably never go away for
-                                    // compatibility with old repositories.
-                                    check_nodeid: false,
                                 };
                                 upload
                                     .upload(&blobrepo)