From a028f33e3b21401f894ec996b6ba45998a92df23 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Sat, 15 May 2021 08:27:28 -0700
Subject: [PATCH] working copy: don't remove already-tracked files in ignored
 directories

The recent optimization to not walk ignored directories did not
account for the case where there already are files in the ignored
directory.
---
 lib/src/working_copy.rs        | 25 ++++++++++++++++++---
 lib/tests/test_working_copy.rs | 40 ++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/lib/src/working_copy.rs b/lib/src/working_copy.rs
index 313989ca4..48f96e290 100644
--- a/lib/src/working_copy.rs
+++ b/lib/src/working_copy.rs
@@ -18,6 +18,7 @@ use std::convert::TryInto;
 use std::fs;
 use std::fs::{File, OpenOptions};
 use std::io::Read;
+use std::ops::Bound;
 #[cfg(unix)]
 use std::os::unix::fs::symlink;
 #[cfg(unix)]
@@ -344,10 +345,28 @@ impl TreeState {
                 }
                 if file_type.is_dir() {
                     let subdir = dir.join(&DirRepoPathComponent::from(name));
-                    if !git_ignore.matches_all_files_in(&subdir.to_internal_string()) {
-                        let disk_subdir = disk_dir.join(file_name);
-                        work.push((subdir, disk_subdir, git_ignore.clone()));
+                    if git_ignore.matches_all_files_in(&subdir.to_internal_string()) {
+                        // If the whole directory is ignored, skip it unless we're already tracking
+                        // some file in it. TODO: This is pretty ugly... Also, we should
+                        // optimize it to check exactly the already-tracked files (we know that
+                        // we won't have to consider new files in the directory).
+                        let first_file_in_dir = dir.join(&FileRepoPathComponent::from("\0"));
+                        if let Some((maybe_subdir_file, _)) = self
+                            .file_states
+                            .range((Bound::Included(&first_file_in_dir), Bound::Unbounded))
+                            .next()
+                        {
+                            if !maybe_subdir_file
+                                .dir()
+                                .components()
+                                .starts_with(dir.components())
+                            {
+                                continue;
+                            }
+                        }
                     }
+                    let disk_subdir = disk_dir.join(file_name);
+                    work.push((subdir, disk_subdir, git_ignore.clone()));
                 } else {
                     let file = dir.join(&FileRepoPathComponent::from(name));
                     let disk_file = disk_dir.join(file_name);
diff --git a/lib/tests/test_working_copy.rs b/lib/tests/test_working_copy.rs
index 23ed738f3..3a3e288f4 100644
--- a/lib/tests/test_working_copy.rs
+++ b/lib/tests/test_working_copy.rs
@@ -386,3 +386,43 @@ fn test_gitignores_checkout_overwrites_ignored(use_git: bool) {
     let (_repo, new_commit) = repo.working_copy_locked().commit(&settings, repo.clone());
     assert!(new_commit.tree().entry("modified").is_some());
 }
+
+#[test_case(false ; "local store")]
+#[test_case(true ; "git store")]
+fn test_gitignores_ignored_directory_already_tracked(use_git: bool) {
+    // Tests that a .gitignore'd directory that already has a tracked file in it
+    // does not get removed when committing the working directory.
+
+    let _home_dir = testutils::new_user_home();
+    let settings = testutils::user_settings();
+    let (_temp_dir, repo) = testutils::init_repo(&settings, use_git);
+
+    // Add a .gitignore file saying to ignore the directory "ignored/"
+    let gitignore_path = FileRepoPath::from(".gitignore");
+    testutils::write_working_copy_file(&repo, &gitignore_path, "/ignored/\n");
+    let file_path = FileRepoPath::from("ignored/file");
+
+    // Create a commit that adds a file in the ignored directory
+    let mut tx = repo.start_transaction("test");
+    let mut tree_builder = repo
+        .store()
+        .tree_builder(repo.store().empty_tree_id().clone());
+    testutils::write_normal_file(&mut tree_builder, &file_path, "contents");
+    let tree_id = tree_builder.write_tree();
+    let commit = CommitBuilder::for_new_commit(&settings, repo.store(), tree_id)
+        .set_open(true)
+        .set_description("add ignored file".to_string())
+        .write_to_repo(tx.mut_repo());
+    let repo = tx.commit();
+
+    // Check out the commit with the file in ignored/
+    repo.working_copy_locked().check_out(commit).unwrap();
+
+    // Check that the file is still in the commit created by committing the working
+    // copy (that it didn't get removed because the directory is ignored)
+    let (_repo, new_commit) = repo.working_copy_locked().commit(&settings, repo.clone());
+    assert!(new_commit
+        .tree()
+        .path_value(&file_path.to_repo_path())
+        .is_some());
+}