From d07a437ec1092f39a8acbda9de713cb233c3a428 Mon Sep 17 00:00:00 2001 From: Rajiv Sharma Date: Wed, 13 Dec 2023 09:52:00 -0800 Subject: [PATCH] Introduce GitPackfileBaseItem as interim packfile state Summary: Currently, when importing Git commits, trees, tags or blobs, we just store the raw Git object in our data stores. These objects are then used when generating packfiles/bundles for Git protocol or for landing through Git workflow. When Mark was profiling the bundle generation code, he realized that a major bottleneck is the ZLib encoding that we need to perform on-demand during bundle generation. One way to mitigate this problem is to store the encoded data during the initial object upload, which adds a minimal overhead during write (gitimport, remote-gitimport) but would provide a considerable benefit during read (which is the most prominent workflow in Git). This diff in particular introduces the GitPackfileBaseItem type which would represent the encoded data that we can directly use in Packfiles Reviewed By: markbt Differential Revision: D52073664 fbshipit-source-id: 0cbaf1e52ce29b49bc91269a0b8b1dc2dfd24260 --- eden/mononoke/Cargo.toml | 2 + eden/mononoke/git/packfile/Cargo.toml | 5 + eden/mononoke/git/packfile/TARGETS | 6 + eden/mononoke/git/packfile/if/Cargo.toml | 34 ++++ eden/mononoke/git/packfile/if/TARGETS | 13 ++ .../git/packfile/if/packfile_thrift.thrift | 22 +++ eden/mononoke/git/packfile/if/thrift_build.rs | 69 ++++++++ eden/mononoke/git/packfile/if/thrift_lib.rs | 2 + .../mononoke/git/packfile/if/types/Cargo.toml | 32 ++++ .../git/packfile/if/types/thrift_build.rs | 69 ++++++++ .../git/packfile/if/types/thrift_lib.rs | 2 + eden/mononoke/git/packfile/src/lib.rs | 2 + eden/mononoke/git/packfile/src/types.rs | 159 ++++++++++++++++-- .../git/packfile/test/packfile_test.rs | 13 ++ 14 files changed, 416 insertions(+), 14 deletions(-) create mode 100644 eden/mononoke/git/packfile/if/Cargo.toml create mode 100644 eden/mononoke/git/packfile/if/TARGETS create mode 100644 eden/mononoke/git/packfile/if/packfile_thrift.thrift create mode 100644 eden/mononoke/git/packfile/if/thrift_build.rs create mode 100644 eden/mononoke/git/packfile/if/thrift_lib.rs create mode 100644 eden/mononoke/git/packfile/if/types/Cargo.toml create mode 100644 eden/mononoke/git/packfile/if/types/thrift_build.rs create mode 100644 eden/mononoke/git/packfile/if/types/thrift_lib.rs diff --git a/eden/mononoke/Cargo.toml b/eden/mononoke/Cargo.toml index 356f2766608d9..80d03fe76cc75 100644 --- a/eden/mononoke/Cargo.toml +++ b/eden/mononoke/Cargo.toml @@ -288,6 +288,8 @@ members = [ "git/import_direct", "git/import_tools", "git/packfile", + "git/packfile/if", + "git/packfile/if/types", "git/protocol", "git_symbolic_refs", "gotham_ext", diff --git a/eden/mononoke/git/packfile/Cargo.toml b/eden/mononoke/git/packfile/Cargo.toml index 7dda2cf6dfbaf..6c64e96670d63 100644 --- a/eden/mononoke/git/packfile/Cargo.toml +++ b/eden/mononoke/git/packfile/Cargo.toml @@ -13,14 +13,19 @@ path = "test/packfile_test.rs" [dependencies] anyhow = "1.0.75" +blobstore = { version = "0.1.0", path = "../../blobstore" } bytes = { version = "1.1", features = ["serde"] } +fbthrift = { version = "0.0.1+unstable", git = "https://github.com/facebook/fbthrift.git", branch = "main" } flate2 = { version = "1.0.26", features = ["rust_backend"], default-features = false } futures = { version = "0.3.28", features = ["async-await", "compat"] } gix-features = { version = "0.32", features = ["parallel", "rustsha1"] } gix-hash = "0.11" gix-object = "0.33" gix-pack = "0.40" +mononoke_types = { version = "0.1.0", path = "../../mononoke_types" } +packfile_thrift = { version = "0.1.0", path = "if" } pin-project = "0.4.30" +quickcheck = "1.0" sha1 = "0.10.5" thiserror = "1.0.49" tokio = { version = "1.29.1", features = ["full", "test-util", "tracing"] } diff --git a/eden/mononoke/git/packfile/TARGETS b/eden/mononoke/git/packfile/TARGETS index 84368a207ba22..d3defaa04eded 100644 --- a/eden/mononoke/git/packfile/TARGETS +++ b/eden/mononoke/git/packfile/TARGETS @@ -17,9 +17,14 @@ rust_library( "fbsource//third-party/rust:gix-object", "fbsource//third-party/rust:gix-pack", "fbsource//third-party/rust:pin-project", + "fbsource//third-party/rust:quickcheck", "fbsource//third-party/rust:sha1", "fbsource//third-party/rust:thiserror", "fbsource//third-party/rust:tokio", + "//eden/mononoke/blobstore:blobstore", + "//eden/mononoke/git/packfile/if:packfile-thrift-rust", + "//eden/mononoke/mononoke_types:mononoke_types", + "//thrift/lib/rust:fbthrift", ], ) @@ -36,6 +41,7 @@ rust_unittest( "fbsource//third-party/rust:gix-hash", "fbsource//third-party/rust:gix-object", "fbsource//third-party/rust:gix-pack", + "fbsource//third-party/rust:quickcheck", "fbsource//third-party/rust:tempfile", ":packfile", "//common/rust/shed/fbinit:fbinit", diff --git a/eden/mononoke/git/packfile/if/Cargo.toml b/eden/mononoke/git/packfile/if/Cargo.toml new file mode 100644 index 0000000000000..39e47a93f5846 --- /dev/null +++ b/eden/mononoke/git/packfile/if/Cargo.toml @@ -0,0 +1,34 @@ +# @generated by autocargo from //eden/mononoke/git/packfile/if:packfile-thrift-rust + +[package] +name = "packfile_thrift" +version = "0.1.0" +authors = ["Facebook"] +edition = "2021" +license = "GPLv2+" +build = "thrift_build.rs" + +[lib] +path = "thrift_lib.rs" +test = false +doctest = false + +[dependencies] +anyhow = "1.0.75" +async-trait = "0.1.71" +codegen_includer_proc_macro = { version = "0.1.0", git = "https://github.com/facebookexperimental/rust-shed.git", branch = "main" } +const-cstr = "0.3.0" +fbthrift = { version = "0.0.1+unstable", git = "https://github.com/facebook/fbthrift.git", branch = "main" } +futures = { version = "0.3.28", features = ["async-await", "compat"] } +packfile_thrift__types = { package = "packfile_thrift_types", version = "0.1.0", path = "types" } +ref-cast = "1.0.18" +thiserror = "1.0.49" +tracing = "0.1.40" +tracing-futures = { version = "0.2.5", features = ["futures-03"] } + +[build-dependencies] +thrift_compiler = { version = "0.1.0", git = "https://github.com/facebookexperimental/rust-shed.git", branch = "main" } + +[features] +default = ["thrift_library_unittests_disabled"] +thrift_library_unittests_disabled = [] diff --git a/eden/mononoke/git/packfile/if/TARGETS b/eden/mononoke/git/packfile/if/TARGETS new file mode 100644 index 0000000000000..e77623c7b5507 --- /dev/null +++ b/eden/mononoke/git/packfile/if/TARGETS @@ -0,0 +1,13 @@ +load("@fbcode_macros//build_defs:thrift_library.bzl", "thrift_library") + +oncall("mononoke") + +thrift_library( + name = "packfile-thrift", + languages = [ + "rust", + "cpp2", + ], + thrift_rust_options = ["deprecated_default_enum_min_i32"], + thrift_srcs = {"packfile_thrift.thrift": []}, +) diff --git a/eden/mononoke/git/packfile/if/packfile_thrift.thrift b/eden/mononoke/git/packfile/if/packfile_thrift.thrift new file mode 100644 index 0000000000000..03271c3253660 --- /dev/null +++ b/eden/mononoke/git/packfile/if/packfile_thrift.thrift @@ -0,0 +1,22 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This software may be used and distributed according to the terms of the + * GNU General Public License version 2. + */ + +/// Struct representing the raw packfile item for base objects in Git +struct GitPackfileBaseItem { + 1: binary id; + 2: i64 decompressed_size; + 3: binary compressed_data; + 4: GitObjectKind kind; +} (rust.exhaustive) + +/// Enum determining the type of Git base object +enum GitObjectKind { + Tree = 0, + Blob = 1, + Commit = 2, + Tag = 3, +} (rust.exhaustive) diff --git a/eden/mononoke/git/packfile/if/thrift_build.rs b/eden/mononoke/git/packfile/if/thrift_build.rs new file mode 100644 index 0000000000000..9b8b3286ab897 --- /dev/null +++ b/eden/mononoke/git/packfile/if/thrift_build.rs @@ -0,0 +1,69 @@ +// @generated by autocargo +use std::env; +use std::fs; +use std::path::Path; + +use thrift_compiler::Config; +use thrift_compiler::GenContext; + +#[rustfmt::skip] +fn main() { + // Rerun if this gets rewritten. + println!("cargo:rerun-if-changed=thrift_build.rs"); + + let out_dir = env::var_os("OUT_DIR").expect("OUT_DIR env not provided"); + let out_dir: &Path = out_dir.as_ref(); + fs::write( + out_dir.join("cratemap"), + "packfile_thrift crate", + ).expect("Failed to write cratemap"); + + let conf = { + let mut conf = Config::from_env(GenContext::Lib).expect("Failed to instantiate thrift_compiler::Config"); + + let path_from_manifest_to_base: &Path = "../../../../..".as_ref(); + let cargo_manifest_dir = + env::var_os("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not provided"); + let cargo_manifest_dir: &Path = cargo_manifest_dir.as_ref(); + let base_path = cargo_manifest_dir + .join(path_from_manifest_to_base) + .canonicalize() + .expect("Failed to canonicalize base_path"); + // TODO: replace canonicalize() with std::path::absolute() when + // https://github.com/rust-lang/rust/pull/91673 is available (~Rust 1.60) + // and remove this block. + #[cfg(windows)] + let base_path = Path::new( + base_path + .as_path() + .to_string_lossy() + .trim_start_matches(r"\\?\"), + ) + .to_path_buf(); + + conf.base_path(base_path); + + conf.types_crate("packfile-thrift__types"); + + let options = "deprecated_default_enum_min_i32"; + if !options.is_empty() { + conf.options(options); + } + + let lib_include_srcs = vec![ + + ]; + let types_include_srcs = vec![ + + ]; + conf.lib_include_srcs(lib_include_srcs); + conf.types_include_srcs(types_include_srcs); + + conf + }; + + let srcs: &[&str] = &[ + "packfile_thrift.thrift" + ]; + conf.run(srcs).expect("Failed while running thrift compilation"); +} diff --git a/eden/mononoke/git/packfile/if/thrift_lib.rs b/eden/mononoke/git/packfile/if/thrift_lib.rs new file mode 100644 index 0000000000000..88a7cd8a77efb --- /dev/null +++ b/eden/mononoke/git/packfile/if/thrift_lib.rs @@ -0,0 +1,2 @@ +// @generated by autocargo +::codegen_includer_proc_macro::include!(); diff --git a/eden/mononoke/git/packfile/if/types/Cargo.toml b/eden/mononoke/git/packfile/if/types/Cargo.toml new file mode 100644 index 0000000000000..90048f9a287e4 --- /dev/null +++ b/eden/mononoke/git/packfile/if/types/Cargo.toml @@ -0,0 +1,32 @@ +# @generated by autocargo from //eden/mononoke/git/packfile/if:packfile-thrift-rust-types + +[package] +name = "packfile_thrift_types" +version = "0.1.0" +authors = ["Facebook"] +edition = "2021" +license = "GPLv2+" +build = "thrift_build.rs" + +[lib] +path = "thrift_lib.rs" +test = false +doctest = false + +[dependencies] +anyhow = "1.0.75" +codegen_includer_proc_macro = { version = "0.1.0", git = "https://github.com/facebookexperimental/rust-shed.git", branch = "main" } +fbthrift = { version = "0.0.1+unstable", git = "https://github.com/facebook/fbthrift.git", branch = "main" } +futures = { version = "0.3.28", features = ["async-await", "compat"] } +once_cell = "1.12" +ref-cast = "1.0.18" +serde = { version = "1.0.185", features = ["derive", "rc"] } +serde_derive = "1.0.185" +thiserror = "1.0.49" + +[build-dependencies] +thrift_compiler = { version = "0.1.0", git = "https://github.com/facebookexperimental/rust-shed.git", branch = "main" } + +[features] +default = ["thrift_library_unittests_disabled"] +thrift_library_unittests_disabled = [] diff --git a/eden/mononoke/git/packfile/if/types/thrift_build.rs b/eden/mononoke/git/packfile/if/types/thrift_build.rs new file mode 100644 index 0000000000000..1f48062b821b7 --- /dev/null +++ b/eden/mononoke/git/packfile/if/types/thrift_build.rs @@ -0,0 +1,69 @@ +// @generated by autocargo +use std::env; +use std::fs; +use std::path::Path; + +use thrift_compiler::Config; +use thrift_compiler::GenContext; + +#[rustfmt::skip] +fn main() { + // Rerun if this gets rewritten. + println!("cargo:rerun-if-changed=thrift_build.rs"); + + let out_dir = env::var_os("OUT_DIR").expect("OUT_DIR env not provided"); + let out_dir: &Path = out_dir.as_ref(); + fs::write( + out_dir.join("cratemap"), + "packfile_thrift crate", + ).expect("Failed to write cratemap"); + + let conf = { + let mut conf = Config::from_env(GenContext::Types).expect("Failed to instantiate thrift_compiler::Config"); + + let path_from_manifest_to_base: &Path = "../../../../../..".as_ref(); + let cargo_manifest_dir = + env::var_os("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not provided"); + let cargo_manifest_dir: &Path = cargo_manifest_dir.as_ref(); + let base_path = cargo_manifest_dir + .join(path_from_manifest_to_base) + .canonicalize() + .expect("Failed to canonicalize base_path"); + // TODO: replace canonicalize() with std::path::absolute() when + // https://github.com/rust-lang/rust/pull/91673 is available (~Rust 1.60) + // and remove this block. + #[cfg(windows)] + let base_path = Path::new( + base_path + .as_path() + .to_string_lossy() + .trim_start_matches(r"\\?\"), + ) + .to_path_buf(); + + conf.base_path(base_path); + + conf.types_crate("packfile-thrift__types"); + + let options = "deprecated_default_enum_min_i32"; + if !options.is_empty() { + conf.options(options); + } + + let lib_include_srcs = vec![ + + ]; + let types_include_srcs = vec![ + + ]; + conf.lib_include_srcs(lib_include_srcs); + conf.types_include_srcs(types_include_srcs); + + conf + }; + + let srcs: &[&str] = &[ + "../packfile_thrift.thrift" + ]; + conf.run(srcs).expect("Failed while running thrift compilation"); +} diff --git a/eden/mononoke/git/packfile/if/types/thrift_lib.rs b/eden/mononoke/git/packfile/if/types/thrift_lib.rs new file mode 100644 index 0000000000000..88a7cd8a77efb --- /dev/null +++ b/eden/mononoke/git/packfile/if/types/thrift_lib.rs @@ -0,0 +1,2 @@ +// @generated by autocargo +::codegen_includer_proc_macro::include!(); diff --git a/eden/mononoke/git/packfile/src/lib.rs b/eden/mononoke/git/packfile/src/lib.rs index 99bcf2d1ab1d3..de0d88499ba21 100644 --- a/eden/mononoke/git/packfile/src/lib.rs +++ b/eden/mononoke/git/packfile/src/lib.rs @@ -9,3 +9,5 @@ pub mod bundle; mod hash_writer; pub mod pack; pub mod types; + +pub use packfile_thrift as thrift; diff --git a/eden/mononoke/git/packfile/src/types.rs b/eden/mononoke/git/packfile/src/types.rs index 9e3822d503943..2288974328353 100644 --- a/eden/mononoke/git/packfile/src/types.rs +++ b/eden/mononoke/git/packfile/src/types.rs @@ -8,8 +8,11 @@ use std::io::Write; use anyhow::Context; +use anyhow::Result; +use blobstore::BlobstoreBytes; use bytes::Bytes; use bytes::BytesMut; +use fbthrift::compact_protocol; use flate2::write::ZlibEncoder; use flate2::Compression; use gix_hash::oid; @@ -18,9 +21,13 @@ use gix_object::Object; use gix_object::ObjectRef; use gix_object::WriteTo; use gix_pack::data::output; +use mononoke_types::private::MononokeTypeError; +use quickcheck::Arbitrary; use sha1::Digest; use sha1::Sha1; +use crate::thrift; + /// The type of items that can be present in a Git packfile. Does not include RefDelta currently /// since we do not use it /// See: https://fburl.com/1yaui1um @@ -31,7 +38,7 @@ pub enum PackfileItem { } impl PackfileItem { - pub fn new_base(object_bytes: Bytes) -> anyhow::Result { + pub fn new_base(object_bytes: Bytes) -> Result { BaseObject::new(object_bytes).map(Self::Base) } @@ -53,7 +60,7 @@ impl PackfileItem { impl TryFrom for output::Entry { type Error = anyhow::Error; - fn try_from(value: PackfileItem) -> Result { + fn try_from(value: PackfileItem) -> Result { match value { PackfileItem::Base(base) => base.try_into(), PackfileItem::OidDelta(oid_delta) => oid_delta.try_into(), @@ -107,7 +114,7 @@ impl DeltaOidObject { impl TryFrom for output::Entry { type Error = anyhow::Error; - fn try_from(value: DeltaOidObject) -> Result { + fn try_from(value: DeltaOidObject) -> Result { let kind = value.kind(); let entry = Self { id: value.oid, @@ -115,7 +122,7 @@ impl TryFrom for output::Entry { compressed_data: value.compressed_data.to_vec(), kind, }; - Ok(entry) + anyhow::Ok(entry) } } @@ -128,7 +135,7 @@ pub struct BaseObject { impl BaseObject { /// Creates a new packfile item from the raw object bytes of the Git object. - pub fn new(object_bytes: Bytes) -> anyhow::Result { + pub fn new(object_bytes: Bytes) -> Result { // Get the hash of the Git object bytes let mut hasher = Sha1::new(); hasher.update(&object_bytes); @@ -141,7 +148,7 @@ impl BaseObject { .context("Failed to convert packfile item hash to Git Object ID")? .into(); // Create the packfile item from the object and the hash - Ok(Self { object, hash }) + anyhow::Ok(Self { object, hash }) } /// The kind of the packfile item. @@ -161,7 +168,7 @@ impl BaseObject { } /// Zlib encode the raw bytes of the Git object and write it to `out`. - pub fn write_encoded(&self, out: &mut BytesMut, include_header: bool) -> anyhow::Result<()> { + pub fn write_encoded(&self, out: &mut BytesMut, include_header: bool) -> Result<()> { let object_bytes = match include_header { true => to_vec_bytes(&self.object)?, false => to_vec_bytes_without_header(&self.object)?, @@ -174,14 +181,14 @@ impl BaseObject { .finish() .context("Failure in ZLib encoding Git object data")?; out.extend(&compressed_object); - Ok(()) + anyhow::Ok(()) } } impl TryFrom for output::Entry { type Error = anyhow::Error; - fn try_from(value: BaseObject) -> Result { + fn try_from(value: BaseObject) -> Result { let id = value.hash().into(); let decompressed_size = value.size(); let kind = value.kind(); @@ -196,22 +203,146 @@ impl TryFrom for output::Entry { decompressed_size, compressed_data, }; - Ok(entry) + anyhow::Ok(entry) + } +} + +impl TryFrom for GitPackfileBaseItem { + type Error = anyhow::Error; + + fn try_from(value: BaseObject) -> Result { + let kind = match value.kind() { + output::entry::Kind::Base(kind) => kind, + _ => anyhow::bail!( + "Cannot convert non-base output entry object into GitPackfileBaseItem" + ), + }; + let output_entry: output::Entry = value.try_into()?; + anyhow::Ok(Self { + id: output_entry.id, + decompressed_size: output_entry.decompressed_size, + compressed_data: output_entry.compressed_data, + kind, + }) + } +} + +/// Struct representing the raw packfile item for base objects in Git +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct GitPackfileBaseItem { + id: ObjectId, + decompressed_size: usize, + kind: gix_object::Kind, + compressed_data: Vec, +} + +impl TryFrom for GitPackfileBaseItem { + type Error = anyhow::Error; + + fn try_from(t: thrift::GitPackfileBaseItem) -> Result { + let id = oid::try_from_bytes(&t.id)?.to_owned(); + let decompressed_size = t.decompressed_size as usize; + let kind = match t.kind { + thrift::GitObjectKind::Blob => gix_object::Kind::Blob, + thrift::GitObjectKind::Tree => gix_object::Kind::Tree, + thrift::GitObjectKind::Commit => gix_object::Kind::Commit, + thrift::GitObjectKind::Tag => gix_object::Kind::Tag, + thrift::GitObjectKind(x) => anyhow::bail!("Unsupported object kind: {}", x), + }; + anyhow::Ok(Self { + id, + decompressed_size, + kind, + compressed_data: t.compressed_data, + }) + } +} + +impl From for thrift::GitPackfileBaseItem { + fn from(packfile_item: GitPackfileBaseItem) -> thrift::GitPackfileBaseItem { + let id = packfile_item.id.as_ref().as_bytes().to_vec(); + let decompressed_size = packfile_item.decompressed_size as i64; + let kind = match packfile_item.kind { + gix_object::Kind::Blob => thrift::GitObjectKind::Blob, + gix_object::Kind::Tree => thrift::GitObjectKind::Tree, + gix_object::Kind::Commit => thrift::GitObjectKind::Commit, + gix_object::Kind::Tag => thrift::GitObjectKind::Tag, + }; + thrift::GitPackfileBaseItem { + id, + decompressed_size, + kind, + compressed_data: packfile_item.compressed_data, + } + } +} + +impl TryFrom for output::Entry { + type Error = anyhow::Error; + + fn try_from(value: GitPackfileBaseItem) -> Result { + let entry = Self { + id: value.id, + kind: output::entry::Kind::Base(value.kind), + decompressed_size: value.decompressed_size, + compressed_data: value.compressed_data, + }; + anyhow::Ok(entry) + } +} + +impl GitPackfileBaseItem { + pub fn from_encoded_bytes(encoded_bytes: Bytes) -> Result { + let thrift_item: thrift::GitPackfileBaseItem = compact_protocol::deserialize(encoded_bytes) + .with_context(|| { + MononokeTypeError::BlobDeserializeError("GitPackfileBaseItem".into()) + })?; + thrift_item.try_into() + } + + pub fn into_blobstore_bytes(self) -> BlobstoreBytes { + let thrift_item: thrift::GitPackfileBaseItem = self.into(); + BlobstoreBytes::from_bytes(compact_protocol::serialize(thrift_item)) + } +} + +impl Arbitrary for GitPackfileBaseItem { + fn arbitrary(g: &mut quickcheck::Gen) -> Self { + let compressed_data: Vec = Vec::arbitrary(g); + let id = oid::try_from_bytes(mononoke_types::hash::Sha1::arbitrary(g).as_ref()) + .unwrap() + .into(); + let decompressed_size = usize::arbitrary(g) / 2; + let kind = g + .choose(&[ + gix_object::Kind::Blob, + gix_object::Kind::Tree, + gix_object::Kind::Commit, + gix_object::Kind::Tag, + ]) + .unwrap() + .clone(); + Self { + id, + decompressed_size, + kind, + compressed_data, + } } } /// Free function responsible for writing only the Git object data to a Vec /// without including the loose format headers -pub(crate) fn to_vec_bytes_without_header(git_object: &Object) -> anyhow::Result> { +pub(crate) fn to_vec_bytes_without_header(git_object: &Object) -> Result> { let mut object_bytes = Vec::new(); git_object.write_to(object_bytes.by_ref())?; - Ok(object_bytes) + anyhow::Ok(object_bytes) } /// Free function responsible for writing Git object data to a Vec /// in loose format -pub fn to_vec_bytes(git_object: &Object) -> anyhow::Result> { +pub fn to_vec_bytes(git_object: &Object) -> Result> { let mut object_bytes = git_object.loose_header().into_vec(); git_object.write_to(object_bytes.by_ref())?; - Ok(object_bytes) + anyhow::Ok(object_bytes) } diff --git a/eden/mononoke/git/packfile/test/packfile_test.rs b/eden/mononoke/git/packfile/test/packfile_test.rs index 9f30f8234162f..3b29ce5eeb52f 100644 --- a/eden/mononoke/git/packfile/test/packfile_test.rs +++ b/eden/mononoke/git/packfile/test/packfile_test.rs @@ -23,9 +23,12 @@ use gix_object::Tag; use packfile::bundle::BundleWriter; use packfile::pack::DeltaForm; use packfile::pack::PackfileWriter; +use packfile::thrift; use packfile::types::to_vec_bytes; use packfile::types::BaseObject; +use packfile::types::GitPackfileBaseItem; use packfile::types::PackfileItem; +use quickcheck::quickcheck; use tempfile::NamedTempFile; async fn get_objects_stream( @@ -434,3 +437,13 @@ async fn validate_staggered_bundle_generation() -> anyhow::Result<()> { .expect("Expected successful finish of bundle creation"); Ok(()) } + +quickcheck! { + fn git_packfile_base_item_thrift_roundtrip(entry: GitPackfileBaseItem) -> bool { + let thrift_entry: thrift::GitPackfileBaseItem = entry.clone().into(); + let from_thrift_entry: GitPackfileBaseItem = thrift_entry.try_into().expect("thrift roundtrips should always be valid"); + println!("entry: {:?}", entry); + println!("entry_from_thrift: {:?}", from_thrift_entry); + entry == from_thrift_entry + } +}