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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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 21913

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))
} }