From 01471d97333de63a1bfa1c0dffa3e27a9ab5e4a3 Mon Sep 17 00:00:00 2001 From: Gustavo Galvao Avena Date: Fri, 31 Jan 2025 08:55:19 -0800 Subject: [PATCH] Migrate megarepo tool merge command to newadmin Summary: ## This stack Migrate all commands from megarepo tool CLI to mononoke admin `megarepo` subcommand, so they use mononoke_app + clap 4, instad of old cmdlib + clap-2. I'll try to do one diff per command. ## This diff Migrate the first sub-command: `merge`. Reviewed By: liubov-dmitrieva Differential Revision: D68831083 fbshipit-source-id: 2917454be054bf5bf117c6e291c0d7c0e50bcd2c --- .../commit_rewriting/megarepo/tool/cli.rs | 19 ----- .../commit_rewriting/megarepo/tool/main.rs | 37 --------- .../megarepo/test-megarepo-fixup-history.t | 6 +- .../test-megarepo-tool-sync-diamond-merge.t | 12 ++- .../integration/megarepo/test-megarepo-tool.t | 7 +- eden/mononoke/tools/admin/Cargo.toml | 2 + .../tools/admin/src/commands/megarepo.rs | 4 + .../admin/src/commands/megarepo/merge.rs} | 82 ++++++++++++++++++- 8 files changed, 102 insertions(+), 67 deletions(-) rename eden/mononoke/{commit_rewriting/megarepo/tool/merging.rs => tools/admin/src/commands/megarepo/merge.rs} (54%) diff --git a/eden/mononoke/commit_rewriting/megarepo/tool/cli.rs b/eden/mononoke/commit_rewriting/megarepo/tool/cli.rs index 68f40acf2e670..da39451afcba6 100644 --- a/eden/mononoke/commit_rewriting/megarepo/tool/cli.rs +++ b/eden/mononoke/commit_rewriting/megarepo/tool/cli.rs @@ -42,7 +42,6 @@ pub const DELETION_CHUNK_SIZE: &str = "deletion-chunk-size"; pub const DIFF_MAPPING_VERSIONS: &str = "diff-mapping-versions"; pub const DRY_RUN: &str = "dry-run"; pub const EVEN_CHUNK_SIZE: &str = "even-chunk-size"; -pub const FIRST_PARENT: &str = "first-parent"; pub const GRADUAL_MERGE_PROGRESS: &str = "gradual-merge-progress"; pub const GRADUAL_MERGE: &str = "gradual-merge"; pub const GRADUAL_DELETE: &str = "gradual-delete"; @@ -56,7 +55,6 @@ pub const MAPPING_VERSION_NAME: &str = "mapping-version-name"; pub const MARK_NOT_SYNCED_COMMAND: &str = "mark-not-synced"; pub const MARK_PUBLIC: &str = "mark-public"; pub const MAX_NUM_OF_MOVES_IN_COMMIT: &str = "max-num-of-moves-in-commit"; -pub const MERGE: &str = "merge"; pub const MOVE: &str = "move"; pub const ORIGIN_REPO: &str = "origin-repo"; pub const OVERWRITE: &str = "overwrite"; @@ -68,7 +66,6 @@ pub const PATHS_FILE: &str = "paths-file"; pub const PRE_DELETION_COMMIT: &str = "pre-deletion-commit"; pub const PRE_MERGE_DELETE: &str = "pre-merge-delete"; pub const RUN_MOVER: &str = "run-mover"; -pub const SECOND_PARENT: &str = "second-parent"; pub const SELECT_PARENTS_AUTOMATICALLY: &str = "select-parents-automatically"; pub const SOURCE_CHANGESET: &str = "source-changeset"; pub const SYNC_COMMIT_AND_ANCESTORS: &str = "sync-commit-and-ancestors"; @@ -248,21 +245,6 @@ pub fn setup_app<'a, 'b>() -> MononokeClapApp<'a, 'b> { .required(true), ); - let merge_subcommand = SubCommand::with_name(MERGE) - .about("create a merge commit with given parents") - .arg( - Arg::with_name(FIRST_PARENT) - .help("first parent of a produced merge commit") - .takes_value(true) - .required(true), - ) - .arg( - Arg::with_name(SECOND_PARENT) - .help("second parent of a produced merge commit") - .takes_value(true) - .required(true), - ); - let sync_diamond_subcommand = SubCommand::with_name(SYNC_DIAMOND_MERGE) .about("sync a diamond merge commit from a small repo into large repo") .arg( @@ -716,7 +698,6 @@ pub fn setup_app<'a, 'b>() -> MononokeClapApp<'a, 'b> { .with_source_and_target_repos() .build() .subcommand(add_resulting_commit_args(move_subcommand)) - .subcommand(add_resulting_commit_args(merge_subcommand)) .subcommand(sync_diamond_subcommand) .subcommand(add_light_resulting_commit_args(pre_merge_delete_subcommand)) .subcommand(history_fixup_delete_subcommand) diff --git a/eden/mononoke/commit_rewriting/megarepo/tool/main.rs b/eden/mononoke/commit_rewriting/megarepo/tool/main.rs index eed72446bb2f8..3c0f344eb08b7 100644 --- a/eden/mononoke/commit_rewriting/megarepo/tool/main.rs +++ b/eden/mononoke/commit_rewriting/megarepo/tool/main.rs @@ -135,7 +135,6 @@ use crate::cli::DELETION_CHUNK_SIZE; use crate::cli::DIFF_MAPPING_VERSIONS; use crate::cli::DRY_RUN; use crate::cli::EVEN_CHUNK_SIZE; -use crate::cli::FIRST_PARENT; use crate::cli::GRADUAL_DELETE; use crate::cli::GRADUAL_MERGE; use crate::cli::GRADUAL_MERGE_PROGRESS; @@ -148,7 +147,6 @@ use crate::cli::MANUAL_COMMIT_SYNC; use crate::cli::MAPPING_VERSION_NAME; use crate::cli::MARK_NOT_SYNCED_COMMAND; use crate::cli::MAX_NUM_OF_MOVES_IN_COMMIT; -use crate::cli::MERGE; use crate::cli::MOVE; use crate::cli::ORIGIN_REPO; use crate::cli::OVERWRITE; @@ -160,7 +158,6 @@ use crate::cli::PATH_REGEX; use crate::cli::PRE_DELETION_COMMIT; use crate::cli::PRE_MERGE_DELETE; use crate::cli::RUN_MOVER; -use crate::cli::SECOND_PARENT; use crate::cli::SELECT_PARENTS_AUTOMATICALLY; use crate::cli::SOURCE_CHANGESET; use crate::cli::SYNC_COMMIT_AND_ANCESTORS; @@ -169,13 +166,11 @@ use crate::cli::TARGET_CHANGESET; use crate::cli::TO_MERGE_CS_ID; use crate::cli::VERSION; use crate::cli::WAIT_SECS; -use crate::merging::perform_merge; mod catchup; mod cli; mod gradual_merge; mod manual_commit_sync; -mod merging; mod sync_diamond_merge; #[derive(Clone)] @@ -273,37 +268,6 @@ async fn run_move<'a>( Ok(()) } -async fn run_merge<'a>( - ctx: &CoreContext, - matches: &MononokeMatches<'a>, - sub_m: &ArgMatches<'a>, -) -> Result<(), Error> { - let first_parent = sub_m.value_of(FIRST_PARENT).unwrap().to_owned(); - let second_parent = sub_m.value_of(SECOND_PARENT).unwrap().to_owned(); - let resulting_changeset_args = cs_args_from_matches(sub_m)?; - let repo = args::not_shardmanager_compatible::open_repo::( - ctx.fb, - &ctx.logger().clone(), - matches, - ) - .await?; - - let first_parent_fut = helpers::csid_resolve(ctx, &repo, first_parent); - let second_parent_fut = helpers::csid_resolve(ctx, &repo, second_parent); - let (first_parent, second_parent) = try_join(first_parent_fut, second_parent_fut).await?; - - info!(ctx.logger(), "Creating a merge commit"); - perform_merge( - ctx.clone(), - repo.clone(), - first_parent, - second_parent, - resulting_changeset_args, - ) - .await - .map(|_| ()) -} - async fn run_sync_diamond_merge<'a>( ctx: &CoreContext, matches: &MononokeMatches<'a>, @@ -1438,7 +1402,6 @@ fn main(fb: FacebookInit) -> Result<()> { (MARK_NOT_SYNCED_COMMAND, Some(sub_m)) => { run_mark_not_synced(ctx, &matches, sub_m).await } - (MERGE, Some(sub_m)) => run_merge(ctx, &matches, sub_m).await, (MOVE, Some(sub_m)) => run_move(ctx, &matches, sub_m).await, (RUN_MOVER, Some(sub_m)) => run_mover(ctx, &matches, sub_m).await, (SYNC_COMMIT_AND_ANCESTORS, Some(sub_m)) => { diff --git a/eden/mononoke/tests/integration/megarepo/test-megarepo-fixup-history.t b/eden/mononoke/tests/integration/megarepo/test-megarepo-fixup-history.t index b015fea1b8412..ef484108cae44 100644 --- a/eden/mononoke/tests/integration/megarepo/test-megarepo-fixup-history.t +++ b/eden/mononoke/tests/integration/megarepo/test-megarepo-fixup-history.t @@ -78,7 +78,11 @@ Start mononoke server f72c4b95a6f2e49b28c830406a0921e00621615b174cefee9f9e31c57346ac5a 4b4ada0c9b89b1d4679e18ddf2100d725d48721571363bbf527e78ab6dbf161d - $ REPOID=$FBS_REPOID megarepo_tool merge cee330c0c3ab8ee70923d9b750e8fb56579e3be4db9fb41a54b63578c975bc8a f72c4b95a6f2e49b28c830406a0921e00621615b174cefee9f9e31c57346ac5a author "history fixup" --mark-public --commit-date-rfc3339 "$COMMIT_DATE" --bookmark master_bookmark 2> /dev/null + $ REPOID=$FBS_REPOID mononoke_admin megarepo merge --repo-id 0 \ + > -i cee330c0c3ab8ee70923d9b750e8fb56579e3be4db9fb41a54b63578c975bc8a \ + > -i f72c4b95a6f2e49b28c830406a0921e00621615b174cefee9f9e31c57346ac5a \ + > -a author -m "history fixup" --mark-public --commit-date-rfc3339 "$COMMIT_DATE" \ + > --set-bookmark master_bookmark 2> /dev/null $ cd "$TESTTMP"/fbs-hg-cnt $ hg pull -q diff --git a/eden/mononoke/tests/integration/megarepo/test-megarepo-tool-sync-diamond-merge.t b/eden/mononoke/tests/integration/megarepo/test-megarepo-tool-sync-diamond-merge.t index 32a7ba2f23b94..dc77eb9eedf9b 100644 --- a/eden/mononoke/tests/integration/megarepo/test-megarepo-tool-sync-diamond-merge.t +++ b/eden/mononoke/tests/integration/megarepo/test-megarepo-tool-sync-diamond-merge.t @@ -59,13 +59,19 @@ blobimport hg servers repos into Mononoke repos $ export COMMIT_DATE="1985-09-04T00:00:00.00Z" move things in small repo with merge - $ megarepo_tool move 1 with_merge_master user "with merge move" --mark-public --commit-date-rfc3339 "$COMMIT_DATE" --bookmark with_merge_move --mapping-version-name TEST_VERSION_NAME &> /dev/null + $ megarepo_tool move 1 with_merge_master user "with merge move" --mark-public \ + > --commit-date-rfc3339 "$COMMIT_DATE" --bookmark with_merge_move \ + > --mapping-version-name TEST_VERSION_NAME &> /dev/null move things in another small repo - $ megarepo_tool move 2 another_master user "another move" --mark-public --commit-date-rfc3339 "$COMMIT_DATE" --bookmark another_move --mapping-version-name TEST_VERSION_NAME &> /dev/null + $ megarepo_tool move 2 another_master user "another move" --mark-public \ + > --commit-date-rfc3339 "$COMMIT_DATE" --bookmark another_move \ + > --mapping-version-name TEST_VERSION_NAME &> /dev/null merge things in both repos - $ megarepo_tool merge with_merge_move another_move user "megarepo merge" --mark-public --commit-date-rfc3339 "$COMMIT_DATE" --bookmark master_bookmark &> /dev/null + $ mononoke_admin megarepo merge --repo-id 0 -B with_merge_move -B another_move -a user \ + > -m "megarepo merge" --mark-public --commit-date-rfc3339 "$COMMIT_DATE" \ + > --set-bookmark master_bookmark &> /dev/null start mononoke server $ start_and_wait_for_mononoke_server diff --git a/eden/mononoke/tests/integration/megarepo/test-megarepo-tool.t b/eden/mononoke/tests/integration/megarepo/test-megarepo-tool.t index 040c80de8c115..ce1a3e676ecdd 100644 --- a/eden/mononoke/tests/integration/megarepo/test-megarepo-tool.t +++ b/eden/mononoke/tests/integration/megarepo/test-megarepo-tool.t @@ -110,10 +110,9 @@ move things in ovrsource in a stack * created 7 commits, with the last commit * (glob) merge things in both repos - $ megarepo_tool merge fbsource_move ovrsource_move user "megarepo merge" --mark-public --commit-date-rfc3339 "$COMMIT_DATE" --bookmark master_bookmark - * using repo "repo" repoid RepositoryId(0) (glob) - * changeset resolved as: * (glob) - * changeset resolved as: * (glob) + $ mononoke_admin megarepo merge --repo-id 0 -B fbsource_move \ + > -B ovrsource_move -a user -m "megarepo merge" --mark-public \ + > --commit-date-rfc3339 "$COMMIT_DATE" --set-bookmark master_bookmark * Creating a merge commit (glob) * Checking if there are any path conflicts (glob) * Done checking path conflicts (glob) diff --git a/eden/mononoke/tools/admin/Cargo.toml b/eden/mononoke/tools/admin/Cargo.toml index 458bab32dd73d..2549a16d558e2 100644 --- a/eden/mononoke/tools/admin/Cargo.toml +++ b/eden/mononoke/tools/admin/Cargo.toml @@ -73,6 +73,7 @@ justknobs = { version = "0.1.0", git = "https://github.com/facebookexperimental/ manifest = { version = "0.1.0", path = "../../manifest" } maplit = "1.0" megarepo_error = { version = "0.1.0", path = "../../megarepo_api/megarepo_error" } +megarepolib = { version = "0.1.0", path = "../../commit_rewriting/megarepo" } mercurial_derivation = { version = "0.1.0", path = "../../derived_data/mercurial_derivation" } mercurial_types = { version = "0.1.0", path = "../../mercurial/types" } metaconfig_types = { version = "0.1.0", path = "../../metaconfig/types" } @@ -121,6 +122,7 @@ walkdir = "2.3" [dev-dependencies] fbinit-tokio = { version = "0.1.2", git = "https://github.com/facebookexperimental/rust-shed.git", branch = "main" } +fixtures = { version = "0.1.0", path = "../../tests/fixtures" } mononoke_macros = { version = "0.1.0", path = "../../mononoke_macros" } test_repo_factory = { version = "0.1.0", path = "../../repo_factory/test_repo_factory" } tests_utils = { version = "0.1.0", path = "../../tests/utils" } diff --git a/eden/mononoke/tools/admin/src/commands/megarepo.rs b/eden/mononoke/tools/admin/src/commands/megarepo.rs index 6dd8e31fe207e..69be8b7a5aecf 100644 --- a/eden/mononoke/tools/admin/src/commands/megarepo.rs +++ b/eden/mononoke/tools/admin/src/commands/megarepo.rs @@ -5,6 +5,7 @@ * GNU General Public License version 2. */ +mod merge; mod pushredirection; use anyhow::Result; @@ -12,6 +13,7 @@ use clap::Parser; use clap::Subcommand; use mononoke_app::MononokeApp; +use self::merge::MergeArgs; use self::pushredirection::PushRedirectionArgs; /// Manage megarepo @@ -25,6 +27,7 @@ pub struct CommandArgs { enum MegarepoSubcommand { /// Manage which repos are pushredirected to the large repo PushRedirection(PushRedirectionArgs), + Merge(MergeArgs), } pub async fn run(app: MononokeApp, args: CommandArgs) -> Result<()> { @@ -32,6 +35,7 @@ pub async fn run(app: MononokeApp, args: CommandArgs) -> Result<()> { match args.subcommand { MegarepoSubcommand::PushRedirection(args) => pushredirection::run(&ctx, app, args).await?, + MegarepoSubcommand::Merge(args) => merge::run(&ctx, app, args).await?, } Ok(()) diff --git a/eden/mononoke/commit_rewriting/megarepo/tool/merging.rs b/eden/mononoke/tools/admin/src/commands/megarepo/merge.rs similarity index 54% rename from eden/mononoke/commit_rewriting/megarepo/tool/merging.rs rename to eden/mononoke/tools/admin/src/commands/megarepo/merge.rs index a614fb97d22a6..d6123a39a5873 100644 --- a/eden/mononoke/commit_rewriting/megarepo/tool/merging.rs +++ b/eden/mononoke/tools/admin/src/commands/megarepo/merge.rs @@ -5,21 +5,73 @@ * GNU General Public License version 2. */ +use anyhow::bail; use anyhow::format_err; use anyhow::Error; +use anyhow::Result; use bonsai_hg_mapping::BonsaiHgMappingRef; +use bookmarks::BookmarkKey; use cloned::cloned; use context::CoreContext; use futures::try_join; use megarepolib::common::create_save_and_generate_hg_changeset; -use megarepolib::common::ChangesetArgs; +use megarepolib::common::ChangesetArgs as MegarepoNewChangesetArgs; use megarepolib::working_copy::get_colliding_paths_between_commits; use mercurial_derivation::DeriveHgChangeset; use mercurial_types::HgChangesetId; +use mononoke_api::Repo; +use mononoke_app::args::ChangesetArgs; +use mononoke_app::args::RepoArgs; +use mononoke_app::MononokeApp; use mononoke_types::ChangesetId; +use mononoke_types::DateTime; use slog::info; -use crate::Repo; +/// Create a merge commit with given parents +#[derive(Debug, clap::Args)] +pub struct MergeArgs { + #[clap(flatten, help = "first and second parents of a produced merge commit")] + pub parents: ChangesetArgs, + #[clap(flatten)] + pub repo_args: RepoArgs, + + #[clap(long, short = 'm')] + pub commit_message: String, + #[clap(long, short = 'a')] + pub commit_author: String, + + #[clap(long = "commit-date-rfc3339")] + pub datetime: Option, + #[clap( + long, + help = "bookmark to point to resulting commits (no sanity checks, will move existing bookmark, be careful)" + )] + pub set_bookmark: Option, + + #[clap(long = "mark-public")] + pub mark_public: bool, +} + +impl TryInto for MergeArgs { + type Error = Error; + + fn try_into(self) -> Result { + let mb_datetime = self + .datetime + .as_deref() + .map_or_else(|| Ok(DateTime::now()), DateTime::from_rfc3339)?; + + let mb_bookmark = self.set_bookmark.map(BookmarkKey::new).transpose()?; + let res = MegarepoNewChangesetArgs { + message: self.commit_message, + author: self.commit_author, + datetime: mb_datetime, + bookmark: mb_bookmark, + mark_public: self.mark_public, + }; + Ok(res) + } +} async fn fail_on_path_conflicts( ctx: &CoreContext, @@ -50,7 +102,7 @@ pub async fn perform_merge( repo: Repo, first_bcs_id: ChangesetId, second_bcs_id: ChangesetId, - resulting_changeset_args: ChangesetArgs, + resulting_changeset_args: MegarepoNewChangesetArgs, ) -> Result { cloned!(ctx, repo); let (first_hg_cs_id, second_hg_cs_id) = try_join!( @@ -72,6 +124,30 @@ pub async fn perform_merge( .await } +pub async fn run(ctx: &CoreContext, app: MononokeApp, args: MergeArgs) -> Result<()> { + info!(ctx.logger(), "Creating a merge commit"); + + let repo: Repo = app.open_repo(&args.repo_args).await?; + + let parents = args.parents.resolve_changesets(ctx, &repo).await?; + let (first_parent, second_parent) = match parents[..] { + [first_parent, second_parent] => (first_parent, second_parent), + _ => bail!("Expected exactly two parent commits"), + }; + + let resulting_changeset_args = MegarepoNewChangesetArgs::from(args.try_into()?); + + perform_merge( + ctx.clone(), + repo.clone(), + first_parent, + second_parent, + resulting_changeset_args, + ) + .await + .map(|_| ()) +} + #[cfg(test)] mod test { use std::str::FromStr;