From 83989438de3d0b62b2b60462fa8b464d53006197 Mon Sep 17 00:00:00 2001 From: Rajiv Sharma Date: Fri, 14 Feb 2025 06:15:39 -0800 Subject: [PATCH] Delete everything related to thrift GitTrees Summary: We don't use this as a derived data type anymore so we should be able to delete and get rid of this type completely. Reviewed By: andreacampi Differential Revision: D69467793 fbshipit-source-id: 9816415e810210c5b8bb31f1b2c88128fc7b00f5 --- .../bookmarks/warm_bookmarks_cache/lib.rs | 1 - .../derived_data/bulk_derivation/lib.rs | 2 - eden/mononoke/git/git_types/Cargo.toml | 2 - .../git/git_types/src/delta_manifest_v2.rs | 23 - eden/mononoke/git/git_types/src/errors.rs | 2 - .../git/git_types/src/handles/blob.rs | 111 ----- .../git/git_types/src/handles/derive_tree.rs | 394 ------------------ .../git/git_types/src/handles/tree.rs | 315 -------------- eden/mononoke/git/git_types/src/lib.rs | 8 - eden/mononoke/git/git_types/src/manifest.rs | 65 --- eden/mononoke/git/git_types/src/store.rs | 13 - .../src/gitexport_tools/commit_rewrite.rs | 2 - eden/mononoke/git/gitimport/src/main.rs | 4 +- .../src/test/test_repo_create_changeset.rs | 4 - .../mononoke_types/src/derivable_type.rs | 5 - eden/mononoke/repo_import/src/tests.rs | 3 - .../admin/src/commands/blobstore/fetch.rs | 4 - eden/mononoke/walker/src/detail/graph.rs | 1 - 18 files changed, 1 insertion(+), 958 deletions(-) delete mode 100644 eden/mononoke/git/git_types/src/handles/blob.rs delete mode 100644 eden/mononoke/git/git_types/src/handles/derive_tree.rs delete mode 100644 eden/mononoke/git/git_types/src/handles/tree.rs delete mode 100644 eden/mononoke/git/git_types/src/manifest.rs diff --git a/eden/mononoke/bookmarks/warm_bookmarks_cache/lib.rs b/eden/mononoke/bookmarks/warm_bookmarks_cache/lib.rs index eade75b6852db..8b82bc6d3e91a 100644 --- a/eden/mononoke/bookmarks/warm_bookmarks_cache/lib.rs +++ b/eden/mononoke/bookmarks/warm_bookmarks_cache/lib.rs @@ -318,7 +318,6 @@ impl WarmBookmarksCacheBuilder { )), DerivableType::TestManifests => None, DerivableType::TestShardedManifests => None, - DerivableType::GitTrees => None, } } diff --git a/eden/mononoke/derived_data/bulk_derivation/lib.rs b/eden/mononoke/derived_data/bulk_derivation/lib.rs index 923d3cb4ba8e7..268d6973702d5 100644 --- a/eden/mononoke/derived_data/bulk_derivation/lib.rs +++ b/eden/mononoke/derived_data/bulk_derivation/lib.rs @@ -34,7 +34,6 @@ use futures::StreamExt; use futures::TryStreamExt; use git_types::MappedGitCommitId; use git_types::RootGitDeltaManifestV2Id; -use git_types::TreeHandle; use mercurial_derivation::MappedHgChangesetId; use mercurial_derivation::RootHgAugmentedManifestId; use mononoke_macros::mononoke; @@ -328,7 +327,6 @@ fn manager_for_type( Arc::new(SingleTypeManager::::new(manager)) } DerivableType::ChangesetInfo => Arc::new(SingleTypeManager::::new(manager)), - DerivableType::GitTrees => Arc::new(SingleTypeManager::::new(manager)), DerivableType::GitCommits => Arc::new(SingleTypeManager::::new(manager)), DerivableType::GitDeltaManifestsV2 => { Arc::new(SingleTypeManager::::new(manager)) diff --git a/eden/mononoke/git/git_types/Cargo.toml b/eden/mononoke/git/git_types/Cargo.toml index 296a99536d4c8..b8a5f77e2837a 100644 --- a/eden/mononoke/git/git_types/Cargo.toml +++ b/eden/mononoke/git/git_types/Cargo.toml @@ -54,11 +54,9 @@ fbinit = { version = "0.2.0", git = "https://github.com/facebookexperimental/rus fbinit-tokio = { version = "0.1.2", git = "https://github.com/facebookexperimental/rust-shed.git", branch = "main" } fixtures = { version = "0.1.0", path = "../../tests/fixtures" } futures-util = "0.3.30" -git2 = "0.20" indoc = "2.0.2" maplit = "1.0" memblob = { version = "0.1.0", path = "../../blobstore/memblob" } rand_distr = "0.4" repo_blobstore = { version = "0.1.0", path = "../../blobrepo/repo_blobstore" } repo_identity = { version = "0.1.0", path = "../../repo_attributes/repo_identity" } -tempfile = "3.15" diff --git a/eden/mononoke/git/git_types/src/delta_manifest_v2.rs b/eden/mononoke/git/git_types/src/delta_manifest_v2.rs index d168858e4b342..af2bb7aa011b0 100644 --- a/eden/mononoke/git/git_types/src/delta_manifest_v2.rs +++ b/eden/mononoke/git/git_types/src/delta_manifest_v2.rs @@ -45,7 +45,6 @@ use mononoke_types::ThriftConvert; use crate::thrift; use crate::GitLeaf; use crate::GitTreeId; -use crate::TreeMember; /// A manifest that contains an entry for each Git object that was added or modified as part of /// a commit. The object needs to be different from all objects at the same path in all parents @@ -152,28 +151,6 @@ pub struct GDMV2ObjectEntry { } impl GDMV2ObjectEntry { - pub fn from_tree_member(member: &TreeMember, inlined_bytes: Option) -> Result { - let oid = ObjectId::from_hex(member.oid().to_hex().as_bytes()).with_context(|| { - format!( - "Error while converting hash {:?} to ObjectId", - member.oid().to_hex() - ) - })?; - let size = member.oid().size(); - let kind = match member.kind() { - crate::ObjectKind::Blob => crate::DeltaObjectKind::Blob, - crate::ObjectKind::Tree => crate::DeltaObjectKind::Tree, - kind => anyhow::bail!("Unexpected object kind {:?} for DeltaObjectEntry", kind), - }; - - Ok(GDMV2ObjectEntry { - oid, - size, - kind, - inlined_bytes, - }) - } - pub async fn from_tree_entry( ctx: &CoreContext, blobstore: &impl Blobstore, diff --git a/eden/mononoke/git/git_types/src/errors.rs b/eden/mononoke/git/git_types/src/errors.rs index 7062e2cf8f19c..f0b5ac78c6ca9 100644 --- a/eden/mononoke/git/git_types/src/errors.rs +++ b/eden/mononoke/git/git_types/src/errors.rs @@ -15,8 +15,6 @@ pub enum MononokeGitError { ContentMissing(FetchKey), #[error("Tree Derivation Failed")] TreeDerivationFailed, - #[error("Invalid Thrift")] - InvalidThrift, } #[derive(Clone, Debug, Error)] diff --git a/eden/mononoke/git/git_types/src/handles/blob.rs b/eden/mononoke/git/git_types/src/handles/blob.rs deleted file mode 100644 index 26947788761c4..0000000000000 --- a/eden/mononoke/git/git_types/src/handles/blob.rs +++ /dev/null @@ -1,111 +0,0 @@ -/* - * 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. - */ - -use anyhow::Error; -use blobstore::Blobstore; -use context::CoreContext; -use filestore::FetchKey; -use mononoke_types::hash::RichGitSha1; -use mononoke_types::BasicFileChange; -use mononoke_types::ContentId; -use mononoke_types::FileType; - -use crate::errors::MononokeGitError; -use crate::mode; -use crate::thrift; -use crate::ObjectKind; - -#[derive(Debug, Copy, Clone, Hash, Eq, PartialEq)] -pub struct BlobHandle { - oid: RichGitSha1, - file_type: FileType, -} - -impl BlobHandle { - pub async fn new( - ctx: &CoreContext, - blobstore: &B, - file_change: &BasicFileChange, - ) -> Result { - let file_type = file_change.file_type(); - - let oid = if file_type == FileType::GitSubmodule { - let bytes = - filestore::fetch_concat_exact(blobstore, ctx, file_change.content_id(), 20).await?; - RichGitSha1::from_bytes(&bytes, ObjectKind::Commit.as_str(), 0)? - } else { - let key = FetchKey::Canonical(file_change.content_id()); - let metadata = filestore::get_metadata(blobstore, ctx, &key) - .await? - .ok_or(MononokeGitError::ContentMissing(key))?; - metadata.git_sha1 - }; - - Ok(Self { oid, file_type }) - } - - pub fn from_oid_and_file_type(oid: RichGitSha1, file_type: FileType) -> Self { - Self { oid, file_type } - } - - pub async fn from_content_id_and_file_type( - ctx: &CoreContext, - blobstore: &B, - content_id: ContentId, - file_type: FileType, - ) -> Result { - let key = FetchKey::Canonical(content_id); - let metadata = filestore::get_metadata(blobstore, ctx, &key) - .await? - .ok_or(MononokeGitError::ContentMissing(key))?; - Ok(Self { - oid: metadata.git_sha1, - file_type, - }) - } - - pub fn filemode(&self) -> i32 { - match self.file_type { - FileType::Regular => mode::GIT_FILEMODE_BLOB, - FileType::Executable => mode::GIT_FILEMODE_BLOB_EXECUTABLE, - FileType::Symlink => mode::GIT_FILEMODE_LINK, - FileType::GitSubmodule => mode::GIT_FILEMODE_COMMIT, - } - } - - pub fn oid(&self) -> &RichGitSha1 { - &self.oid - } -} - -impl TryFrom for BlobHandle { - type Error = Error; - - fn try_from(t: thrift::BlobHandle) -> Result { - let size = t.size.try_into()?; - let oid = RichGitSha1::from_bytes(&t.oid.0, ObjectKind::Blob.as_str(), size)?; - - Ok(Self { - oid, - file_type: FileType::from_thrift(t.file_type)?, - }) - } -} - -impl From for thrift::BlobHandle { - fn from(bh: BlobHandle) -> thrift::BlobHandle { - let size = bh.oid.size(); - - thrift::BlobHandle { - oid: bh.oid.into_thrift(), - size: size - .try_into() - .expect("Blob size must fit in a i64 for Thrift serialization"), - file_type: bh.file_type.into_thrift(), - } - } -} diff --git a/eden/mononoke/git/git_types/src/handles/derive_tree.rs b/eden/mononoke/git/git_types/src/handles/derive_tree.rs deleted file mode 100644 index a655e43f38aef..0000000000000 --- a/eden/mononoke/git/git_types/src/handles/derive_tree.rs +++ /dev/null @@ -1,394 +0,0 @@ -/* - * 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. - */ - -use anyhow::anyhow; -use anyhow::bail; -use anyhow::Context; -use anyhow::Error; -use anyhow::Result; -use async_trait::async_trait; -use blobstore::Blobstore; -use blobstore::Storable; -use cloned::cloned; -use context::CoreContext; -use derived_data_manager::dependencies; -use derived_data_manager::BonsaiDerivable; -use derived_data_manager::DerivableType; -use derived_data_manager::DerivationContext; -use derived_data_service_if as thrift; -use filestore::FilestoreConfig; -use futures::future::ready; -use futures::stream::FuturesUnordered; -use futures::stream::TryStreamExt; -use manifest::derive_manifest; -use manifest::flatten_subentries; -use mononoke_types::BonsaiChangeset; -use mononoke_types::ChangesetId; -use mononoke_types::GitLfs; -use mononoke_types::NonRootMPath; -#[allow(unused_imports)] -use tokio::runtime::Runtime; - -use crate::errors::MononokeGitError; -use crate::fetch_non_blob_git_object; -use crate::git_lfs::generate_and_store_git_lfs_pointer; -use crate::upload_non_blob_git_object; -use crate::BlobHandle; -use crate::Tree; -use crate::TreeBuilder; -use crate::TreeHandle; - -fn format_key(derivation_ctx: &DerivationContext, changeset_id: ChangesetId) -> String { - let root_prefix = "git.derived_root."; - let key_prefix = derivation_ctx.mapping_key_prefix::(); - format!("{}{}{}", root_prefix, key_prefix, changeset_id) -} - -#[async_trait] -impl BonsaiDerivable for TreeHandle { - const VARIANT: DerivableType = DerivableType::GitTrees; - - type Dependencies = dependencies![]; - type PredecessorDependencies = dependencies![]; - - async fn derive_single( - ctx: &CoreContext, - derivation_ctx: &DerivationContext, - bonsai: BonsaiChangeset, - parents: Vec, - ) -> Result { - if bonsai.is_snapshot() { - bail!("Can't derive TreeHandle for snapshot") - } - let blobstore = derivation_ctx.blobstore().clone(); - let filestore_config = derivation_ctx.filestore_config(); - let cs_id = bonsai.get_changeset_id(); - let changes = get_file_changes(&blobstore, filestore_config, ctx, bonsai).await?; - // Check whether the git commit for this bonsai commit is already known. - // If so, then the raw git tree will also exist, as it would have been uploaded - // alongside the commit. If not, then the raw tree git may not already exist and - // we should derive it. - let derive_raw_tree = derivation_ctx - .bonsai_git_mapping()? - .get_git_sha1_from_bonsai(ctx, cs_id) - .await - .with_context(|| format!("Error in getting Git Sha1 for Bonsai Changeset {}", cs_id))? - .is_none(); - derive_git_manifest(ctx, blobstore, parents, changes, derive_raw_tree).await - } - - async fn store_mapping( - self, - ctx: &CoreContext, - derivation_ctx: &DerivationContext, - changeset_id: ChangesetId, - ) -> Result<()> { - let key = format_key(derivation_ctx, changeset_id); - derivation_ctx.blobstore().put(ctx, key, self.into()).await - } - - async fn fetch( - ctx: &CoreContext, - derivation_ctx: &DerivationContext, - changeset_id: ChangesetId, - ) -> Result> { - let key = format_key(derivation_ctx, changeset_id); - Ok(derivation_ctx - .blobstore() - .get(ctx, &key) - .await? - .map(TryInto::try_into) - .transpose()?) - } - - fn from_thrift(data: thrift::DerivedData) -> Result { - if let thrift::DerivedData::tree_handle(thrift::DerivedDataTreeHandle::tree_handle(id)) = - data - { - Self::try_from(id) - } else { - Err(anyhow!( - "Can't convert {} from provided thrift::DerivedData", - Self::NAME.to_string(), - )) - } - } - - fn into_thrift(data: Self) -> Result { - Ok(thrift::DerivedData::tree_handle( - thrift::DerivedDataTreeHandle::tree_handle(data.into()), - )) - } -} - -async fn derive_git_manifest( - ctx: &CoreContext, - blobstore: B, - parents: Vec, - changes: Vec<(NonRootMPath, Option)>, - derive_raw_tree: bool, -) -> Result { - let handle = derive_manifest( - ctx.clone(), - blobstore.clone(), - parents, - changes, - { - cloned!(ctx, blobstore); - move |tree_info| { - cloned!(ctx, blobstore); - async move { - let members = flatten_subentries(&ctx, &(), tree_info.subentries) - .await? - .map(|(p, (_, entry))| (p, entry.into())) - .collect(); - - let builder = TreeBuilder::new(members); - let (mut tree_bytes_without_header, tree) = builder.into_tree_with_bytes(); - let oid = tree.handle().oid(); - let git_hash = oid.to_object_id()?; - if derive_raw_tree { - // Store the raw git tree before storing the thrift version - // Need to prepend the object header before storing the Git tree - let mut raw_tree_bytes = oid.prefix(); - raw_tree_bytes.append(&mut tree_bytes_without_header); - upload_non_blob_git_object( - &ctx, - &blobstore, - git_hash.as_ref(), - raw_tree_bytes, - ) - .await?; - } else { - // We don't need to store the raw git tree because it already exists. Validate that the existing tree - // is present in the blobstore with the same hash as we computed. If not, then it means that we computed - // a thirft Git tree that is different than the stored raw tree. This could be due to a bug so we need to - // fail before storing the thrift tree - fetch_non_blob_git_object(&ctx, &blobstore, git_hash.as_ref()) - .await - .with_context(|| { - format!( - "Raw Git tree with hash {} should have been present already", - git_hash.to_hex() - ) - })?; - } - // Upload the thrift Git Tree - let handle = tree.store(&ctx, &blobstore).await?; - Ok(((), handle)) - } - } - }, - { - // NOTE: A None leaf will happen in derive_manifest if the parents have conflicting - // leaves. However, since we're deriving from a Bonsai changeset and using our Git Tree - // manifest which has leaves that are equivalent derived to their Bonsai - // representation, that won't happen. - |leaf_info| { - let leaf = leaf_info - .change - .ok_or_else(|| MononokeGitError::TreeDerivationFailed.into()) - .map(|l| ((), l)); - ready(leaf) - } - }, - ) - .await?; - - match handle { - Some(handle) => Ok(handle), - None => { - let tree: Tree = TreeBuilder::default().into(); - tree.store(ctx, &blobstore).await - } - } -} - -pub async fn get_file_changes( - blobstore: &B, - filestore_config: FilestoreConfig, - ctx: &CoreContext, - bcs: BonsaiChangeset, -) -> Result)>, Error> { - bcs.into_mut() - .file_changes - .into_iter() - .map(|(mpath, file_change)| async move { - match file_change.simplify() { - Some(basic_file_change) => match basic_file_change.git_lfs() { - // File is not LFS file - GitLfs::FullContent => Ok(( - mpath, - Some(BlobHandle::new(ctx, blobstore, basic_file_change).await?), - )), - // No pointer content provided: we're generatic spec conformant canonical - // pointer - GitLfs::GitLfsPointer { - non_canonical_pointer: None, - } => { - let oid = generate_and_store_git_lfs_pointer( - blobstore, - filestore_config, - ctx, - basic_file_change, - ) - .await?; - Ok(( - mpath, - Some(BlobHandle::from_oid_and_file_type( - oid, - basic_file_change.file_type(), - )), - )) - } - // Non canonical LFS pointer provided - let's use it as is - GitLfs::GitLfsPointer { - non_canonical_pointer: Some(non_canonical_pointer), - } => Ok(( - mpath, - Some( - BlobHandle::from_content_id_and_file_type( - ctx, - blobstore, - non_canonical_pointer, - basic_file_change.file_type(), - ) - .await?, - ), - )), - }, - None => Ok((mpath, None)), - } - }) - .collect::>() - .try_collect() - .await -} - -#[cfg(test)] -mod test { - use std::fs::File; - use std::io::Write; - use std::path::Path; - use std::str::FromStr; - - use anyhow::format_err; - use bonsai_git_mapping::BonsaiGitMapping; - use bonsai_hg_mapping::BonsaiHgMapping; - use bookmarks::BookmarkKey; - use bookmarks::Bookmarks; - use bookmarks::BookmarksRef; - use commit_graph::CommitGraph; - use commit_graph::CommitGraphWriter; - use fbinit::FacebookInit; - use filestore::Alias; - use filestore::FetchKey; - use filestore::FilestoreConfig; - use fixtures::TestRepoFixture; - use git2::Oid; - use git2::Repository; - use manifest::ManifestOps; - use mononoke_macros::mononoke; - use repo_blobstore::RepoBlobstore; - use repo_blobstore::RepoBlobstoreArc; - use repo_derived_data::RepoDerivedData; - use repo_derived_data::RepoDerivedDataRef; - use repo_identity::RepoIdentity; - use tempfile::TempDir; - - use super::*; - - #[facet::container] - struct Repo( - dyn BonsaiGitMapping, - dyn BonsaiHgMapping, - dyn Bookmarks, - RepoBlobstore, - RepoDerivedData, - RepoIdentity, - CommitGraph, - dyn CommitGraphWriter, - FilestoreConfig, - ); - - /// This function creates a new Git tree from the fixture's master Bonsai bookmark, - /// materializes it to disk, then verifies that libgit produces the same Git tree for it. - async fn run_tree_derivation_for_fixture( - fb: FacebookInit, - repo: impl BookmarksRef + RepoBlobstoreArc + RepoDerivedDataRef + Send + Sync, - ) -> Result<(), Error> { - let ctx = CoreContext::test_mock(fb); - - let bcs_id = repo - .bookmarks() - .get(ctx.clone(), &BookmarkKey::from_str("master")?) - .await? - .ok_or_else(|| format_err!("no master"))?; - - let tree = repo - .repo_derived_data() - .derive::(&ctx, bcs_id) - .await?; - - let leaves = tree - .list_leaf_entries(ctx.clone(), repo.repo_blobstore_arc()) - .try_collect::>() - .await?; - - let tmp_dir = TempDir::with_prefix("git_types_test.")?; - let root_path = tmp_dir.path(); - let git = Repository::init(root_path)?; - let mut index = git.index()?; - - for (mpath, blob_handle) in leaves.into_iter() { - let blob = filestore::fetch_concat( - repo.repo_blobstore(), - &ctx, - FetchKey::Aliased(Alias::GitSha1(blob_handle.oid().sha1())), - ) - .await?; - - let path = &mpath.to_string(); - let path = Path::new(&path); - File::create(root_path.join(path))?.write_all(&blob)?; - - index.add_path(path)?; - } - - let git_oid = index.write_tree()?; - let derived_tree_oid = Oid::from_bytes(tree.oid().as_ref())?; - assert_eq!(git_oid, derived_tree_oid); - - tmp_dir.close()?; - - Ok(()) - } - - macro_rules! impl_test { - ($test_name:ident, $fixture:ident) => { - #[mononoke::fbinit_test] - fn $test_name(fb: FacebookInit) -> Result<(), Error> { - let runtime = Runtime::new()?; - runtime.block_on(async move { - let repo: Repo = fixtures::$fixture::get_repo(fb).await; - run_tree_derivation_for_fixture(fb, repo).await - }) - } - }; - } - - impl_test!(linear, Linear); - impl_test!(branch_even, BranchEven); - impl_test!(branch_uneven, BranchUneven); - impl_test!(branch_wide, BranchWide); - impl_test!(merge_even, MergeEven); - impl_test!(many_files_dirs, ManyFilesDirs); - impl_test!(merge_uneven, MergeUneven); - impl_test!(unshared_merge_even, UnsharedMergeEven); - impl_test!(unshared_merge_uneven, UnsharedMergeUneven); - impl_test!(many_diamonds, ManyDiamonds); -} diff --git a/eden/mononoke/git/git_types/src/handles/tree.rs b/eden/mononoke/git/git_types/src/handles/tree.rs deleted file mode 100644 index d277bc6146bff..0000000000000 --- a/eden/mononoke/git/git_types/src/handles/tree.rs +++ /dev/null @@ -1,315 +0,0 @@ -/* - * 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. - */ - -use std::cmp; -use std::cmp::Ordering; -use std::collections::HashMap; -use std::fmt; -use std::fmt::Display; -use std::io; -use std::io::Write; - -use ::manifest::Entry; -use anyhow::Error; -use mononoke_types::hash::RichGitSha1; -use mononoke_types::MPathElement; - -use crate::errors::MononokeGitError; -use crate::mode; -use crate::thrift; -use crate::BlobHandle; -use crate::ObjectKind; - -#[derive(Debug, Copy, Clone, Hash, Eq, PartialEq)] -pub struct TreeHandle { - oid: RichGitSha1, -} - -impl TreeHandle { - pub fn filemode(&self) -> i32 { - mode::GIT_FILEMODE_TREE - } - - pub fn oid(&self) -> &RichGitSha1 { - &self.oid - } - - pub fn blobstore_key(&self) -> String { - format!("git_tree.{}", self.oid) - } -} - -impl TryFrom for TreeHandle { - type Error = Error; - - fn try_from(t: thrift::TreeHandle) -> Result { - let size = t.size.try_into()?; - let oid = RichGitSha1::from_bytes(&t.oid.0, ObjectKind::Tree.as_str(), size)?; - Ok(Self { oid }) - } -} - -impl From for thrift::TreeHandle { - fn from(th: TreeHandle) -> thrift::TreeHandle { - let size = th.oid.size(); - - thrift::TreeHandle { - oid: th.oid.into_thrift(), - size: size.try_into().expect("Tree size must fit in a i64"), - } - } -} - -#[derive(Debug, Copy, Clone, Hash, Eq, PartialEq)] -pub enum TreeMember { - Blob(BlobHandle), - Tree(TreeHandle), -} - -impl From for Entry { - fn from(tm: TreeMember) -> Entry { - match tm { - TreeMember::Blob(handle) => Entry::Leaf(handle), - TreeMember::Tree(handle) => Entry::Tree(handle), - } - } -} - -impl From> for TreeMember { - fn from(entry: Entry) -> Self { - match entry { - Entry::Leaf(handle) => Self::Blob(handle), - Entry::Tree(handle) => Self::Tree(handle), - } - } -} - -impl TreeMember { - pub fn filemode(&self) -> i32 { - match self { - Self::Blob(ref blob) => blob.filemode(), - Self::Tree(ref tree) => tree.filemode(), - } - } - - pub fn oid(&self) -> &RichGitSha1 { - match self { - Self::Blob(ref blob) => blob.oid(), - Self::Tree(ref tree) => tree.oid(), - } - } - - pub fn kind(&self) -> ObjectKind { - match self { - Self::Blob(..) => ObjectKind::Blob, - Self::Tree(..) => ObjectKind::Tree, - } - } -} - -impl TryFrom for TreeMember { - type Error = Error; - - fn try_from(t: thrift::TreeMember) -> Result { - match t { - thrift::TreeMember::Blob(blob) => Ok(Self::Blob(blob.try_into()?)), - thrift::TreeMember::Tree(tree) => Ok(Self::Tree(tree.try_into()?)), - thrift::TreeMember::UnknownField(..) => Err(MononokeGitError::InvalidThrift.into()), - } - } -} - -impl From for thrift::TreeMember { - fn from(tm: TreeMember) -> thrift::TreeMember { - match tm { - TreeMember::Blob(blob) => thrift::TreeMember::Blob(blob.into()), - TreeMember::Tree(tree) => thrift::TreeMember::Tree(tree.into()), - } - } -} - -#[derive(Debug, Clone)] -pub struct Tree { - handle: TreeHandle, - members: HashMap, -} - -impl Tree { - pub fn handle(&self) -> &TreeHandle { - &self.handle - } -} - -impl TryFrom for Tree { - type Error = Error; - - fn try_from(t: thrift::Tree) -> Result { - let handle = t.handle.try_into()?; - - let members = t - .members - .into_iter() - .map(|(path, member)| { - let path = MPathElement::from_thrift(path)?; - let member = member.try_into()?; - Ok((path, member)) - }) - .collect::, Error>>()?; - - Ok(Self { handle, members }) - } -} - -impl From for thrift::Tree { - fn from(t: Tree) -> thrift::Tree { - let Tree { handle, members } = t; - - let members = members - .into_iter() - .map(|(path, member)| (path.into_thrift(), member.into())) - .collect(); - - thrift::Tree { - handle: handle.into(), - members, - } - } -} - -#[derive(Debug, Clone, Default)] -pub struct TreeBuilder { - members: HashMap, -} - -impl TreeBuilder { - // TODO: Can we verify members here (git_path_isvalid) - pub fn new(members: HashMap) -> Self { - Self { members } - } - - pub fn into_tree_with_bytes(self) -> (Vec, Tree) { - let mut object_buff = Vec::new(); - self.write_serialized_object(&mut object_buff) - .expect("Writes to Vec cannot fail"); - - let oid = ObjectKind::Tree.create_oid(&object_buff); - - let tree = Tree { - handle: TreeHandle { oid }, - members: self.members, - }; - (object_buff, tree) - } -} - -impl From for Tree { - fn from(tb: TreeBuilder) -> Tree { - let mut object_buff = Vec::new(); - tb.write_serialized_object(&mut object_buff) - .expect("Writes to Vec cannot fail"); - - let oid = ObjectKind::Tree.create_oid(&object_buff); - - Tree { - handle: TreeHandle { oid }, - members: tb.members, - } - } -} - -pub trait Treeish { - fn members(&self) -> &HashMap; - - fn write_serialized_object(&self, writer: &mut impl Write) -> Result<(), io::Error> { - for (path, member) in iter_members_git_path_order(self.members()) { - write!(writer, "{:o} ", member.filemode())?; - writer.write_all(path.as_ref())?; - writer.write_all(&[0])?; - writer.write_all(member.oid().as_ref())?; - } - - Ok(()) - } - - fn write_humanized_representation(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - for (path, member) in iter_members_git_path_order(self.members()) { - write!( - f, - "{:06o} {} {}\t{}\n", - member.filemode(), - member.kind().as_str(), - member.oid(), - path - )?; - } - - Ok(()) - } -} - -impl Treeish for Tree { - fn members(&self) -> &HashMap { - &self.members - } -} - -impl Treeish for TreeBuilder { - fn members(&self) -> &HashMap { - &self.members - } -} - -impl Display for Tree { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - self.write_humanized_representation(f) - } -} - -impl Display for TreeBuilder { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - self.write_humanized_representation(f) - } -} - -fn iter_members_git_path_order( - members: &HashMap, -) -> impl Iterator { - let mut members: Vec<_> = members.iter().collect(); - members.sort_by(|(p1, e1), (p2, e2)| git_path_cmp(p1, e1, p2, e2)); - members.into_iter() -} - -// TODO: Expose git_path_cmp from libgit2 and use it here -// https://github.com/libgit2/libgit2/blob/fb439c975a2de33f5b0c317f3fdea49dc94b27dc/src/path.c#L850 -fn git_path_cmp( - p1: &MPathElement, - e1: &TreeMember, - p2: &MPathElement, - e2: &TreeMember, -) -> Ordering { - const NULL: u8 = 0; - const SLASH: u8 = b'/'; - - let p1 = p1.as_ref(); - let p2 = p2.as_ref(); - let len = cmp::min(p1.len(), p2.len()); - - let ordering = p1[..len].cmp(&p2[..len]); - if ordering != Ordering::Equal { - return ordering; - } - - let c1 = p1 - .get(len) - .unwrap_or(if e1.kind().is_tree() { &SLASH } else { &NULL }); - - let c2 = p2 - .get(len) - .unwrap_or(if e2.kind().is_tree() { &SLASH } else { &NULL }); - - c1.cmp(c2) -} diff --git a/eden/mononoke/git/git_types/src/lib.rs b/eden/mononoke/git/git_types/src/lib.rs index 12ad608e95607..bf8bcbec322cc 100644 --- a/eden/mononoke/git/git_types/src/lib.rs +++ b/eden/mononoke/git/git_types/src/lib.rs @@ -8,7 +8,6 @@ #![feature(error_generic_member_access)] #![feature(iterator_try_reduce)] -pub mod handles; pub mod mode; mod thrift { @@ -22,7 +21,6 @@ mod derive_commit; mod derive_delta_manifest_v2; mod errors; pub mod git_lfs; -mod manifest; mod object; mod store; pub mod tree; @@ -51,12 +49,6 @@ pub use crate::delta_manifest_v2::GDMV2ObjectEntry; pub use crate::delta_manifest_v2::GitDeltaManifestV2; pub use crate::derive_delta_manifest_v2::RootGitDeltaManifestV2Id; pub use crate::errors::GitError; -pub use crate::handles::blob::BlobHandle; -pub use crate::handles::tree::Tree; -pub use crate::handles::tree::TreeBuilder; -pub use crate::handles::tree::TreeHandle; -pub use crate::handles::tree::TreeMember; -pub use crate::handles::tree::Treeish; pub use crate::store::fetch_git_object; pub use crate::store::fetch_git_object_bytes; pub use crate::store::fetch_non_blob_git_object; diff --git a/eden/mononoke/git/git_types/src/manifest.rs b/eden/mononoke/git/git_types/src/manifest.rs deleted file mode 100644 index e58877c595102..0000000000000 --- a/eden/mononoke/git/git_types/src/manifest.rs +++ /dev/null @@ -1,65 +0,0 @@ -/* - * 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. - */ - -use ::manifest::Entry; -use ::manifest::Manifest; -use anyhow::Result; -use async_trait::async_trait; -use context::CoreContext; -use futures::stream; -use futures::stream::BoxStream; -use futures::stream::StreamExt; -use mononoke_types::MPathElement; -use mononoke_types::SortedVectorTrieMap; - -use crate::BlobHandle; -use crate::Tree; -use crate::TreeHandle; -use crate::Treeish; - -#[async_trait] -impl Manifest for Tree { - type TreeId = TreeHandle; - type Leaf = BlobHandle; - type TrieMapType = SortedVectorTrieMap>; - - async fn lookup( - &self, - _ctx: &CoreContext, - _blobstore: &Store, - name: &MPathElement, - ) -> Result>> { - Ok(self.members().get(name).map(|e| e.clone().into())) - } - - async fn list( - &self, - _ctx: &CoreContext, - _blobstore: &Store, - ) -> Result)>>> - { - let members: Vec<_> = self - .members() - .iter() - .map(|(k, v)| (k.clone(), v.clone().into())) - .collect(); - Ok(stream::iter(members).map(Ok).boxed()) - } - - async fn into_trie_map( - self, - _ctx: &CoreContext, - _blobstore: &Store, - ) -> Result { - let members = self - .members() - .iter() - .map(|(k, v)| (k.clone().to_smallvec(), v.clone().into())) - .collect(); - Ok(SortedVectorTrieMap::new(members)) - } -} diff --git a/eden/mononoke/git/git_types/src/store.rs b/eden/mononoke/git/git_types/src/store.rs index 573ff985eb045..9bac801b3e47b 100644 --- a/eden/mononoke/git/git_types/src/store.rs +++ b/eden/mononoke/git/git_types/src/store.rs @@ -9,7 +9,6 @@ use std::io::Write; use std::sync::Arc; use anyhow::Result; -use blobstore::impl_loadable_storable; use blobstore::Blobstore; use bytes::Bytes; use context::CoreContext; @@ -17,7 +16,6 @@ use filestore::fetch_with_size; use filestore::hash_bytes; use filestore::Sha1IncrementalHasher; use futures::TryStreamExt; -use futures_watchdog::WatchdogExt; use gix_hash::ObjectId; use gix_object::WriteTo; use mononoke_types::hash::GitSha1; @@ -27,17 +25,6 @@ use packfile::types::BaseObject; use packfile::types::GitPackfileBaseItem; use crate::errors::GitError; -use crate::thrift::Tree as ThriftTree; -use crate::thrift::TreeHandle as ThriftTreeHandle; -use crate::Tree; -use crate::TreeHandle; - -impl_loadable_storable! { - handle_type => TreeHandle, - handle_thrift_type => ThriftTreeHandle, - value_type => Tree, - value_thrift_type => ThriftTree, -} const GIT_OBJECT_PREFIX: &str = "git_object"; const GIT_PACKFILE_BASE_ITEM_PREFIX: &str = "git_packfile_base_item"; diff --git a/eden/mononoke/git/gitexport/src/gitexport_tools/commit_rewrite.rs b/eden/mononoke/git/gitexport/src/gitexport_tools/commit_rewrite.rs index fd84376f7d9e0..50d35a754e366 100644 --- a/eden/mononoke/git/gitexport/src/gitexport_tools/commit_rewrite.rs +++ b/eden/mononoke/git/gitexport/src/gitexport_tools/commit_rewrite.rs @@ -33,7 +33,6 @@ use futures::StreamExt; use futures_stats::TimedTryFutureExt; use git_types::MappedGitCommitId; use git_types::RootGitDeltaManifestV2Id; -use git_types::TreeHandle; use indicatif::ProgressBar; use indicatif::ProgressStyle; use maplit::hashmap; @@ -558,7 +557,6 @@ async fn create_temp_repo(fb: FacebookInit, ctx: &CoreContext) -> Result Result<(), Error> { .iter() .filter(|ty| match ty { // If we discard submodules, we can't derive the git data types since they are inconsistent - DerivableType::GitCommits - | DerivableType::GitDeltaManifestsV2 - | DerivableType::GitTrees => false, + DerivableType::GitCommits | DerivableType::GitDeltaManifestsV2 => false, _ => true, }) .cloned() diff --git a/eden/mononoke/mononoke_api/src/test/test_repo_create_changeset.rs b/eden/mononoke/mononoke_api/src/test/test_repo_create_changeset.rs index 24e797396cb94..a1fbc75db187d 100644 --- a/eden/mononoke/mononoke_api/src/test/test_repo_create_changeset.rs +++ b/eden/mononoke/mononoke_api/src/test/test_repo_create_changeset.rs @@ -191,10 +191,6 @@ async fn validate_unnecessary_derived_data_is_not_derived( derived_data_to_derive: DerivableType, ) -> Result<(), Error> { for ty in &repo.repo().repo_derived_data_arc().active_config().types { - if *ty == DerivableType::GitTrees { - // Derived data utils doesn't support git_trees, so we have to skip it - continue; - } let not_derived = repo .repo() .repo_derived_data() diff --git a/eden/mononoke/mononoke_types/src/derivable_type.rs b/eden/mononoke/mononoke_types/src/derivable_type.rs index 0672d0233682f..c6b6a6f71ff16 100644 --- a/eden/mononoke/mononoke_types/src/derivable_type.rs +++ b/eden/mononoke/mononoke_types/src/derivable_type.rs @@ -36,7 +36,6 @@ pub enum DerivableType { Fsnodes, HgChangesets, HgAugmentedManifests, - GitTrees, GitCommits, GitDeltaManifestsV2, SkeletonManifests, @@ -62,7 +61,6 @@ impl DerivableType { "fsnodes" => DerivableType::Fsnodes, "hgchangesets" => DerivableType::HgChangesets, "hg_augmented_manifests" => DerivableType::HgAugmentedManifests, - "git_trees" => DerivableType::GitTrees, "git_commits" => DerivableType::GitCommits, "git_delta_manifests_v2" => DerivableType::GitDeltaManifestsV2, "skeleton_manifests" => DerivableType::SkeletonManifests, @@ -88,7 +86,6 @@ impl DerivableType { DerivableType::Fsnodes => "fsnodes", DerivableType::HgChangesets => "hgchangesets", DerivableType::HgAugmentedManifests => "hg_augmented_manifests", - DerivableType::GitTrees => "git_trees", DerivableType::GitCommits => "git_commits", DerivableType::GitDeltaManifestsV2 => "git_delta_manifests_v2", DerivableType::SkeletonManifests => "skeleton_manifests", @@ -111,7 +108,6 @@ impl DerivableType { thrift::DerivedDataType::FSNODE => Self::Fsnodes, thrift::DerivedDataType::HG_CHANGESET => Self::HgChangesets, thrift::DerivedDataType::HG_AUGMENTED_MANIFEST => Self::HgAugmentedManifests, - thrift::DerivedDataType::TREE_HANDLE => Self::GitTrees, thrift::DerivedDataType::COMMIT_HANDLE => Self::GitCommits, thrift::DerivedDataType::GIT_DELTA_MANIFEST_V2 => Self::GitDeltaManifestsV2, thrift::DerivedDataType::SKELETON_MANIFEST => Self::SkeletonManifests, @@ -135,7 +131,6 @@ impl DerivableType { Self::Fsnodes => thrift::DerivedDataType::FSNODE, Self::HgChangesets => thrift::DerivedDataType::HG_CHANGESET, Self::HgAugmentedManifests => thrift::DerivedDataType::HG_AUGMENTED_MANIFEST, - Self::GitTrees => thrift::DerivedDataType::TREE_HANDLE, Self::GitCommits => thrift::DerivedDataType::COMMIT_HANDLE, Self::GitDeltaManifestsV2 => thrift::DerivedDataType::GIT_DELTA_MANIFEST_V2, Self::SkeletonManifests => thrift::DerivedDataType::SKELETON_MANIFEST, diff --git a/eden/mononoke/repo_import/src/tests.rs b/eden/mononoke/repo_import/src/tests.rs index b146f798388f4..8ecfc04d913ca 100644 --- a/eden/mononoke/repo_import/src/tests.rs +++ b/eden/mononoke/repo_import/src/tests.rs @@ -33,7 +33,6 @@ mod tests { use futures::stream::TryStreamExt; use git_types::MappedGitCommitId; use git_types::RootGitDeltaManifestV2Id; - use git_types::TreeHandle; use live_commit_sync_config::CfgrLiveCommitSyncConfig; use live_commit_sync_config::LiveCommitSyncConfig; use live_commit_sync_config::TestLiveCommitSyncConfig; @@ -104,7 +103,6 @@ mod tests { .get_active_config_mut() .expect("No enabled derived data types config"); // Repo import has no need of these derived data types - config.types.remove(&TreeHandle::VARIANT); config.types.remove(&MappedGitCommitId::VARIANT); config.types.remove(&RootGitDeltaManifestV2Id::VARIANT); }) @@ -126,7 +124,6 @@ mod tests { .get_active_config_mut() .expect("No enabled derived data types config"); // Repo import has no need of these derived data types - config.types.remove(&TreeHandle::VARIANT); config.types.remove(&MappedGitCommitId::VARIANT); config.types.remove(&RootGitDeltaManifestV2Id::VARIANT); }) diff --git a/eden/mononoke/tools/admin/src/commands/blobstore/fetch.rs b/eden/mononoke/tools/admin/src/commands/blobstore/fetch.rs index 433691493ebc6..c73723b964c1a 100644 --- a/eden/mononoke/tools/admin/src/commands/blobstore/fetch.rs +++ b/eden/mononoke/tools/admin/src/commands/blobstore/fetch.rs @@ -24,7 +24,6 @@ use context::CoreContext; use futures::TryStreamExt; use git_types::GDMV2Entry; use git_types::GitDeltaManifestV2; -use git_types::Tree as GitTree; use mercurial_types::HgAugmentedManifestEntry; use mercurial_types::HgAugmentedManifestEnvelope; use mercurial_types::HgChangesetEnvelope; @@ -94,7 +93,6 @@ pub enum DecodeAs { HgAugmentedManifest, ShardedHgAugmentedManifestMapNode, ShardedHgAugmentedManifest, - GitTree, GitDeltaManifestV2MapNode, GitDeltaManifestV2, SkeletonManifest, @@ -139,7 +137,6 @@ impl DecodeAs { DecodeAs::ShardedHgAugmentedManifestMapNode, ), ("hgaugmentedmanifest.", DecodeAs::HgAugmentedManifest), - ("git.tree.", DecodeAs::GitTree), ("gdm2.map2node.", DecodeAs::GitDeltaManifestV2MapNode), ("gdm2.", DecodeAs::GitDeltaManifestV2), ("skeletonmanifest.", DecodeAs::SkeletonManifest), @@ -259,7 +256,6 @@ async fn decode( Err(e) => Decoded::Fail(e.to_string()), } } - DecodeAs::GitTree => Decoded::try_display(GitTree::try_from(data)), DecodeAs::GitDeltaManifestV2 => { Decoded::try_debug(GitDeltaManifestV2::from_bytes(&data.into_raw_bytes())) } diff --git a/eden/mononoke/walker/src/detail/graph.rs b/eden/mononoke/walker/src/detail/graph.rs index 6e97ed2a71160..0180e22bf1de0 100644 --- a/eden/mononoke/walker/src/detail/graph.rs +++ b/eden/mononoke/walker/src/detail/graph.rs @@ -1194,7 +1194,6 @@ mod tests { // list, otherwise it won't get scrubbed and thus you would be unaware of different representation // in different stores let grandfathered: HashSet = HashSet::from_iter(vec![ - DerivableType::GitTrees, DerivableType::GitCommits, DerivableType::GitDeltaManifestsV2, DerivableType::TestManifests,