This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10548

dirstate-tree: Ignore FIFOs etc. in the status algorithm
ClosedPublic

Authored by SimonSapin on May 3 2021, 6:25 AM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHGaeb03758f37a: dirstate-tree: Ignore FIFOs etc. in the status algorithm
rHG26202768a9b5: dirstate-tree: Ignore FIFOs etc. in the status algorithm
Summary

If a filesystem directory contains anything that is not:

  • a "normal" file
  • a symbolic link
  • or a directory

… act as if that directory entry was not there. For example, if that path was
previously a tracked file, mark it as deleted or removed.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

SimonSapin created this revision.May 3 2021, 6:25 AM
Alphare added a subscriber: Alphare.May 3 2021, 10:39 AM

If the non-file, non-symlink entry is exactly matched, it should be reported as a warning as a "bad" match

SimonSapin added a comment.May 6 2021, 5:59 AM

Yes, tests/test-symlinks.t covers this:

[…]
  $ mkfifo a.c
  $ hg status
  M dir/b.o
  ! a.c
  ! dir/a.o
  ? .hgignore
  $ hg status a.c
  a.c: unsupported file type (type is fifo)
  ! a.c

And it passes when I run it with --extra-config-opt experimental.dirstate-tree.in-memory=1. Maybe this is taken care of when constructing the matcher?

Alphare added a comment.May 6 2021, 11:37 AM
In D10548#162022, @SimonSapin wrote:

And it passes when I run it with --extra-config-opt experimental.dirstate-tree.in-memory=1. Maybe this is taken care of when constructing the matcher?

I find it weird because the only place I see bad being appended to is if DirEntry::read_dir returns an IoError, which should not happen in the test-case you're quoting. Or am I missing something?

baymax updated this revision to Diff 27624.May 6 2021, 12:03 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

Alphare accepted this revision.May 6 2021, 12:14 PM

This most likely isn't going through Rust for this case. This can be handled in a follow-up changeset.

This revision is now accepted and ready to land.May 6 2021, 12:14 PM
SimonSapin added a comment.May 6 2021, 12:26 PM

Ah, I figured it out. The relevant command in the test creates a patternmatcher which is not in allowed_matchers, so dirstate.py falls back to Python status(), so the code in this patch does not end up being used.

… which is weird because I definitely remember this test failing with --extra-config-opt experimental.dirstate-tree.in-memory=1. Could the matcher type have changed between that and a rebase?

Alphare added a comment.May 6 2021, 12:35 PM
In D10548#162066, @SimonSapin wrote:

… which is weird because I definitely remember this test failing with --extra-config-opt experimental.dirstate-tree.in-memory=1. Could the matcher type have changed between that and a rebase?

I don't think so, I haven't seen anything that touched code around matches since you started this work.

Revision Contents
Changeset List

Diff 27658

rust/hg-core/src/dirstate_tree/status.rs

fn traverse_fs_and_dirstate( fn traverse_fs_and_dirstate(
&mut self, &mut self,
fs_entry: &DirEntry, fs_entry: &DirEntry,
hg_path: &'tree HgPath, hg_path: &'tree HgPath,
dirstate_node: &'tree mut Node, dirstate_node: &'tree mut Node,
has_ignored_ancestor: bool, has_ignored_ancestor: bool,
) { ) {
if fs_entry.metadata.is_dir() { let file_type = fs_entry.metadata.file_type();
if self.options.collect_traversed_dirs { let file_or_symlink = file_type.is_file() || file_type.is_symlink();
self.outcome.traversed.push(hg_path.into()) if !file_or_symlink {
}
// If we previously had a file here, it was removed (with // If we previously had a file here, it was removed (with
// `hg rm` or similar) or deleted before it could be // `hg rm` or similar) or deleted before it could be
// replaced by a directory. // replaced by a directory or something else.
self.mark_removed_or_deleted_if_file( self.mark_removed_or_deleted_if_file(
hg_path, hg_path,
dirstate_node.state(), dirstate_node.state(),
); );
}
if file_type.is_dir() {
if self.options.collect_traversed_dirs {
self.outcome.traversed.push(hg_path.into())
}
let is_ignored = has_ignored_ancestor || (self.ignore_fn)(hg_path); let is_ignored = has_ignored_ancestor || (self.ignore_fn)(hg_path);
let is_at_repo_root = false; let is_at_repo_root = false;
self.traverse_fs_directory_and_dirstate( self.traverse_fs_directory_and_dirstate(
is_ignored, is_ignored,
&mut dirstate_node.children, &mut dirstate_node.children,
hg_path, hg_path,
&fs_entry.full_path, &fs_entry.full_path,
is_at_repo_root, is_at_repo_root,
); );
} else { } else {
if self.matcher.matches(hg_path) { if file_or_symlink && self.matcher.matches(hg_path) {
let full_path = Cow::from(hg_path); let full_path = Cow::from(hg_path);
if let Some(entry) = &dirstate_node.entry { if let Some(entry) = &dirstate_node.entry {
match entry.state { match entry.state {
EntryState::Added => { EntryState::Added => {
self.outcome.added.push(full_path) self.outcome.added.push(full_path)
} }
EntryState::Removed => { EntryState::Removed => {
self.outcome.removed.push(full_path) self.outcome.removed.push(full_path)
/// Something in the filesystem has no corresponding dirstate node /// Something in the filesystem has no corresponding dirstate node
fn traverse_fs_only( fn traverse_fs_only(
&mut self, &mut self,
has_ignored_ancestor: bool, has_ignored_ancestor: bool,
directory_hg_path: &HgPath, directory_hg_path: &HgPath,
fs_entry: &DirEntry, fs_entry: &DirEntry,
) { ) {
let hg_path = directory_hg_path.join(&fs_entry.base_name); let hg_path = directory_hg_path.join(&fs_entry.base_name);
if fs_entry.metadata.is_dir() { let file_type = fs_entry.metadata.file_type();
let file_or_symlink = file_type.is_file() || file_type.is_symlink();
if file_type.is_dir() {
let is_ignored = let is_ignored =
has_ignored_ancestor || (self.ignore_fn)(&hg_path); has_ignored_ancestor || (self.ignore_fn)(&hg_path);
let traverse_children = if is_ignored { let traverse_children = if is_ignored {
// Descendants of an ignored directory are all ignored // Descendants of an ignored directory are all ignored
self.options.list_ignored self.options.list_ignored
} else { } else {
// Descendants of an unknown directory may be either unknown or // Descendants of an unknown directory may be either unknown or
// ignored // ignored
&hg_path, &hg_path,
&child_fs_entry, &child_fs_entry,
) )
} }
} }
if self.options.collect_traversed_dirs { if self.options.collect_traversed_dirs {
self.outcome.traversed.push(hg_path.into()) self.outcome.traversed.push(hg_path.into())
} }
} else if self.matcher.matches(&hg_path) { } else if file_or_symlink && self.matcher.matches(&hg_path) {
self.mark_unknown_or_ignored(has_ignored_ancestor, hg_path.into()) self.mark_unknown_or_ignored(has_ignored_ancestor, hg_path.into())
} }
} }
fn mark_unknown_or_ignored( fn mark_unknown_or_ignored(
&mut self, &mut self,
has_ignored_ancestor: bool, has_ignored_ancestor: bool,
hg_path: Cow<'tree, HgPath>, hg_path: Cow<'tree, HgPath>,