git: do not delete or track git submodules.

A new FileType, GitSubmodule is added which is ignored. Files or
directories having this type are not added to the work queue and
are ignored in snapshot. Submodules are not created by jujutsu
when resetting or checking out a tree, they should be currently
managed using git.

CHANGELOG.md

@@ -97,6 +97,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 * `jj git import` no longer crashes when all Git refs are removed.
 
+* Git submodules are now ignored completely. Earlier, files present in the
+  submodule directory in the working copy would become added (tracked), and
+  later removed if you checked out another commit. You can now use `git` to
+  populate the submodule directory and `jj` will leave it alone.
+
 ### Contributors
 
 Thanks to the people who made this release happen!
@@ -109,6 +114,7 @@ Thanks to the people who made this release happen!
  * Ruben Slabbert (@rslabbert)
  * Waleed Khan (@arxanas)
  * Sean E. Russell (@xxxserxxx)
+ * Pranay Sashank (@pranaysashank)
 
 
 ## [0.5.1] - 2022-10-17

lib/src/protos/working_copy.proto

@@ -21,6 +21,7 @@ enum FileType {
   Symlink = 1;
   Executable = 2;
   Conflict = 3;
+  GitSubmodule = 4;
 }
 
 message FileState {

lib/src/working_copy.rs

@@ -48,6 +48,7 @@ use crate::tree_builder::TreeBuilder;
 pub enum FileType {
     Normal { executable: bool },
     Symlink,
+    GitSubmodule,
     Conflict { id: ConflictId },
 }
 
@@ -89,6 +90,14 @@ impl FileState {
         }
     }
 
+    fn for_gitsubmodule() -> Self {
+        FileState {
+            file_type: FileType::GitSubmodule,
+            mtime: MillisSinceEpoch(0),
+            size: 0,
+        }
+    }
+
     #[cfg_attr(unix, allow(dead_code))]
     fn is_executable(&self) -> bool {
         if let FileType::Normal { executable } = &self.file_type {
@@ -125,6 +134,7 @@ fn file_state_from_proto(proto: &crate::protos::working_copy::FileState) -> File
             let id = ConflictId::new(proto.conflict_id.to_vec());
             FileType::Conflict { id }
         }
+        crate::protos::working_copy::FileType::GitSubmodule => FileType::GitSubmodule,
     };
     FileState {
         file_type,
@@ -143,6 +153,7 @@ fn file_state_to_proto(file_state: &FileState) -> crate::protos::working_copy::F
             proto.conflict_id = id.to_bytes();
             crate::protos::working_copy::FileType::Conflict
         }
+        FileType::GitSubmodule => crate::protos::working_copy::FileType::GitSubmodule,
     };
     proto.file_type = EnumOrUnknown::new(file_type);
     proto.mtime_millis_since_epoch = file_state.mtime.0;
@@ -468,8 +479,19 @@ impl TreeState {
             self.working_copy_path.clone(),
             base_ignores,
         )];
+
         let mut tree_builder = self.store.tree_builder(self.tree_id.clone());
-        let mut deleted_files: HashSet<_> = self.file_states.keys().cloned().collect();
+        let mut deleted_files: HashSet<_> = self
+            .file_states
+            .iter()
+            .filter_map(|(path, state)| {
+                if state.file_type != FileType::GitSubmodule {
+                    Some(path.clone())
+                } else {
+                    None
+                }
+            })
+            .collect();
         while let Some((dir, disk_dir, git_ignore)) = work.pop() {
             if sparse_matcher.visit(&dir).is_nothing() {
                 continue;
@@ -489,6 +511,12 @@ impl TreeState {
                     continue;
                 }
                 let sub_path = dir.join(&RepoPathComponent::from(name));
+                if let Some(file_state) = self.file_states.get(&sub_path) {
+                    if file_state.file_type == FileType::GitSubmodule {
+                        continue;
+                    }
+                }
+
                 if file_type.is_dir() {
                     // If the whole directory is ignored, skip it unless we're already tracking
                     // some file in it.
@@ -555,6 +583,7 @@ impl TreeState {
             // ignore it.
             return Ok(());
         }
+
         let disk_path = dir_entry.path();
         let metadata = dir_entry.metadata().map_err(|err| SnapshotError::IoError {
             message: format!("Failed to stat file {}", disk_path.display()),
@@ -661,6 +690,7 @@ impl TreeState {
                 Ok(TreeValue::Symlink(id))
             }
             FileType::Conflict { .. } => panic!("conflicts should be handled by the caller"),
+            FileType::GitSubmodule => panic!("git submodule cannot be written to store"),
         }
     }
 
@@ -858,7 +888,7 @@ impl TreeState {
                         TreeValue::Conflict(id) => self.write_conflict(&disk_path, &path, &id)?,
                         TreeValue::GitSubmodule(_id) => {
                             println!("ignoring git submodule at {:?}", path);
-                            return Ok(());
+                            FileState::for_gitsubmodule()
                         }
                         TreeValue::Tree(_id) => {
                             panic!("unexpected tree entry in diff at {:?}", path);
@@ -895,8 +925,7 @@ impl TreeState {
                         }
                         (_, TreeValue::GitSubmodule(_id)) => {
                             println!("ignoring git submodule at {:?}", path);
-                            self.file_states.remove(&path);
-                            return Ok(());
+                            FileState::for_gitsubmodule()
                         }
                         (_, TreeValue::Tree(_id)) => {
                             panic!("unexpected tree entry in diff at {:?}", path);
@@ -937,7 +966,7 @@ impl TreeState {
                         TreeValue::Conflict(id) => FileType::Conflict { id },
                         TreeValue::GitSubmodule(_id) => {
                             println!("ignoring git submodule at {:?}", path);
-                            continue;
+                            FileType::GitSubmodule
                         }
                         TreeValue::Tree(_id) => {
                             panic!("unexpected tree entry in diff at {:?}", path);

lib/tests/test_working_copy.rs

@@ -69,7 +69,7 @@ fn test_checkout_file_transitions(use_git: bool) {
     let store = repo.store().clone();
     let workspace_root = test_workspace.workspace.workspace_root().clone();
 
-    #[derive(Debug, Clone, Copy)]
+    #[derive(Debug, PartialEq, Eq, Clone, Copy)]
     enum Kind {
         Missing,
         Normal,
@@ -669,6 +669,71 @@ fn test_dotgit_ignored(use_git: bool) {
     locked_wc.discard();
 }
 
+#[test]
+fn test_gitsubmodule() {
+    // Tests that git submodules are ignored.
+
+    let settings = testutils::user_settings();
+    let mut test_workspace = TestWorkspace::init(&settings, true);
+    let repo = &test_workspace.repo;
+    let store = repo.store().clone();
+    let workspace_root = test_workspace.workspace.workspace_root().clone();
+
+    let mut tree_builder = store.tree_builder(store.empty_tree_id().clone());
+
+    let added_path = RepoPath::from_internal_string("added");
+    let submodule_path = RepoPath::from_internal_string("submodule");
+    let added_submodule_path = RepoPath::from_internal_string("submodule/added");
+
+    tree_builder.set(
+        added_path.clone(),
+        TreeValue::File {
+            id: testutils::write_file(repo.store(), &added_path, "added\n"),
+            executable: false,
+        },
+    );
+
+    let mut tx = repo.start_transaction(&settings, "create submodule commit");
+    let submodule_id = create_random_commit(&settings, repo)
+        .write_to_repo(tx.mut_repo())
+        .id()
+        .clone();
+    tx.commit();
+
+    tree_builder.set(
+        submodule_path.clone(),
+        TreeValue::GitSubmodule(submodule_id),
+    );
+
+    let tree_id = tree_builder.write_tree();
+    let tree = store.get_tree(&RepoPath::root(), &tree_id).unwrap();
+    let wc = test_workspace.workspace.working_copy_mut();
+    wc.check_out(repo.op_id().clone(), None, &tree).unwrap();
+
+    std::fs::create_dir(submodule_path.to_fs_path(&workspace_root)).unwrap();
+
+    testutils::write_working_copy_file(
+        &workspace_root,
+        &added_submodule_path,
+        "i am a file in a submodule\n",
+    );
+
+    // Check that the files present in the submodule are not tracked
+    // when we snapshot
+    let mut locked_wc = wc.start_mutation();
+    let new_tree_id = locked_wc.snapshot(GitIgnoreFile::empty()).unwrap();
+    locked_wc.discard();
+    assert_eq!(new_tree_id, tree_id);
+
+    // Check that the files in the submodule are not deleted
+    let file_in_submodule_path = added_submodule_path.to_fs_path(&workspace_root);
+    assert!(
+        file_in_submodule_path.metadata().is_ok(),
+        "{:?} should exist",
+        file_in_submodule_path
+    );
+}
+
 #[cfg(unix)]
 #[test_case(false ; "local backend")]
 #[test_case(true ; "git backend")]