( )⚙ D8662 rust-status: improve documentation and readability

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8662

rust-status: improve documentation and readability
ClosedPublic

Authored by Alphare on Jun 25 2020, 5:02 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG470d306e616c: rust-status: improve documentation and readability
Summary

This patch is 2/3 in the series to improve the dirstate status code. It adds a
number of common type aliases to add more obvious semantics to function
signatures, improves/adds documentation where necessary and improves one or two
patterns to be more idiomatic.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

Alphare created this revision.Jun 25 2020, 5:02 AM
Alphare added a commit: rHG470d306e616c: rust-status: improve documentation and readability.Jul 15 2020, 12:13 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG470d306e616c: rust-status: improve documentation and readability (authored by Alphare). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mrust/hg-core/src/dirstate/status.rs (113 lines)

Diff 21709

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

/// Was explicitly matched but cannot be found/accessed /// Was explicitly matched but cannot be found/accessed
#[derive(Debug, Copy, Clone)] #[derive(Debug, Copy, Clone)]
pub enum BadMatch { pub enum BadMatch {
OsError(i32), OsError(i32),
BadType(BadType), BadType(BadType),
} }
/// Marker enum used to dispatch new status entries into the right collections. /// Enum used to dispatch new status entries into the right collections.
/// Is similar to `crate::EntryState`, but represents the transient state of /// Is similar to `crate::EntryState`, but represents the transient state of
/// entries during the lifetime of a command. /// entries during the lifetime of a command.
#[derive(Debug, Copy, Clone)] #[derive(Debug, Copy, Clone)]
pub enum Dispatch { pub enum Dispatch {
Unsure, Unsure,
Modified, Modified,
Added, Added,
Removed, Removed,
Deleted, Deleted,
Clean, Clean,
Unknown, Unknown,
Ignored, Ignored,
/// Empty dispatch, the file is not worth listing /// Empty dispatch, the file is not worth listing
None, None,
/// Was explicitly matched but cannot be found/accessed /// Was explicitly matched but cannot be found/accessed
Bad(BadMatch), Bad(BadMatch),
Directory { Directory {
/// True if the directory used to be a file in the dmap so we can say /// True if the directory used to be a file in the dmap so we can say
/// that it's been removed. /// that it's been removed.
was_file: bool, was_file: bool,
}, },
} }
type IoResult<T> = std::io::Result<T>; type IoResult<T> = std::io::Result<T>;
/// `Box<dyn Trait>` is syntactic sugar for `Box<dyn Trait, 'static>`, so add /// `Box<dyn Trait>` is syntactic sugar for `Box<dyn Trait, 'static>`, so add
/// an explicit lifetime here to not fight `'static` bounds "out of nowhere". /// an explicit lifetime here to not fight `'static` bounds "out of nowhere".
type IgnoreFnType<'a> = Box<dyn for<'r> Fn(&'r HgPath) -> bool + Sync + 'a>; type IgnoreFnType<'a> = Box<dyn for<'r> Fn(&'r HgPath) -> bool + Sync + 'a>;
/// We have a good mix of owned (from directory traversal) and borrowed (from
/// the dirstate/explicit) paths, this comes up a lot.
type HgPathCow<'a> = Cow<'a, HgPath>;
/// A path with its computed ``Dispatch`` information
type DispatchedPath<'a> = (HgPathCow<'a>, Dispatch);
/// Dates and times that are outside the 31-bit signed range are compared /// Dates and times that are outside the 31-bit signed range are compared
/// modulo 2^31. This should prevent hg from behaving badly with very large /// modulo 2^31. This should prevent hg from behaving badly with very large
/// files or corrupt dates while still having a high probability of detecting /// files or corrupt dates while still having a high probability of detecting
/// changes. (issue2608) /// changes. (issue2608)
/// TODO I haven't found a way of having `b` be `Into<i32>`, since `From<u64>` /// TODO I haven't found a way of having `b` be `Into<i32>`, since `From<u64>`
/// is not defined for `i32`, and there is no `As` trait. This forces the /// is not defined for `i32`, and there is no `As` trait. This forces the
/// caller to cast `b` as `i32`. /// caller to cast `b` as `i32`.
fn mod_compare(a: i32, b: i32) -> bool { fn mod_compare(a: i32, b: i32) -> bool {
pub list_ignored: bool, pub list_ignored: bool,
/// Whether to collect traversed dirs for applying a callback later. /// Whether to collect traversed dirs for applying a callback later.
/// Used by `hg purge` for example. /// Used by `hg purge` for example.
pub collect_traversed_dirs: bool, pub collect_traversed_dirs: bool,
} }
#[derive(Debug)] #[derive(Debug)]
pub struct DirstateStatus<'a> { pub struct DirstateStatus<'a> {
pub modified: Vec<Cow<'a, HgPath>>, pub modified: Vec<HgPathCow<'a>>,
pub added: Vec<Cow<'a, HgPath>>, pub added: Vec<HgPathCow<'a>>,
pub removed: Vec<Cow<'a, HgPath>>, pub removed: Vec<HgPathCow<'a>>,
pub deleted: Vec<Cow<'a, HgPath>>, pub deleted: Vec<HgPathCow<'a>>,
pub clean: Vec<Cow<'a, HgPath>>, pub clean: Vec<HgPathCow<'a>>,
pub ignored: Vec<Cow<'a, HgPath>>, pub ignored: Vec<HgPathCow<'a>>,
pub unknown: Vec<Cow<'a, HgPath>>, pub unknown: Vec<HgPathCow<'a>>,
pub bad: Vec<(Cow<'a, HgPath>, BadMatch)>, pub bad: Vec<(HgPathCow<'a>, BadMatch)>,
/// Only filled if `collect_traversed_dirs` is `true` /// Only filled if `collect_traversed_dirs` is `true`
pub traversed: Vec<HgPathBuf>, pub traversed: Vec<HgPathBuf>,
} }
#[derive(Debug)] #[derive(Debug)]
pub enum StatusError { pub enum StatusError {
/// Generic IO error
IO(std::io::Error), IO(std::io::Error),
/// An invalid path that cannot be represented in Mercurial was found
Path(HgPathError), Path(HgPathError),
/// An invalid "ignore" pattern was found
Pattern(PatternError), Pattern(PatternError),
} }
pub type StatusResult<T> = Result<T, StatusError>; pub type StatusResult<T> = Result<T, StatusError>;
impl From<PatternError> for StatusError { impl From<PatternError> for StatusError {
fn from(e: PatternError) -> Self { fn from(e: PatternError) -> Self {
StatusError::Pattern(e) StatusError::Pattern(e)
match self { match self {
StatusError::IO(e) => e.to_string(), StatusError::IO(e) => e.to_string(),
StatusError::Path(e) => e.to_string(), StatusError::Path(e) => e.to_string(),
StatusError::Pattern(e) => e.to_string(), StatusError::Pattern(e) => e.to_string(),
} }
} }
} }
/// Gives information about which files are changed in the working directory
/// and how, compared to the revision we're based on
pub struct Status<'a, M: Matcher + Sync> { pub struct Status<'a, M: Matcher + Sync> {
dmap: &'a DirstateMap, dmap: &'a DirstateMap,
matcher: &'a M, matcher: &'a M,
root_dir: PathBuf, root_dir: PathBuf,
options: StatusOptions, options: StatusOptions,
ignore_fn: IgnoreFnType<'a>, ignore_fn: IgnoreFnType<'a>,
} }
root_dir, root_dir,
options, options,
ignore_fn, ignore_fn,
}, },
warnings, warnings,
)) ))
} }
/// Is the path ignored?
pub fn is_ignored(&self, path: impl AsRef<HgPath>) -> bool { pub fn is_ignored(&self, path: impl AsRef<HgPath>) -> bool {
(self.ignore_fn)(path.as_ref()) (self.ignore_fn)(path.as_ref())
} }
/// Is the path or one of its ancestors ignored? /// Is the path or one of its ancestors ignored?
pub fn dir_ignore(&self, dir: impl AsRef<HgPath>) -> bool { pub fn dir_ignore(&self, dir: impl AsRef<HgPath>) -> bool {
// Only involve ignore mechanism if we're listing unknowns or ignored. // Only involve ignore mechanism if we're listing unknowns or ignored.
if self.options.list_ignored || self.options.list_unknown { if self.options.list_ignored || self.options.list_unknown {
if self.is_ignored(&dir) { if self.is_ignored(&dir) {
true true
} else { } else {
for p in find_dirs(dir.as_ref()) { for p in find_dirs(dir.as_ref()) {
if self.is_ignored(p) { if self.is_ignored(p) {
return true; return true;
} }
} }
false false
} }
} else { } else {
true true
} }
} }
/// Get stat data about the files explicitly specified by match. /// Get stat data about the files explicitly specified by the matcher.
/// Returns a tuple of the directories that need to be traversed and the
/// files with their corresponding `Dispatch`.
/// TODO subrepos /// TODO subrepos
#[timed] #[timed]
pub fn walk_explicit( pub fn walk_explicit(
&self, &self,
traversed_sender: crossbeam::Sender<HgPathBuf>, traversed_sender: crossbeam::Sender<HgPathBuf>,
) -> ( ) -> (Vec<DispatchedPath<'a>>, Vec<DispatchedPath<'a>>) {
Vec<(Cow<'a, HgPath>, Dispatch)>,
Vec<(Cow<'a, HgPath>, Dispatch)>,
) {
self.matcher self.matcher
.file_set() .file_set()
.unwrap_or(&DEFAULT_WORK) .unwrap_or(&DEFAULT_WORK)
.par_iter() .par_iter()
.map(|&filename| -> Option<IoResult<_>> { .map(|&filename| -> Option<IoResult<_>> {
// TODO normalization // TODO normalization
let normalized = filename; let normalized = filename;
/// the current `DirstateMap`. /// the current `DirstateMap`.
/// ///
/// This takes a mutable reference to the results to account for the /// This takes a mutable reference to the results to account for the
/// `extend` in timings /// `extend` in timings
#[timed] #[timed]
pub fn traverse( pub fn traverse(
&self, &self,
path: impl AsRef<HgPath>, path: impl AsRef<HgPath>,
old_results: &FastHashMap<Cow<'a, HgPath>, Dispatch>, old_results: &FastHashMap<HgPathCow<'a>, Dispatch>,
results: &mut Vec<(Cow<'a, HgPath>, Dispatch)>, results: &mut Vec<DispatchedPath<'a>>,
traversed_sender: crossbeam::Sender<HgPathBuf>, traversed_sender: crossbeam::Sender<HgPathBuf>,
) -> IoResult<()> { ) -> IoResult<()> {
// The traversal is done in parallel, so use a channel to gather // The traversal is done in parallel, so use a channel to gather
// entries. `crossbeam::Sender` is `Sync`, while `mpsc::Sender` // entries. `crossbeam::Sender` is `Sync`, while `mpsc::Sender`
// is not. // is not.
let (files_transmitter, files_receiver) = let (files_transmitter, files_receiver) =
crossbeam::channel::unbounded(); crossbeam::channel::unbounded();
VisitChildrenSet::This | VisitChildrenSet::Recursive => None, VisitChildrenSet::This | VisitChildrenSet::Recursive => None,
VisitChildrenSet::Set(set) => Some(set), VisitChildrenSet::Set(set) => Some(set),
}; };
let buf = hg_path_to_path_buf(directory)?; let buf = hg_path_to_path_buf(directory)?;
let dir_path = self.root_dir.join(buf); let dir_path = self.root_dir.join(buf);
let skip_dot_hg = !directory.as_bytes().is_empty(); let skip_dot_hg = !directory.as_bytes().is_empty();
let entries = match list_directory(dir_path, skip_dot_hg) { let entries = match list_directory(dir_path, skip_dot_hg) {
Err(e) => match e.kind() { Err(e) => {
return match e.kind() {
ErrorKind::NotFound | ErrorKind::PermissionDenied => { ErrorKind::NotFound | ErrorKind::PermissionDenied => {
files_sender files_sender
.send(Ok(( .send(Ok((
directory.to_owned(), directory.to_owned(),
Dispatch::Bad(BadMatch::OsError( Dispatch::Bad(BadMatch::OsError(
// Unwrapping here is OK because the error // Unwrapping here is OK because the error
// always is a // always is a
// real os error // real os error
e.raw_os_error().unwrap(), e.raw_os_error().unwrap(),
)), )),
))) )))
.unwrap(); .expect("receiver should outlive sender");
return Ok(()); Ok(())
}
_ => Err(e),
};
} }
_ => return Err(e),
},
Ok(entries) => entries, Ok(entries) => entries,
}; };
rayon::scope(|scope| -> IoResult<()> { rayon::scope(|scope| -> IoResult<()> {
for (filename, dir_entry) in entries { for (filename, dir_entry) in entries {
if let Some(ref set) = visit_entries { if let Some(ref set) = visit_entries {
if !set.contains(filename.deref()) { if !set.contains(filename.deref()) {
continue; continue;
traversed_sender.clone(), traversed_sender.clone(),
)?; )?;
} }
} }
Ok(()) Ok(())
}) })
} }
/// Checks all files that are in the dirstate but were not found during the
/// working directory traversal. This means that the rest must
/// be either ignored, under a symlink or under a new nested repo.
///
/// This takes a mutable reference to the results to account for the /// This takes a mutable reference to the results to account for the
/// `extend` in timings /// `extend` in timings
#[timed] #[timed]
fn handle_unknowns( fn handle_unknowns(
&self, &self,
results: &mut Vec<(Cow<'a, HgPath>, Dispatch)>, results: &mut Vec<DispatchedPath<'a>>,
) -> IoResult<()> { ) -> IoResult<()> {
let to_visit: Vec<(&HgPath, &DirstateEntry)> = let to_visit: Vec<(&HgPath, &DirstateEntry)> =
if results.is_empty() && self.matcher.matches_everything() { if results.is_empty() && self.matcher.matches_everything() {
self.dmap.iter().map(|(f, e)| (f.deref(), e)).collect() self.dmap.iter().map(|(f, e)| (f.deref(), e)).collect()
} else { } else {
// Only convert to a hashmap if needed. // Only convert to a hashmap if needed.
let old_results: FastHashMap<_, _> = let old_results: FastHashMap<_, _> =
results.iter().cloned().collect(); results.iter().cloned().collect();
self.dmap self.dmap
.iter() .iter()
.filter_map(move |(f, e)| { .filter_map(move |(f, e)| {
if !old_results.contains_key(f.deref()) if !old_results.contains_key(f.deref())
&& self.matcher.matches(f) && self.matcher.matches(f)
{ {
Some((f.deref(), e)) Some((f.deref(), e))
} else { } else {
None None
} }
}) })
.collect() .collect()
}; };
// We walked all dirs under the roots that weren't ignored, and
// everything that matched was stat'ed and is already in results.
// The rest must thus be ignored or under a symlink.
let path_auditor = PathAuditor::new(&self.root_dir); let path_auditor = PathAuditor::new(&self.root_dir);
// TODO don't collect. Find a way of replicating the behavior of // TODO don't collect. Find a way of replicating the behavior of
// `itertools::process_results`, but for `rayon::ParallelIterator` // `itertools::process_results`, but for `rayon::ParallelIterator`
let new_results: IoResult<Vec<_>> = to_visit let new_results: IoResult<Vec<_>> = to_visit
.into_par_iter() .into_par_iter()
.filter_map(|(filename, entry)| -> Option<IoResult<_>> { .filter_map(|(filename, entry)| -> Option<IoResult<_>> {
// Report ignored items in the dmap as long as they are not // Report ignored items in the dmap as long as they are not
}) })
.collect(); .collect();
results.par_extend(new_results?); results.par_extend(new_results?);
Ok(()) Ok(())
} }
/// Add the files in the dirstate to the results.
///
/// This takes a mutable reference to the results to account for the /// This takes a mutable reference to the results to account for the
/// `extend` in timings /// `extend` in timings
#[timed] #[timed]
fn extend_from_dmap( fn extend_from_dmap(&self, results: &mut Vec<DispatchedPath<'a>>) {
&self,
results: &mut Vec<(Cow<'a, HgPath>, Dispatch)>,
) {
results.par_extend(self.dmap.par_iter().flat_map( results.par_extend(self.dmap.par_iter().flat_map(
move |(filename, entry)| { move |(filename, entry)| {
let filename: &HgPath = filename; let filename: &HgPath = filename;
let filename_as_path = hg_path_to_path_buf(filename)?; let filename_as_path = hg_path_to_path_buf(filename)?;
let meta = let meta =
self.root_dir.join(filename_as_path).symlink_metadata(); self.root_dir.join(filename_as_path).symlink_metadata();
match meta { match meta {
} }
}, },
)); ));
} }
} }
#[timed] #[timed]
fn build_response<'a>( fn build_response<'a>(
results: impl IntoIterator<Item = (Cow<'a, HgPath>, Dispatch)>, results: impl IntoIterator<Item = DispatchedPath<'a>>,
traversed: Vec<HgPathBuf>, traversed: Vec<HgPathBuf>,
) -> (Vec<Cow<'a, HgPath>>, DirstateStatus<'a>) { ) -> (Vec<HgPathCow<'a>>, DirstateStatus<'a>) {
let mut lookup = vec![]; let mut lookup = vec![];
let mut modified = vec![]; let mut modified = vec![];
let mut added = vec![]; let mut added = vec![];
let mut removed = vec![]; let mut removed = vec![];
let mut deleted = vec![]; let mut deleted = vec![];
let mut clean = vec![]; let mut clean = vec![];
let mut ignored = vec![]; let mut ignored = vec![];
let mut unknown = vec![]; let mut unknown = vec![];
#[timed] #[timed]
pub fn status<'a>( pub fn status<'a>(
dmap: &'a DirstateMap, dmap: &'a DirstateMap,
matcher: &'a (impl Matcher + Sync), matcher: &'a (impl Matcher + Sync),
root_dir: PathBuf, root_dir: PathBuf,
ignore_files: Vec<PathBuf>, ignore_files: Vec<PathBuf>,
options: StatusOptions, options: StatusOptions,
) -> StatusResult<( ) -> StatusResult<(
(Vec<Cow<'a, HgPath>>, DirstateStatus<'a>), (Vec<HgPathCow<'a>>, DirstateStatus<'a>),
Vec<PatternFileWarning>, Vec<PatternFileWarning>,
)> { )> {
let (traversed_sender, traversed_receiver) = let (traversed_sender, traversed_receiver) =
crossbeam::channel::unbounded(); crossbeam::channel::unbounded();
let (st, warnings) = let (st, warnings) =
Status::new(dmap, matcher, root_dir, ignore_files, options)?; Status::new(dmap, matcher, root_dir, ignore_files, options)?;
// Step 1: check the files explicitly mentioned by the user // Step 1: check the files explicitly mentioned by the user
} }
} }
_ => unreachable!("There can only be directories in `work`"), _ => unreachable!("There can only be directories in `work`"),
} }
} }
} }
if !matcher.is_exact() { if !matcher.is_exact() {
// Step 3: Check the remaining files from the dmap.
// If a dmap file is not in results yet, it was either
// a) not matched b) ignored, c) missing, or d) under a
// symlink directory.
if options.list_unknown { if options.list_unknown {
st.handle_unknowns(&mut results)?; st.handle_unknowns(&mut results)?;
} else { } else {
// We may not have walked the full directory tree above, so stat // TODO this is incorrect, see issue6335
// and check everything we missed. // This requires a fix in both Python and Rust that can happen
// with other pending changes to `status`.
st.extend_from_dmap(&mut results); st.extend_from_dmap(&mut results);
} }
} }
drop(traversed_sender); drop(traversed_sender);
let traversed = traversed_receiver.into_iter().collect(); let traversed = traversed_receiver.into_iter().collect();
Ok((build_response(results, traversed), warnings)) Ok((build_response(results, traversed), warnings))
} }