diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fca72b5cf4..820a94cf6cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,6 +72,11 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). contains pack files. [#8661](https://github.com/jj-vcs/jj/issues/8661) +* `.gitignore` files are now respected even if they aren't materialized in the + working copy because they are excluded by the sparse patterns. Previously, + ignored files could become tracked in a sparse working copy. + [#2289](https://github.com/jj-vcs/jj/issues/2289) + ## [0.45.1] - 2026-09-03 This release fixes an error that prevented the new jj-core crate from being diff --git a/lib/src/local_working_copy.rs b/lib/src/local_working_copy.rs index 33950f00560..597f76f344c 100644 --- a/lib/src/local_working_copy.rs +++ b/lib/src/local_working_copy.rs @@ -1342,6 +1342,7 @@ impl TreeState { tree_state: self, current_tree: &self.tree, matcher: &matcher, + sparse_matcher: sparse_matcher.as_ref(), start_tracking_matcher, force_tracking_matcher, // Move tx sides so they'll be dropped at the end of the scope. @@ -1531,6 +1532,7 @@ struct FileSnapshotter<'a> { tree_state: &'a TreeState, current_tree: &'a MergedTree, matcher: &'a dyn Matcher, + sparse_matcher: &'a dyn Matcher, start_tracking_matcher: &'a dyn Matcher, force_tracking_matcher: &'a dyn Matcher, tree_entries_tx: Sender<(RepoPathBuf, MergedTreeValue)>, @@ -1581,7 +1583,9 @@ impl FileSnapshotter<'_> { file_states, } = directory_to_visit; - let git_ignore = git_ignore.chain_with_file(&dir, disk_dir.join(".gitignore"))?; + let git_ignore = self + .chain_gitignore(&dir, &disk_dir, &git_ignore) + .block_on()?; let dir_entries: Vec<_> = disk_dir .read_dir() .and_then(|entries| entries.try_collect()) @@ -1610,6 +1614,53 @@ impl FileSnapshotter<'_> { Ok(()) } + /// Loads the `.gitignore` file of `dir` and chains it to the ignore rules + /// inherited from the parent directories. + /// + /// If the file isn't materialized on disk because it is excluded by the + /// sparse patterns, it is read from the current tree instead. Otherwise, a + /// sparse working copy would start tracking files that the user has + /// explicitly ignored. + async fn chain_gitignore( + &self, + dir: &RepoPath, + disk_dir: &Path, + git_ignore: &Arc, + ) -> Result, SnapshotError> { + let ignore_disk_path = disk_dir.join(".gitignore"); + if ignore_disk_path.is_file() { + return Ok(git_ignore.chain_with_file(dir, ignore_disk_path)?); + } + let repo_ignore_path = dir.join(RepoPathComponent::new(".gitignore").unwrap()); + if self.sparse_matcher.matches(&repo_ignore_path) { + // The file should have been materialized, so its absence means it + // was deleted from the working copy. + return Ok(git_ignore.clone()); + } + let tree_values = self.current_tree.path_value(&repo_ignore_path).await?; + let file_id = match tree_values.as_normal() { + Some(TreeValue::File { id, .. }) => id, + None + | Some(TreeValue::Symlink(_) | TreeValue::GitSubmodule(_) | TreeValue::Tree(_)) => { + // Conflicted .gitignore files are ignored as we won't be able to read them normally. + // Git does not follow symlinks when reading .gitignore files, so we ignore them too. + // Submodules and directories are ignored as they can't be .gitignore files either. + return Ok(git_ignore.clone()); + } + }; + let mut buf = vec![]; + let mut reader = self.store().read_file(&repo_ignore_path, file_id).await?; + reader + .read_to_end(&mut buf) + .await + .map_err(|err| BackendError::ReadFile { + path: repo_ignore_path.clone(), + id: file_id.clone(), + source: err.into(), + })?; + Ok(git_ignore.chain(dir, &ignore_disk_path, &buf)?) + } + async fn process_dir_entry<'scope>( &'scope self, dir: &RepoPath, diff --git a/lib/tests/test_local_working_copy_sparse.rs b/lib/tests/test_local_working_copy_sparse.rs index da07c675e1b..78af21b8058 100644 --- a/lib/tests/test_local_working_copy_sparse.rs +++ b/lib/tests/test_local_working_copy_sparse.rs @@ -26,6 +26,7 @@ use testutils::TestResult; use testutils::TestWorkspace; use testutils::commit_with_tree; use testutils::create_tree; +use testutils::create_tree_with; use testutils::repo_path; fn to_owned_path_vec(paths: &[&RepoPath]) -> Vec { @@ -327,3 +328,145 @@ fn test_sparse_commit_gitignore() -> TestResult { assert_eq!(entries[0].0.as_ref(), dir1_file2_path); Ok(()) } + +/// Test that tracked .gitignore files are respected even if they aren't +/// materialized in the working copy because of the sparse patterns. +/// https://github.com/jj-vcs/jj/issues/2289 +#[test] +fn test_sparse_commit_gitignore_sparsed_away() -> TestResult { + let mut test_workspace = TestWorkspace::init(); + let repo = &test_workspace.repo; + let working_copy_path = test_workspace.workspace.workspace_root().to_owned(); + + let root_gitignore_path = repo_path(".gitignore"); + let dir1_gitignore_path = repo_path("dir1/.gitignore"); + let dir1_subdir1_path = repo_path("dir1/subdir1"); + let dir1_subdir1_file1_path = repo_path("dir1/subdir1/file1"); + let dir1_subdir1_file2_path = repo_path("dir1/subdir1/file2"); + let dir1_subdir1_file3_path = repo_path("dir1/subdir1/file3"); + + let tree = create_tree( + repo, + &[ + (root_gitignore_path, "file1\n"), + (dir1_gitignore_path, "file2\n"), + ], + ); + let commit = commit_with_tree(repo.store(), tree); + test_workspace + .workspace + .check_out(repo.op_id().clone(), None, &commit) + .block_on()?; + + // Set sparse patterns to only dir1/subdir1/, so that both .gitignore files + // are removed from disk. + let mut locked_ws = test_workspace + .workspace + .start_working_copy_mutation() + .block_on()?; + let sparse_patterns = to_owned_path_vec(&[dir1_subdir1_path]); + locked_ws + .locked_wc() + .set_sparse_patterns(sparse_patterns) + .block_on()?; + locked_ws.finish(repo.op_id().clone()).block_on()?; + assert!( + !root_gitignore_path + .to_fs_path_unchecked(&working_copy_path) + .exists() + ); + assert!( + !dir1_gitignore_path + .to_fs_path_unchecked(&working_copy_path) + .exists() + ); + + std::fs::create_dir_all(dir1_subdir1_path.to_fs_path_unchecked(&working_copy_path))?; + for path in [ + dir1_subdir1_file1_path, + dir1_subdir1_file2_path, + dir1_subdir1_file3_path, + ] { + std::fs::write(path.to_fs_path_unchecked(&working_copy_path), "contents")?; + } + + // file1 is ignored by the root .gitignore and file2 by dir1/.gitignore, so + // only file3 should be tracked (in addition to the sparsed-away files.) + let modified_tree = test_workspace.snapshot()?; + let entries = modified_tree.entries().map(|(path, _)| path).collect_vec(); + assert_eq!( + entries.iter().map(AsRef::as_ref).collect_vec(), + vec![ + root_gitignore_path, + dir1_gitignore_path, + dir1_subdir1_file3_path, + ] + ); + Ok(()) +} + +/// A symlinked `.gitignore` read from the tree must not contribute any +/// patterns. Git doesn't follow such symlinks either, so that the rules +/// don't depend on whether the file is read from the filesystem or from a +/// tree. See the "NOTES" section of gitignore(5). +#[test] +fn test_sparse_commit_gitignore_symlink_not_followed() -> TestResult { + let mut test_workspace = TestWorkspace::init(); + let repo = &test_workspace.repo; + let working_copy_path = test_workspace.workspace.workspace_root().to_owned(); + + let dir1_gitignore_path = repo_path("dir1/.gitignore"); + let dir1_file1_path = repo_path("dir1/file1"); + let dir1_subdir1_path = repo_path("dir1/subdir1"); + let dir1_subdir1_file1_path = repo_path("dir1/subdir1/file1"); + + // `dir1/.gitignore` is a symlink pointing at `file1`. Both the symlink + // target itself and the contents of `dir1/file1` would ignore + // `dir1/subdir1/file1` if either were incorrectly used as patterns. + let tree = create_tree_with(repo, |builder| { + builder.file(dir1_file1_path, "file1\n"); + builder.symlink(dir1_gitignore_path, "file1"); + }); + let commit = commit_with_tree(repo.store(), tree); + test_workspace + .workspace + .check_out(repo.op_id().clone(), None, &commit) + .block_on()?; + + // Set sparse patterns to only dir1/subdir1/, so that the symlinked + // .gitignore has to be read from the tree. + let mut locked_ws = test_workspace + .workspace + .start_working_copy_mutation() + .block_on()?; + let sparse_patterns = to_owned_path_vec(&[dir1_subdir1_path]); + locked_ws + .locked_wc() + .set_sparse_patterns(sparse_patterns) + .block_on()?; + locked_ws.finish(repo.op_id().clone()).block_on()?; + assert!( + !dir1_gitignore_path + .to_fs_path_unchecked(&working_copy_path) + .exists() + ); + + std::fs::create_dir_all(dir1_subdir1_path.to_fs_path_unchecked(&working_copy_path))?; + std::fs::write( + dir1_subdir1_file1_path.to_fs_path_unchecked(&working_copy_path), + "contents", + )?; + + // The symlink contributes no patterns, so the new file is tracked. + let modified_tree = test_workspace.snapshot()?; + let entries = modified_tree.entries().map(|(path, _)| path).collect_vec(); + assert_eq!( + entries.iter().map(|path| path.as_ref()).collect_vec(), + vec![ + dir1_gitignore_path, + dir1_file1_path, + dir1_subdir1_file1_path, + ] + ); + Ok(()) +}