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.

Download Raw Diff

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

Branch

default

Lint

No Linters Available

Unit

No Unit Test Coverage

Event Timeline

SimonSapin created this revision.May 3 2021, 6:25 AM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptMay 3 2021, 6:25 AM

Herald added a subscriber: mercurial-patches. · View Herald Transcript

SimonSapin added a child revision: D10549: dirstate-tree: Handle I/O errors in status.May 3 2021, 6:25 AM

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

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?

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?

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

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

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?

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.

SimonSapin added a commit: rHG26202768a9b5: dirstate-tree: Ignore FIFOs etc. in the status algorithm.May 6 2021, 12:54 PM

Closed by commit rHG26202768a9b5: dirstate-tree: Ignore FIFOs etc. in the status algorithm (authored by SimonSapin). · Explain Why

This revision was automatically updated to reflect the committed changes.

SimonSapin added a commit: rHGaeb03758f37a: dirstate-tree: Ignore FIFOs etc. in the status algorithm.May 6 2021, 3:42 PM

Revision Contents
Changeset List

			Path	Packages
M			rust/hg-core/src/dirstate_tree/status.rs (22 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	27323		May 3 2021, 6:25 AM	★	★
Diff 2	27624		May 6 2021, 12:03 PM	★	★
Diff 3	27658	rHG26202768a9b58fa2d032a994b599797540e23efe	Apr 26 2021, 1:16 PM	★	★

Commit	Parents	Author	Summary	Date
2193beeb702e	9b100e296e82	Simon Sapin		Apr 26 2021, 1:16 PM

Status	Author	Revision
Abandoned	SimonSapin	D10366 rust: Refactor parse_dirstate() to take a callback
Closed	SimonSapin	D10560 dirstate-tree: Borrow paths from the "on disk" bytes
Closed	SimonSapin	D10559 dirstate-tree: Borrow copy source paths from the "on disk" bytes
Closed	SimonSapin	D10558 rust: Use `&HgPath` instead of `&HgPathBuf` in may APIs
Closed	SimonSapin	D10557 dirstate-tree: Make `DirstateMap` borrow from a bytes buffer
Closed	SimonSapin	D10556 rust: Read dirstate from disk in DirstateMap constructor
Closed	SimonSapin	D10555 rust: Remove handling of `parents` in `DirstateMap`
Closed	SimonSapin	D10554 dirstate-tree: Fold "tracked descendants" counter update in main walk
Closed	SimonSapin	D10553 dirstate-tree: Use HashMap instead of BTreeMap
Closed	SimonSapin	D10552 dirstate-tree: Add #[timed] attribute to `status` and `DirstateMap::read`
Closed	SimonSapin	D10551 dirstate-tree: Paralellize the status algorithm with Rayon
Closed	SimonSapin	D10550 dirstate-tree: Avoid BTreeMap double-lookup when inserting a dirstate entry
Closed	SimonSapin	D10549 dirstate-tree: Handle I/O errors in status
Closed	SimonSapin	D10548 dirstate-tree: Ignore FIFOs etc. in the status algorithm
Closed	SimonSapin	D10547 dirstate-tree: Add the new `status()` algorithm
Closed	SimonSapin	D10546 dirstate-tree: Give to `status()` mutable access to the `DirstateMap`
Closed	SimonSapin	D10495 rust: Add doc-comments to DirstateStatus fields
Closed	SimonSapin	D10494 rust: Move "lookup" a.k.a. "unsure" paths into `DirstateStatus` struct
Closed	SimonSapin	D10493 rust: Remove DirstateMap::file_fold_map
Closed	SimonSapin	D10492 dirstate-tree: Add "non normal" and "from other parent" sets
Closed	SimonSapin	D10491 dirstate-tree: Add add_file, remove_file, and drop_file
Closed	SimonSapin	D10490 dirstate-tree: Add has_dir and has_tracked_dir
Closed	SimonSapin	D10489 dirstate-tree: Add clear_ambiguous_times in the new DirstateMap
Closed	SimonSapin	D10488 dirstate-tree: Add copy_map_insert and copy_map_remove
Closed	SimonSapin	D10487 dirstate-tree: Maintain a counter of DirstateEntry’s and copy sources
Closed	SimonSapin	D10486 dirstate-tree: Serialize to disk
Closed	SimonSapin	D10485 rust: Add a Timestamp struct instead of abusing Duration
Closed	SimonSapin	D10370 dirstate-tree: Add tree traversal/iteration
Closed	SimonSapin	D10369 dirstate-tree: Add map `get` and `contains_key` methods
Closed	SimonSapin	D10368 dirstate-tree: Add parsing only dirstate parents from disk
Closed	SimonSapin	D10367 dirstate-tree: Implement DirstateMap::read
Closed	SimonSapin	D10365 dirstate-tree: Add `WithBasename` wrapper for `HgPath`
Closed	SimonSapin	D10364 dirstate-tree: Empty shell for a second Rust DirstateMap implementation
Closed	SimonSapin	D10363 dirstate-tree: Abstract "non-normal" and "other parent" sets
Closed	SimonSapin	D10362 dirstate-tree: Make Rust DirstateMap bindings go through a trait object

Diff 27323

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>,