mirrors/jj
1
0
Fork 1
mirror of https://github.com/martinvonz/jj.git synced 2025-01-18 10:07:28 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

working_copy: don't visit whole ignored tree even if last in order

Browse source 
When committing the working copy, we try to not visit ignored
directories (as e.g. `target/` often is), but we need to visit it if
there are already tracked files in it. I initially missed that in
c1060610bd and then fixed it in a028f33e3b. The fix works by
checking if the next path after the ignored path is inside the ignore
path (viewed as a directory). However, I forgot to handle the case
where there are no paths at all after the ignored path. So, for
example, if the `target/` directory should be ignored and it there
were no tracked paths after `target/` in alphabetical order, we would
still visit the directory. That's why the bug reproduced in the
`git-branchless` repo but not in the `jj` repo (because there are
files under `testing/` and `tests/` here).

Closes #247.
This commit is contained in:
Martin von Zweigbergk 2022-04-26 15:43:37 -07:00 committed by Martin von Zweigbergk
parent 2d4e653e90
commit 0fbb1d3971
1 changed files with 24 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

39
lib/src/working_copy.rs
View file

@ -352,21 +352,12 @@ impl TreeState {
}
let sub_path = dir.join(&RepoPathComponent::from(name));
if file_type.is_dir() {
if git_ignore.matches_all_files_in(&sub_path.to_internal_dir_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(&RepoPathComponent::from("\0"));
if let Some((maybe_subdir_file, _)) = self
.file_states
.range((Bound::Included(&first_file_in_dir), Bound::Unbounded))
.next()
{
if !dir.contains(&maybe_subdir_file.parent().unwrap()) {
continue;
}
}
// If the whole directory is ignored, skip it unless we're already tracking
// some file in it.
if git_ignore.matches_all_files_in(&sub_path.to_internal_dir_string())
&& !self.has_files_under(&sub_path)
{
continue;
}
work.push((sub_path, entry.path(), git_ignore.clone()));
} else {
@ -391,6 +382,24 @@ impl TreeState {
self.tree_id.clone()
}
fn has_files_under(&self, dir: &RepoPath) -> bool {
// 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(&RepoPathComponent::from("\0"));
if let Some((subdir_file, _)) = self
.file_states
.range((Bound::Included(&first_file_in_dir), Bound::Unbounded))
.next()
{
dir.contains(subdir_file)
} else {
// There are no tracked paths at all after `dir/` in alphabetical order, so
// there are no paths under `dir/`.
false
}
}
fn update_file_state(
&mut self,
repo_path: RepoPath,