From f2a723b25293f326d6f3216a834dd388601ac6b1 Mon Sep 17 00:00:00 2001 From: Gustavo Galvao Avena Date: Fri, 15 Mar 2024 02:53:48 -0700 Subject: [PATCH] Move structs to types module Summary: ## This stack See D54855752 for more context. ## This diff As title, to clean up the `commit_syncers_lib` module a bit more. Don't feel strongly about this one though, so I'm happy to abandon it if anyone sees any downsides. Reviewed By: mitrandir77 Differential Revision: D54867144 fbshipit-source-id: c1d710262061308cc4d20c400d54e925d848ee75 --- .../src/commit_in_memory_syncer.rs | 6 +- .../src/commit_sync_outcome.rs | 2 +- .../cross_repo_sync/src/commit_syncer.rs | 8 +- .../cross_repo_sync/src/commit_syncers_lib.rs | 179 +---------------- .../cross_repo_sync/src/git_submodules.rs | 2 +- .../cross_repo_sync/src/lib.rs | 10 +- .../cross_repo_sync/src/pushrebase_hook.rs | 2 +- .../cross_repo_sync/src/types.rs | 180 ++++++++++++++++++ .../cross_repo_sync/src/validation.rs | 2 +- 9 files changed, 199 insertions(+), 192 deletions(-) diff --git a/eden/mononoke/commit_rewriting/cross_repo_sync/src/commit_in_memory_syncer.rs b/eden/mononoke/commit_rewriting/cross_repo_sync/src/commit_in_memory_syncer.rs index daf0c569efcb2..7f7e9d58d5804 100644 --- a/eden/mononoke/commit_rewriting/cross_repo_sync/src/commit_in_memory_syncer.rs +++ b/eden/mononoke/commit_rewriting/cross_repo_sync/src/commit_in_memory_syncer.rs @@ -36,14 +36,14 @@ use crate::commit_syncers_lib::get_mover_by_version; use crate::commit_syncers_lib::get_x_repo_submodule_metadata_file_prefx_from_config; use crate::commit_syncers_lib::rewrite_commit; use crate::commit_syncers_lib::strip_removed_parents; -use crate::commit_syncers_lib::ErrorKind; -use crate::commit_syncers_lib::Repo; -use crate::commit_syncers_lib::SubmoduleDeps; use crate::reporting::CommitSyncContext; use crate::sync_config_version_utils::get_mapping_change_version; use crate::sync_config_version_utils::get_version; use crate::sync_config_version_utils::get_version_for_merge; +use crate::types::ErrorKind; +use crate::types::Repo; use crate::types::Source; +use crate::types::SubmoduleDeps; use crate::types::Target; // TODO(T182311609): remove duplication from `CommitSyncOutcome` diff --git a/eden/mononoke/commit_rewriting/cross_repo_sync/src/commit_sync_outcome.rs b/eden/mononoke/commit_rewriting/cross_repo_sync/src/commit_sync_outcome.rs index ac2f9446cd6ed..fc6100be7382e 100644 --- a/eden/mononoke/commit_rewriting/cross_repo_sync/src/commit_sync_outcome.rs +++ b/eden/mononoke/commit_rewriting/cross_repo_sync/src/commit_sync_outcome.rs @@ -26,7 +26,7 @@ use synced_commit_mapping::SyncedCommitMapping; use synced_commit_mapping::WorkingCopyEquivalence; use crate::commit_sync_config_utils::get_small_repos_for_version; -use crate::commit_syncers_lib::Repo; +use crate::types::Repo; use crate::types::Source; use crate::types::Target; diff --git a/eden/mononoke/commit_rewriting/cross_repo_sync/src/commit_syncer.rs b/eden/mononoke/commit_rewriting/cross_repo_sync/src/commit_syncer.rs index 7487d10b5c017..b5c6a7a8d670c 100644 --- a/eden/mononoke/commit_rewriting/cross_repo_sync/src/commit_syncer.rs +++ b/eden/mononoke/commit_rewriting/cross_repo_sync/src/commit_syncer.rs @@ -67,10 +67,6 @@ use crate::commit_syncers_lib::rewrite_commit; use crate::commit_syncers_lib::run_with_lease; use crate::commit_syncers_lib::update_mapping_with_version; use crate::commit_syncers_lib::CommitSyncRepos; -use crate::commit_syncers_lib::ErrorKind; -use crate::commit_syncers_lib::PushrebaseRewriteDates; -use crate::commit_syncers_lib::Repo; -use crate::commit_syncers_lib::SubmoduleDeps; use crate::commit_syncers_lib::SyncedAncestorsVersions; use crate::pushrebase_hook::CrossRepoSyncPushrebaseHook; use crate::reporting; @@ -78,7 +74,11 @@ use crate::reporting::log_rewrite; use crate::reporting::CommitSyncContext; use crate::sync_config_version_utils::get_version; use crate::sync_config_version_utils::set_mapping_change_version; +use crate::types::ErrorKind; +use crate::types::PushrebaseRewriteDates; +use crate::types::Repo; use crate::types::Source; +use crate::types::SubmoduleDeps; use crate::types::Target; #[derive(Clone)] diff --git a/eden/mononoke/commit_rewriting/cross_repo_sync/src/commit_syncers_lib.rs b/eden/mononoke/commit_rewriting/cross_repo_sync/src/commit_syncers_lib.rs index fa55b7b01ea84..bb6bbc1498a6b 100644 --- a/eden/mononoke/commit_rewriting/cross_repo_sync/src/commit_syncers_lib.rs +++ b/eden/mononoke/commit_rewriting/cross_repo_sync/src/commit_syncers_lib.rs @@ -16,28 +16,11 @@ use anyhow::anyhow; use anyhow::format_err; use anyhow::Error; use anyhow::Result; -use bonsai_git_mapping::BonsaiGitMapping; -use bonsai_git_mapping::BonsaiGitMappingArc; -use bonsai_git_mapping::BonsaiGitMappingRef; -use bonsai_globalrev_mapping::BonsaiGlobalrevMapping; -use bonsai_globalrev_mapping::BonsaiGlobalrevMappingArc; -use bonsai_hg_mapping::BonsaiHgMapping; -use bonsai_hg_mapping::BonsaiHgMappingRef; -use bookmarks::BookmarkUpdateLog; -use bookmarks::BookmarkUpdateLogArc; -use bookmarks::BookmarkUpdateLogRef; -use bookmarks::Bookmarks; -use bookmarks::BookmarksArc; -use bookmarks::BookmarksRef; use borrowed::borrowed; use cacheblob::InProcessLease; use cacheblob::LeaseOps; use cacheblob::MemcacheOps; use changeset_info::ChangesetInfo; -use changesets::Changesets; -use changesets::ChangesetsRef; -use commit_graph::CommitGraph; -use commit_graph::CommitGraphRef; use commit_transformation::rewrite_commit_with_file_changes_filter; use commit_transformation::FileChangeFilter; use commit_transformation::FileChangeFilterApplication; @@ -48,8 +31,6 @@ use context::CoreContext; use derived_data::BonsaiDerived; use environment::Caching; use fbinit::FacebookInit; -use filestore::FilestoreConfig; -use filestore::FilestoreConfigRef; use futures::channel::oneshot; use futures::future::try_join; use futures::stream; @@ -62,8 +43,6 @@ use metaconfig_types::CommitSyncConfigVersion; use metaconfig_types::CommitSyncDirection; use metaconfig_types::CommonCommitSyncConfig; use metaconfig_types::GitSubmodulesChangesAction; -use metaconfig_types::RepoConfig; -use metaconfig_types::RepoConfigRef; use mononoke_types::BonsaiChangesetMut; use mononoke_types::ChangesetId; use mononoke_types::FileChange; @@ -71,30 +50,10 @@ use mononoke_types::FileType; use mononoke_types::NonRootMPath; use mononoke_types::RepositoryId; use movers::Mover; -use mutable_counters::MutableCounters; -use mutable_counters::MutableCountersArc; -use phases::Phases; -use phases::PhasesRef; -use pushrebase::PushrebaseError; -use pushrebase_mutation_mapping::PushrebaseMutationMapping; -use pushrebase_mutation_mapping::PushrebaseMutationMappingRef; -use repo_blobstore::RepoBlobstore; -use repo_blobstore::RepoBlobstoreArc; -use repo_blobstore::RepoBlobstoreRef; -use repo_bookmark_attrs::RepoBookmarkAttrs; -use repo_bookmark_attrs::RepoBookmarkAttrsRef; -use repo_cross_repo::RepoCrossRepo; -use repo_cross_repo::RepoCrossRepoRef; -use repo_derived_data::RepoDerivedData; -use repo_derived_data::RepoDerivedDataRef; -use repo_identity::RepoIdentity; -use repo_identity::RepoIdentityRef; use slog::info; -use static_assertions::assert_impl_all; use synced_commit_mapping::SyncedCommitMapping; use synced_commit_mapping::SyncedCommitMappingEntry; use synced_commit_mapping::SyncedCommitSourceRepo; -use thiserror::Error; use topo_sort::sort_topological; use crate::commit_sync_config_utils::get_mover; @@ -105,43 +64,14 @@ use crate::commit_sync_outcome::PluralCommitSyncOutcome; use crate::commit_syncer::CommitSyncer; use crate::git_submodules::expand_all_git_submodule_file_changes; use crate::sync_config_version_utils::get_mapping_change_version; +use crate::types::ErrorKind; +use crate::types::Repo; use crate::types::Source; +use crate::types::SubmoduleDeps; use crate::types::Target; const LEASE_WARNING_THRESHOLD: Duration = Duration::from_secs(60); -#[derive(Debug, Error)] -pub enum ErrorKind { - #[error("Pushrebase of synced commit failed - check config for overlaps: {0:?}")] - PushrebaseFailure(PushrebaseError), - #[error("Remapped commit {0} expected in target repo, but not present")] - MissingRemappedCommit(ChangesetId), - #[error("Could not find a commit in the target repo with the same working copy as {0}")] - SameWcSearchFail(ChangesetId), - #[error("Parent commit {0} hasn't been remapped")] - ParentNotRemapped(ChangesetId), - #[error("Parent commit {0} is not a sync candidate")] - ParentNotSyncCandidate(ChangesetId), - #[error("Cannot choose working copy equivalent for {0}")] - AmbiguousWorkingCopyEquivalent(ChangesetId), - #[error( - "expected {expected_version} mapping version to be used to remap {cs_id}, but actually {actual_version} mapping version was used" - )] - UnexpectedVersion { - expected_version: CommitSyncConfigVersion, - actual_version: CommitSyncConfigVersion, - cs_id: ChangesetId, - }, - #[error("X-repo sync is temporarily disabled, contact source control oncall")] - XRepoSyncDisabled, -} - -#[derive(PartialEq, Copy, Clone, Debug)] -pub enum PushrebaseRewriteDates { - Yes, - No, -} - /// Create a version of `cs` with `Mover` applied to all changes /// The return value can be: /// - `Err` if the rewrite failed @@ -479,109 +409,6 @@ where )) } -pub trait Repo = BookmarksArc - + BookmarksRef - + BookmarkUpdateLogArc - + BookmarkUpdateLogRef - + RepoBlobstoreArc - + BonsaiHgMappingRef - + BonsaiGlobalrevMappingArc - + RepoCrossRepoRef - + PushrebaseMutationMappingRef - + RepoBookmarkAttrsRef - + BonsaiGitMappingRef - + BonsaiGitMappingArc - + FilestoreConfigRef - + ChangesetsRef - + RepoIdentityRef - + MutableCountersArc - + PhasesRef - + RepoBlobstoreRef - + RepoConfigRef - + RepoDerivedDataRef - + CommitGraphRef - + Send - + Sync - + Clone - + 'static; - -/// Simplest repo that implements cross_repo_sync::Repo trait -#[facet::container] -#[derive(Clone)] -pub struct ConcreteRepo { - #[facet] - bookmarks: dyn Bookmarks, - - #[facet] - bookmark_update_log: dyn BookmarkUpdateLog, - - #[facet] - bonsai_hg_mapping: dyn BonsaiHgMapping, - - #[facet] - bonsai_git_mapping: dyn BonsaiGitMapping, - - #[facet] - bonsai_globalrev_mapping: dyn BonsaiGlobalrevMapping, - - #[facet] - pushrebase_mutation_mapping: dyn PushrebaseMutationMapping, - - #[facet] - filestore_config: FilestoreConfig, - - #[facet] - changesets: dyn Changesets, - - #[facet] - id: RepoIdentity, - - #[facet] - phases: dyn Phases, - - #[facet] - repo_cross_repo: RepoCrossRepo, - - #[facet] - repo_bookmark_attrs: RepoBookmarkAttrs, - - #[facet] - config: RepoConfig, - - #[facet] - derived_data: RepoDerivedData, - - #[facet] - blobstore: RepoBlobstore, - - #[facet] - mutable_counters: dyn MutableCounters, - - #[facet] - commit_graph: CommitGraph, -} - -assert_impl_all!(ConcreteRepo: Repo); - -/// Syncing commits from a small Mononoke repo with submodule file changes to a -/// large repo requires the small repo submodule dependencies to be available. -/// -/// However, LargeToSmall sync and some SmallToLarge operations don't require -/// loading these repos, in which case this value will be set to `None`. -/// When rewriting commits from small to large (i.e. calling `rewrite_commit`), -/// this map has to be available, or the operation will crash otherwise. -#[derive(Clone)] -pub enum SubmoduleDeps { - ForSync(HashMap), - NotNeeded, -} - -impl Default for SubmoduleDeps { - fn default() -> Self { - Self::NotNeeded - } -} - #[derive(Clone)] pub enum CommitSyncRepos { LargeToSmall { diff --git a/eden/mononoke/commit_rewriting/cross_repo_sync/src/git_submodules.rs b/eden/mononoke/commit_rewriting/cross_repo_sync/src/git_submodules.rs index 9040ff50858c4..543ca71adf276 100644 --- a/eden/mononoke/commit_rewriting/cross_repo_sync/src/git_submodules.rs +++ b/eden/mononoke/commit_rewriting/cross_repo_sync/src/git_submodules.rs @@ -46,7 +46,7 @@ use mononoke_types::TrackedFileChange; use slog::debug; use sorted_vector_map::SortedVectorMap; -use crate::commit_syncers_lib::Repo; +use crate::types::Repo; /// Wrapper to differentiate submodule paths from file changes paths at the /// type level. diff --git a/eden/mononoke/commit_rewriting/cross_repo_sync/src/lib.rs b/eden/mononoke/commit_rewriting/cross_repo_sync/src/lib.rs index 1c3d8a15d1fa4..270027ee9fb35 100644 --- a/eden/mononoke/commit_rewriting/cross_repo_sync/src/lib.rs +++ b/eden/mononoke/commit_rewriting/cross_repo_sync/src/lib.rs @@ -44,17 +44,17 @@ pub use commit_syncers_lib::get_x_repo_submodule_metadata_file_prefx_from_config pub use commit_syncers_lib::rewrite_commit; pub use commit_syncers_lib::update_mapping_with_version; pub use commit_syncers_lib::CommitSyncRepos; -pub use commit_syncers_lib::ConcreteRepo; -pub use commit_syncers_lib::ErrorKind; -pub use commit_syncers_lib::PushrebaseRewriteDates; -pub use commit_syncers_lib::Repo; -pub use commit_syncers_lib::SubmoduleDeps; pub use commit_syncers_lib::Syncers; pub use reporting::CommitSyncContext; pub use sync_config_version_utils::CHANGE_XREPO_MAPPING_EXTRA; +pub use types::ConcreteRepo; +pub use types::ErrorKind; pub use types::Large; +pub use types::PushrebaseRewriteDates; +pub use types::Repo; pub use types::Small; pub use types::Source; +pub use types::SubmoduleDeps; pub use types::Target; pub use validation::find_bookmark_diff; pub use validation::report_different; diff --git a/eden/mononoke/commit_rewriting/cross_repo_sync/src/pushrebase_hook.rs b/eden/mononoke/commit_rewriting/cross_repo_sync/src/pushrebase_hook.rs index 69b9ec0ffec04..c909211ec9176 100644 --- a/eden/mononoke/commit_rewriting/cross_repo_sync/src/pushrebase_hook.rs +++ b/eden/mononoke/commit_rewriting/cross_repo_sync/src/pushrebase_hook.rs @@ -21,7 +21,7 @@ use synced_commit_mapping::add_many_in_txn; use synced_commit_mapping::SyncedCommitMappingEntry; use crate::commit_syncers_lib::create_synced_commit_mapping_entry; -use crate::commit_syncers_lib::Repo; +use crate::types::Repo; use crate::CommitSyncRepos; use crate::ErrorKind; diff --git a/eden/mononoke/commit_rewriting/cross_repo_sync/src/types.rs b/eden/mononoke/commit_rewriting/cross_repo_sync/src/types.rs index 47e2f6560ec8b..034e5e05887e2 100644 --- a/eden/mononoke/commit_rewriting/cross_repo_sync/src/types.rs +++ b/eden/mononoke/commit_rewriting/cross_repo_sync/src/types.rs @@ -5,6 +5,7 @@ * GNU General Public License version 2. */ +use std::collections::HashMap; use std::fmt; use std::fmt::Debug; use std::fmt::Display; @@ -13,7 +14,51 @@ use std::hash::Hasher; use std::ops::Deref; use std::ops::DerefMut; +use bonsai_git_mapping::BonsaiGitMapping; +use bonsai_git_mapping::BonsaiGitMappingArc; +use bonsai_git_mapping::BonsaiGitMappingRef; +use bonsai_globalrev_mapping::BonsaiGlobalrevMapping; +use bonsai_globalrev_mapping::BonsaiGlobalrevMappingArc; +use bonsai_hg_mapping::BonsaiHgMapping; +use bonsai_hg_mapping::BonsaiHgMappingRef; +use bookmarks::BookmarkUpdateLog; +use bookmarks::BookmarkUpdateLogArc; +use bookmarks::BookmarkUpdateLogRef; +use bookmarks::Bookmarks; +use bookmarks::BookmarksArc; +use bookmarks::BookmarksRef; +use changesets::Changesets; +use changesets::ChangesetsRef; +use commit_graph::CommitGraph; +use commit_graph::CommitGraphRef; +use filestore::FilestoreConfig; +use filestore::FilestoreConfigRef; +use metaconfig_types::CommitSyncConfigVersion; +use metaconfig_types::RepoConfig; +use metaconfig_types::RepoConfigRef; +use mononoke_types::ChangesetId; +use mononoke_types::NonRootMPath; +use mutable_counters::MutableCounters; +use mutable_counters::MutableCountersArc; +use phases::Phases; +use phases::PhasesRef; +use pushrebase::PushrebaseError; +use pushrebase_mutation_mapping::PushrebaseMutationMapping; +use pushrebase_mutation_mapping::PushrebaseMutationMappingRef; use ref_cast::RefCast; +use repo_blobstore::RepoBlobstore; +use repo_blobstore::RepoBlobstoreArc; +use repo_blobstore::RepoBlobstoreRef; +use repo_bookmark_attrs::RepoBookmarkAttrs; +use repo_bookmark_attrs::RepoBookmarkAttrsRef; +use repo_cross_repo::RepoCrossRepo; +use repo_cross_repo::RepoCrossRepoRef; +use repo_derived_data::RepoDerivedData; +use repo_derived_data::RepoDerivedDataRef; +use repo_identity::RepoIdentity; +use repo_identity::RepoIdentityRef; +use static_assertions::assert_impl_all; +use thiserror::Error; macro_rules! generic_newtype_with_obvious_impls { ($name: ident) => { @@ -91,3 +136,138 @@ generic_newtype_with_obvious_impls! { Large } generic_newtype_with_obvious_impls! { Small } generic_newtype_with_obvious_impls! { Source } generic_newtype_with_obvious_impls! { Target } + +#[derive(Debug, Error)] +pub enum ErrorKind { + #[error("Pushrebase of synced commit failed - check config for overlaps: {0:?}")] + PushrebaseFailure(PushrebaseError), + #[error("Remapped commit {0} expected in target repo, but not present")] + MissingRemappedCommit(ChangesetId), + #[error("Could not find a commit in the target repo with the same working copy as {0}")] + SameWcSearchFail(ChangesetId), + #[error("Parent commit {0} hasn't been remapped")] + ParentNotRemapped(ChangesetId), + #[error("Parent commit {0} is not a sync candidate")] + ParentNotSyncCandidate(ChangesetId), + #[error("Cannot choose working copy equivalent for {0}")] + AmbiguousWorkingCopyEquivalent(ChangesetId), + #[error( + "expected {expected_version} mapping version to be used to remap {cs_id}, but actually {actual_version} mapping version was used" + )] + UnexpectedVersion { + expected_version: CommitSyncConfigVersion, + actual_version: CommitSyncConfigVersion, + cs_id: ChangesetId, + }, + #[error("X-repo sync is temporarily disabled, contact source control oncall")] + XRepoSyncDisabled, +} + +#[derive(PartialEq, Copy, Clone, Debug)] +pub enum PushrebaseRewriteDates { + Yes, + No, +} + +pub trait Repo = BookmarksArc + + BookmarksRef + + BookmarkUpdateLogArc + + BookmarkUpdateLogRef + + RepoBlobstoreArc + + BonsaiHgMappingRef + + BonsaiGlobalrevMappingArc + + RepoCrossRepoRef + + PushrebaseMutationMappingRef + + RepoBookmarkAttrsRef + + BonsaiGitMappingRef + + BonsaiGitMappingArc + + FilestoreConfigRef + + ChangesetsRef + + RepoIdentityRef + + MutableCountersArc + + PhasesRef + + RepoBlobstoreRef + + RepoConfigRef + + RepoDerivedDataRef + + CommitGraphRef + + Send + + Sync + + Clone + + 'static; + +/// Simplest repo that implements cross_repo_sync::Repo trait +#[facet::container] +#[derive(Clone)] +pub struct ConcreteRepo { + #[facet] + bookmarks: dyn Bookmarks, + + #[facet] + bookmark_update_log: dyn BookmarkUpdateLog, + + #[facet] + bonsai_hg_mapping: dyn BonsaiHgMapping, + + #[facet] + bonsai_git_mapping: dyn BonsaiGitMapping, + + #[facet] + bonsai_globalrev_mapping: dyn BonsaiGlobalrevMapping, + + #[facet] + pushrebase_mutation_mapping: dyn PushrebaseMutationMapping, + + #[facet] + filestore_config: FilestoreConfig, + + #[facet] + changesets: dyn Changesets, + + #[facet] + id: RepoIdentity, + + #[facet] + phases: dyn Phases, + + #[facet] + repo_cross_repo: RepoCrossRepo, + + #[facet] + repo_bookmark_attrs: RepoBookmarkAttrs, + + #[facet] + config: RepoConfig, + + #[facet] + derived_data: RepoDerivedData, + + #[facet] + blobstore: RepoBlobstore, + + #[facet] + mutable_counters: dyn MutableCounters, + + #[facet] + commit_graph: CommitGraph, +} + +assert_impl_all!(ConcreteRepo: Repo); + +/// Syncing commits from a small Mononoke repo with submodule file changes to a +/// large repo requires the small repo submodule dependencies to be available. +/// +/// However, LargeToSmall sync and some SmallToLarge operations don't require +/// loading these repos, in which case this value will be set to `None`. +/// When rewriting commits from small to large (i.e. calling `rewrite_commit`), +/// this map has to be available, or the operation will crash otherwise. +#[derive(Clone)] +pub enum SubmoduleDeps { + ForSync(HashMap), + NotNeeded, +} + +impl Default for SubmoduleDeps { + fn default() -> Self { + Self::NotNeeded + } +} diff --git a/eden/mononoke/commit_rewriting/cross_repo_sync/src/validation.rs b/eden/mononoke/commit_rewriting/cross_repo_sync/src/validation.rs index 4df858e7a741f..8b05602cb69e1 100644 --- a/eden/mononoke/commit_rewriting/cross_repo_sync/src/validation.rs +++ b/eden/mononoke/commit_rewriting/cross_repo_sync/src/validation.rs @@ -48,7 +48,7 @@ use slog::info; use synced_commit_mapping::SyncedCommitMapping; use crate::commit_syncer::CommitSyncer; -use crate::commit_syncers_lib::Repo; +use crate::types::Repo; use crate::types::Source; use crate::types::Target;