mononoke: make get_mover and get_reverse_mover functions deprecated

Summary:
get_mover() and get_reverse_mover() functions return the mover for the
"current" version of the commit sync config, which means these are movers for the version
of the config that's used to create the latest commits on master branch.

So this function returns correct mover only for the latest master commit, but
for all other commits it returns an incorrect mover! This is wrong and it
happened to work just by change, and that's why these functions are marked as deprecated
now, and later we'll add functions 'get_mover_by_version()' which could be used to
replace deprecated functions.

Note that the story for get_bookmark_renamer()/get_reverse_bookmark_renamer()
functions seems to be different. If we can always figure out what's the correct
mover for a commit by e.g. look at its parent we can't really do the same for
bookmarks. Because of that I suggest to keep using the current version for
get_bookmark_renamer() function.

Reviewed By: ikostia

Differential Revision: D23929582

fbshipit-source-id: 3e5e9b46224aca0b75cf2d981ea21c4f9a378ba9
This commit is contained in:
Stanislau Hlebik 2020-09-25 14:19:18 -07:00 committed by Facebook GitHub Bot
parent 092875e01d
commit 1063f8e44a
7 changed files with 16 additions and 12 deletions

View File

@ -510,7 +510,7 @@ async fn verify_mapping_and_all_wc(
) -> Result<(), Error> { ) -> Result<(), Error> {
let source_repo = commit_syncer.get_source_repo(); let source_repo = commit_syncer.get_source_repo();
let target_repo = commit_syncer.get_target_repo(); let target_repo = commit_syncer.get_target_repo();
let mover = commit_syncer.get_mover(&ctx)?; let mover = commit_syncer.get_current_mover_DEPRECATED(&ctx)?;
verify_bookmarks(ctx.clone(), commit_syncer.clone()).await?; verify_bookmarks(ctx.clone(), commit_syncer.clone()).await?;
@ -589,7 +589,7 @@ async fn verify_bookmarks(
) -> Result<(), Error> { ) -> Result<(), Error> {
let source_repo = commit_syncer.get_source_repo(); let source_repo = commit_syncer.get_source_repo();
let target_repo = commit_syncer.get_target_repo(); let target_repo = commit_syncer.get_target_repo();
let mover = commit_syncer.get_mover(&ctx)?; let mover = commit_syncer.get_current_mover_DEPRECATED(&ctx)?;
let bookmark_renamer = commit_syncer.get_bookmark_renamer(&ctx)?; let bookmark_renamer = commit_syncer.get_bookmark_renamer(&ctx)?;
let bookmarks = source_repo let bookmarks = source_repo

View File

@ -516,12 +516,16 @@ where
&self.mapping &self.mapping
} }
pub fn get_mover(&self, ctx: &CoreContext) -> Result<Mover, Error> { // TODO(stash): remove callers of this function
#[allow(non_snake_case)]
pub fn get_current_mover_DEPRECATED(&self, ctx: &CoreContext) -> Result<Mover, Error> {
let (_, _, mover) = self.get_source_target_mover(ctx)?; let (_, _, mover) = self.get_source_target_mover(ctx)?;
Ok(mover) Ok(mover)
} }
pub fn get_reverse_mover(&self, ctx: &CoreContext) -> Result<Mover, Error> { // TODO(stash): remove callers of this function
#[allow(non_snake_case)]
pub fn get_current_reverse_mover_DEPRECATED(&self, ctx: &CoreContext) -> Result<Mover, Error> {
let (source_repo, target_repo, version_name) = self.get_source_target_version(ctx)?; let (source_repo, target_repo, version_name) = self.get_source_target_version(ctx)?;
self.commit_sync_data_provider.get_reverse_mover( self.commit_sync_data_provider.get_reverse_mover(

View File

@ -55,8 +55,8 @@ pub async fn verify_working_copy<M: SyncedCommitMapping + Clone + 'static>(
Target::ref_cast(target_repo), Target::ref_cast(target_repo),
Source(source_hash), Source(source_hash),
Target(target_hash), Target(target_hash),
&commit_syncer.get_mover(&ctx)?, &commit_syncer.get_current_mover_DEPRECATED(&ctx)?,
&commit_syncer.get_reverse_mover(&ctx)?, &commit_syncer.get_current_reverse_mover_DEPRECATED(&ctx)?,
) )
.await .await
} }

View File

@ -76,7 +76,7 @@ where
ctx.clone(), ctx.clone(),
source_bcs_mut, source_bcs_mut,
&map, &map,
commit_syncer.get_mover(&ctx)?, commit_syncer.get_current_mover_DEPRECATED(&ctx)?,
source_repo.clone(), source_repo.clone(),
) )
.await .await

View File

@ -219,7 +219,7 @@ async fn create_rewritten_merge_commit(
ctx.clone(), ctx.clone(),
merge_bcs, merge_bcs,
&remapped_parents, &remapped_parents,
syncers.small_to_large.get_mover(&ctx)?, syncers.small_to_large.get_current_mover_DEPRECATED(&ctx)?,
syncers.small_to_large.get_source_repo().clone(), syncers.small_to_large.get_source_repo().clone(),
) )
.await?; .await?;
@ -262,7 +262,7 @@ async fn generate_additional_file_changes(
BonsaiDiffFileChange::Changed(ref path, ..) BonsaiDiffFileChange::Changed(ref path, ..)
| BonsaiDiffFileChange::ChangedReusedId(ref path, ..) | BonsaiDiffFileChange::ChangedReusedId(ref path, ..)
| BonsaiDiffFileChange::Deleted(ref path) => { | BonsaiDiffFileChange::Deleted(ref path) => {
let maybe_new_path = large_to_small.get_mover(&ctx)?(path)?; let maybe_new_path = large_to_small.get_current_mover_DEPRECATED(&ctx)?(path)?;
if maybe_new_path.is_some() { if maybe_new_path.is_some() {
continue; continue;
} }

View File

@ -921,7 +921,7 @@ async fn repo_import(
maybe_call_sign: call_sign.clone(), maybe_call_sign: call_sign.clone(),
}); });
movers.push(syncers.small_to_large.get_mover(&ctx)?); movers.push(syncers.small_to_large.get_current_mover_DEPRECATED(&ctx)?);
repo_import_setting = large_repo_import_setting; repo_import_setting = large_repo_import_setting;
repo = large_repo; repo = large_repo;
repo_config = large_repo_config; repo_config = large_repo_config;

View File

@ -666,7 +666,7 @@ mod tests {
DefaultAction::PrependPrefix(mp("dest_path_prefix")), DefaultAction::PrependPrefix(mp("dest_path_prefix")),
)?; )?;
movers.push(importing_mover); movers.push(importing_mover);
movers.push(syncers.small_to_large.get_mover(&ctx)?); movers.push(syncers.small_to_large.get_current_mover_DEPRECATED(&ctx)?);
let combined_mover: Mover = Arc::new(move |source_path: &MPath| { let combined_mover: Mover = Arc::new(move |source_path: &MPath| {
let mut mutable_path = source_path.clone(); let mut mutable_path = source_path.clone();
@ -817,7 +817,7 @@ mod tests {
DefaultAction::PrependPrefix(mp("dest_path_prefix")), DefaultAction::PrependPrefix(mp("dest_path_prefix")),
)?; )?;
movers.push(importing_mover); movers.push(importing_mover);
movers.push(syncers.small_to_large.get_mover(&ctx)?); movers.push(syncers.small_to_large.get_current_mover_DEPRECATED(&ctx)?);
let combined_mover: Mover = Arc::new(move |source_path: &MPath| { let combined_mover: Mover = Arc::new(move |source_path: &MPath| {
let mut mutable_path = source_path.clone(); let mut mutable_path = source_path.clone();