Use correct submodule action during backsync

Summary:
## This stack
See D57204338.

## This diff
As mentioned in D56826268 and D56881703, there's currently a bug in `get_git_submodule_action_by_version` (just renamed from `get_strip_git_submodules_by_version`) that leads using the default submodule action (i.e. STRIP) when it's called during backsyncing (large to small).

Luckily this led to a behaviour very close to what we're looking for **now** while backsyncing submodule changes is not supported:
1. If the commit doesn't touch submodule expansions, everything works normally.
2. If it does, it will sync it but the synced commit will be "broken". It will fail to derive fsnodes and other manifests.

We still want to keep #1, but we don't want #2. What we want right now is to **explicitly fail sync if someone tries to backsync any changes modifying submodule expansions**.

This diff implements this.

Reviewed By: mitrandir77

Differential Revision: D57151944

fbshipit-source-id: b4931d3cc58815b2beaeaae90de99dda15b5fbb9

eden/mononoke/commit_rewriting/cross_repo_sync/src/commit_sync_config_utils.rs

@@ -57,24 +57,20 @@ pub async fn get_git_submodule_action_by_version(
             .submodule_config
             .git_submodules_action
             .clone();
-        // TODO(T179530927): return small repo action instead of default one
-        if small_repo_action != GitSubmodulesChangesAction::default() {
-            log_warning(
-                ctx,
-                format!(
-                    "Using default submodule action for backsyncing. Actual submodule action configured for small repo is {0:#?}",
-                    small_repo_action
-                ),
-            );
-        };
-    };
 
-    // TODO(T179530927): get the correct submodule action from small repo
-    // when call is made during backsyncing.
-    // Currently, when this is called for backsyncing, we look for the large
-    // repo in the small repo configs, don't find it and return the default
-    // action of STRIP.
+        return Ok(small_repo_action);
+    };
 
+    // If the action if not found above, there might be something wrong. We
+    // should still fallback to the default action to avoid ever syncing submodule
+    // file changes to the large repo, but let's at least log a warning.
+    log_warning(
+        ctx,
+        format!(
+            "Couldn't find git submodule action for source repo id {}, target repo id {} and commit sync version {}",
+            source_repo_id, target_repo_id, version,
+        ),
+    );
     Ok(GitSubmodulesChangesAction::default())
 }
 

eden/mononoke/commit_rewriting/cross_repo_sync/src/git_submodules/expand.rs

@@ -28,6 +28,7 @@ use either::Either::*;
 use futures::stream;
 use futures::stream::StreamExt;
 use futures::stream::TryStreamExt;
+use itertools::Itertools;
 use manifest::BonsaiDiffFileChange;
 use maplit::hashmap;
 use mononoke_types::BlobstoreValue;
@@ -47,6 +48,7 @@ use crate::commit_syncers_lib::mover_to_multi_mover;
 use crate::git_submodules::in_memory_repo::InMemoryRepo;
 use crate::git_submodules::utils::build_recursive_submodule_deps;
 use crate::git_submodules::utils::get_git_hash_from_submodule_file;
+use crate::git_submodules::utils::get_submodule_expansions_affected;
 use crate::git_submodules::utils::get_submodule_file_content_id;
 use crate::git_submodules::utils::get_submodule_repo;
 use crate::git_submodules::utils::get_x_repo_submodule_metadata_file_path;
@@ -120,10 +122,35 @@ pub async fn rewrite_commit_with_submodule_expansion<'a, R: Repo>(
     remapped_parents: &'a HashMap<ChangesetId, ChangesetId>,
     rewrite_opts: RewriteOpts,
 ) -> Result<Option<BonsaiChangesetMut>> {
-    ensure!(
-        source_repo.repo_identity().id() != *sm_exp_data.large_repo_id,
-        "Can't sync changes from large to small repo if small repo has submodule expansion enabled"
-    );
+    let is_forward_sync = source_repo.repo_identity().id() != *sm_exp_data.large_repo_id;
+    if !is_forward_sync {
+        // Backsyncing, so ensure that submodule expansions are not being modified
+        let submodules_affected =
+            get_submodule_expansions_affected(&sm_exp_data, &bonsai, mover.clone())?;
+
+        ensure!(
+            submodules_affected.is_empty(),
+            "Changeset can't be synced from large to small repo because it modifies the expansion of submodules: {0:#?}",
+            submodules_affected
+                .into_iter()
+                .map(|p| p.to_string())
+                .sorted()
+                .collect::<Vec<_>>(),
+        );
+
+        // No submodule expansions are being modified, so it's safe to backsync
+        return rewrite_commit(
+            ctx,
+            bonsai,
+            remapped_parents,
+            mover_to_multi_mover(mover.clone()),
+            source_repo,
+            None,
+            rewrite_opts,
+        )
+        .await
+        .context("Failed to create small repo bonsai");
+    };
 
     let new_bonsai = run_and_log_stats_to_scuba(
         ctx,

eden/mononoke/commit_rewriting/cross_repo_sync/src/git_submodules/utils.rs

@@ -22,21 +22,25 @@ use futures::stream::Stream;
 use futures::stream::StreamExt;
 use futures::stream::TryStreamExt;
 use git_types::ObjectKind;
+use itertools::Itertools;
 use manifest::bonsai_diff;
 use manifest::BonsaiDiffFileChange;
 use manifest::Entry;
 use manifest::ManifestOps;
 use mononoke_types::hash::GitSha1;
 use mononoke_types::hash::RichGitSha1;
+use mononoke_types::BonsaiChangesetMut;
 use mononoke_types::ChangesetId;
 use mononoke_types::ContentId;
 use mononoke_types::FileType;
 use mononoke_types::FsnodeId;
 use mononoke_types::MPathElement;
 use mononoke_types::NonRootMPath;
+use movers::Mover;
 use repo_blobstore::RepoBlobstoreArc;
 use repo_derived_data::RepoDerivedDataRef;
 
+use crate::git_submodules::expand::SubmoduleExpansionData;
 use crate::git_submodules::expand::SubmodulePath;
 use crate::git_submodules::in_memory_repo::InMemoryRepo;
 use crate::types::Repo;
@@ -308,3 +312,49 @@ pub(crate) fn build_recursive_submodule_deps<R: Repo>(
 
     rec_small_repo_deps
 }
+
+/// Returns the submodule expansions affected by a large repo changeset.
+///
+/// This could happen by directly modifying the submodule's expansion or its
+/// metadata file.
+pub(crate) fn get_submodule_expansions_affected<'a, R: Repo>(
+    sm_exp_data: &SubmoduleExpansionData<'a, R>,
+    // Bonsai from the large repo
+    bonsai: &BonsaiChangesetMut,
+    mover: Mover,
+) -> Result<Vec<NonRootMPath>> {
+    let submodules_affected = sm_exp_data
+        .submodule_deps
+        .iter()
+        .map(|(submodule_path, _)| {
+            // Get the submodule's metadata file path
+            let metadata_file_path = get_x_repo_submodule_metadata_file_path(
+                &SubmodulePath(submodule_path.clone()),
+                sm_exp_data.x_repo_submodule_metadata_file_prefix,
+            )?;
+
+            let submodule_expansion_changed = bonsai
+                .file_changes
+                .iter()
+                .map(|(p, _)| mover(p))
+                .filter_map(Result::transpose)
+                .process_results(|mut iter| {
+                    iter.any(|small_repo_path| {
+                        // File modified expansion directly
+                        submodule_path.is_prefix_of(&small_repo_path)
+                        // or the submodule's metadata file
+                        || small_repo_path == metadata_file_path
+                    })
+                })?;
+
+            if submodule_expansion_changed {
+                return anyhow::Ok(Some(submodule_path.clone()));
+            };
+
+            Ok(None)
+        })
+        .filter_map(Result::transpose)
+        .collect::<Result<Vec<_>>>()?;
+
+    Ok(submodules_affected)
+}