This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8086

rust-status: refactor options into a `StatusOptions` struct
ClosedPublic

Authored by Alphare on Feb 6 2020, 10:15 AM.

Details

Reviewers
marmoute
durin42
Group Reviewers
hg-reviewers
Commits
rHG483fce658e43: rust-status: refactor options into a `StatusOptions` struct

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.Feb 6 2020, 10:15 AM
kevincox added inline comments.Feb 10 2020, 10:56 AM
rust/hg-core/src/dirstate/status.rs
208

These names aren't very meaningful to me but maybe that is because I'm not super familiar with the domain.

Alphare added inline comments.Feb 10 2020, 11:22 AM
rust/hg-core/src/dirstate/status.rs
208

It doesn't hurt to have some documentation. They're not super obvious indeed.

Alphare updated this revision to Diff 20044.Feb 10 2020, 11:30 AM
Alphare updated this revision to Diff 20155.Feb 11 2020, 5:55 AM
Alphare updated this revision to Diff 20164.Feb 11 2020, 6:13 AM
Alphare updated this revision to Diff 20184.Feb 13 2020, 12:37 PM
marmoute accepted this revision.Feb 14 2020, 3:44 AM
marmoute added a subscriber: marmoute.

looks good to me. thanks for plitting

durin42 accepted this revision.Feb 28 2020, 11:58 AM
This revision is now accepted and ready to land.Feb 28 2020, 11:58 AM
Alphare updated this revision to Diff 20496.Mar 5 2020, 3:33 AM
kevincox removed a subscriber: kevincox.Mar 5 2020, 3:40 AM

Revision Contents
Changeset List

Diff 20717

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

} }
/// The file corresponding to the dirstate entry was found on the filesystem. /// The file corresponding to the dirstate entry was found on the filesystem.
fn dispatch_found( fn dispatch_found(
filename: impl AsRef<HgPath>, filename: impl AsRef<HgPath>,
entry: DirstateEntry, entry: DirstateEntry,
metadata: HgMetadata, metadata: HgMetadata,
copy_map: &CopyMap, copy_map: &CopyMap,
check_exec: bool, options: StatusOptions,
list_clean: bool,
last_normal_time: i64,
) -> Dispatch { ) -> Dispatch {
let DirstateEntry { let DirstateEntry {
state, state,
mode, mode,
mtime, mtime,
size, size,
} = entry; } = entry;
let HgMetadata { let HgMetadata {
st_mode, st_mode,
st_size, st_size,
st_mtime, st_mtime,
.. ..
} = metadata; } = metadata;
match state { match state {
EntryState::Normal => { EntryState::Normal => {
let size_changed = mod_compare(size, st_size as i32); let size_changed = mod_compare(size, st_size as i32);
let mode_changed = let mode_changed =
(mode ^ st_mode as i32) & 0o100 != 0o000 && check_exec; (mode ^ st_mode as i32) & 0o100 != 0o000 && options.check_exec;
let metadata_changed = size >= 0 && (size_changed || mode_changed); let metadata_changed = size >= 0 && (size_changed || mode_changed);
let other_parent = size == SIZE_FROM_OTHER_PARENT; let other_parent = size == SIZE_FROM_OTHER_PARENT;
if metadata_changed if metadata_changed
|| other_parent || other_parent
|| copy_map.contains_key(filename.as_ref()) || copy_map.contains_key(filename.as_ref())
{ {
Dispatch::Modified Dispatch::Modified
} else if mod_compare(mtime, st_mtime as i32) { } else if mod_compare(mtime, st_mtime as i32) {
Dispatch::Unsure Dispatch::Unsure
} else if st_mtime == last_normal_time { } else if st_mtime == options.last_normal_time {
// the file may have just been marked as normal and // the file may have just been marked as normal and
// it may have changed in the same second without // it may have changed in the same second without
// changing its size. This can happen if we quickly // changing its size. This can happen if we quickly
// do multiple commits. Force lookup, so we don't // do multiple commits. Force lookup, so we don't
// miss such a racy file change. // miss such a racy file change.
Dispatch::Unsure Dispatch::Unsure
} else if list_clean { } else if options.list_clean {
Dispatch::Clean Dispatch::Clean
} else { } else {
Dispatch::Unknown Dispatch::Unknown
} }
} }
EntryState::Merged => Dispatch::Modified, EntryState::Merged => Dispatch::Modified,
EntryState::Added => Dispatch::Added, EntryState::Added => Dispatch::Added,
EntryState::Removed => Dispatch::Removed, EntryState::Removed => Dispatch::Removed,
} }
/// Get stat data about the files explicitly specified by match. /// Get stat data about the files explicitly specified by match.
/// TODO subrepos /// TODO subrepos
fn walk_explicit<'a>( fn walk_explicit<'a>(
files: &'a HashSet<&HgPath>, files: &'a HashSet<&HgPath>,
dmap: &'a DirstateMap, dmap: &'a DirstateMap,
root_dir: impl AsRef<Path> + Sync + Send, root_dir: impl AsRef<Path> + Sync + Send,
check_exec: bool, options: StatusOptions,
list_clean: bool,
last_normal_time: i64,
) -> impl ParallelIterator<Item = IoResult<(&'a HgPath, Dispatch)>> { ) -> impl ParallelIterator<Item = IoResult<(&'a HgPath, Dispatch)>> {
files.par_iter().filter_map(move |filename| { files.par_iter().filter_map(move |filename| {
// TODO normalization // TODO normalization
let normalized = filename.as_ref(); let normalized = filename.as_ref();
let buf = match hg_path_to_path_buf(normalized) { let buf = match hg_path_to_path_buf(normalized) {
Ok(x) => x, Ok(x) => x,
Err(e) => return Some(Err(e.into())), Err(e) => return Some(Err(e.into())),
}; };
let target = root_dir.as_ref().join(buf); let target = root_dir.as_ref().join(buf);
let st = target.symlink_metadata(); let st = target.symlink_metadata();
match st { match st {
Ok(meta) => { Ok(meta) => {
let file_type = meta.file_type(); let file_type = meta.file_type();
if file_type.is_file() || file_type.is_symlink() { if file_type.is_file() || file_type.is_symlink() {
if let Some(entry) = dmap.get(normalized) { if let Some(entry) = dmap.get(normalized) {
return Some(Ok(( return Some(Ok((
normalized, normalized,
dispatch_found( dispatch_found(
&normalized, &normalized,
*entry, *entry,
HgMetadata::from_metadata(meta), HgMetadata::from_metadata(meta),
&dmap.copy_map, &dmap.copy_map,
check_exec, options,
list_clean,
last_normal_time,
), ),
))); )));
} }
} else { } else {
if dmap.contains_key(normalized) { if dmap.contains_key(normalized) {
return Some(Ok((normalized, Dispatch::Removed))); return Some(Ok((normalized, Dispatch::Removed)));
} }
} }
} }
Err(_) => { Err(_) => {
if let Some(entry) = dmap.get(normalized) { if let Some(entry) = dmap.get(normalized) {
return Some(Ok(( return Some(Ok((
normalized, normalized,
dispatch_missing(entry.state), dispatch_missing(entry.state),
))); )));
} }
} }
}; };
None None
}) })
} }
#[derive(Debug, Copy, Clone)]
pub struct StatusOptions {
/// Remember the most recent modification timeslot for status, to make
/// sure we won't miss future size-preserving file content modifications
/// that happen within the same timeslot.
pub last_normal_time: i64,
kevincoxUnsubmitted
Not Done

These names aren't very meaningful to me but maybe that is because I'm not super familiar with the domain.

kevincox: These names aren't very meaningful to me but maybe that is because I'm not super familiar with…
AlphareAuthorUnsubmitted
Done

It doesn't hurt to have some documentation. They're not super obvious indeed.

Alphare: It doesn't hurt to have some documentation. They're not super obvious indeed.
/// Whether we are on a filesystem with UNIX-like exec flags
pub check_exec: bool,
pub list_clean: bool,
}
/// Stat all entries in the `DirstateMap` and mark them for dispatch into /// Stat all entries in the `DirstateMap` and mark them for dispatch into
/// the relevant collections. /// the relevant collections.
fn stat_dmap_entries( fn stat_dmap_entries(
dmap: &DirstateMap, dmap: &DirstateMap,
root_dir: impl AsRef<Path> + Sync + Send, root_dir: impl AsRef<Path> + Sync + Send,
check_exec: bool, options: StatusOptions,
list_clean: bool,
last_normal_time: i64,
) -> impl ParallelIterator<Item = IoResult<(&HgPath, Dispatch)>> { ) -> impl ParallelIterator<Item = IoResult<(&HgPath, Dispatch)>> {
dmap.par_iter().map(move |(filename, entry)| { dmap.par_iter().map(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 = root_dir.as_ref().join(filename_as_path).symlink_metadata(); let meta = root_dir.as_ref().join(filename_as_path).symlink_metadata();
match meta { match meta {
Ok(ref m) Ok(ref m)
if !(m.file_type().is_file() if !(m.file_type().is_file()
|| m.file_type().is_symlink()) => || m.file_type().is_symlink()) =>
{ {
Ok((filename, dispatch_missing(entry.state))) Ok((filename, dispatch_missing(entry.state)))
} }
Ok(m) => Ok(( Ok(m) => Ok((
filename, filename,
dispatch_found( dispatch_found(
filename, filename,
*entry, *entry,
HgMetadata::from_metadata(m), HgMetadata::from_metadata(m),
&dmap.copy_map, &dmap.copy_map,
check_exec, options,
list_clean,
last_normal_time,
), ),
)), )),
Err(ref e) Err(ref e)
if e.kind() == std::io::ErrorKind::NotFound if e.kind() == std::io::ErrorKind::NotFound
|| e.raw_os_error() == Some(20) => || e.raw_os_error() == Some(20) =>
{ {
// Rust does not yet have an `ErrorKind` for // Rust does not yet have an `ErrorKind` for
// `NotADirectory` (errno 20) // `NotADirectory` (errno 20)
}, },
)) ))
} }
pub fn status<'a: 'c, 'b: 'c, 'c>( pub fn status<'a: 'c, 'b: 'c, 'c>(
dmap: &'a DirstateMap, dmap: &'a DirstateMap,
matcher: &'b impl Matcher, matcher: &'b impl Matcher,
root_dir: impl AsRef<Path> + Sync + Send + Copy, root_dir: impl AsRef<Path> + Sync + Send + Copy,
list_clean: bool, options: StatusOptions,
last_normal_time: i64,
check_exec: bool,
) -> IoResult<(Vec<&'c HgPath>, StatusResult<'c>)> { ) -> IoResult<(Vec<&'c HgPath>, StatusResult<'c>)> {
let files = matcher.file_set(); let files = matcher.file_set();
let mut results = vec![]; let mut results = vec![];
if let Some(files) = files { if let Some(files) = files {
results.par_extend(walk_explicit( results.par_extend(walk_explicit(&files, &dmap, root_dir, options));
&files,
&dmap,
root_dir,
check_exec,
list_clean,
last_normal_time,
));
} }
if !matcher.is_exact() { if !matcher.is_exact() {
let stat_results = stat_dmap_entries( let stat_results = stat_dmap_entries(&dmap, root_dir, options);
&dmap,
root_dir,
check_exec,
list_clean,
last_normal_time,
);
results.par_extend(stat_results); results.par_extend(stat_results);
} }
build_response(results) build_response(results)
} }

rust/hg-core/src/lib.rs

// Copyright 2018-2020 Georges Racinet <georges.racinet@octobus.net> // Copyright 2018-2020 Georges Racinet <georges.racinet@octobus.net>
// and Mercurial contributors // and Mercurial contributors
// //
// This software may be used and distributed according to the terms of the // This software may be used and distributed according to the terms of the
// GNU General Public License version 2 or any later version. // GNU General Public License version 2 or any later version.
mod ancestors; mod ancestors;
pub mod dagops; pub mod dagops;
pub use ancestors::{AncestorsIterator, LazyAncestors, MissingAncestors}; pub use ancestors::{AncestorsIterator, LazyAncestors, MissingAncestors};
mod dirstate; mod dirstate;
pub mod discovery; pub mod discovery;
pub mod testing; // unconditionally built, for use from integration tests pub mod testing; // unconditionally built, for use from integration tests
pub use dirstate::{ pub use dirstate::{
dirs_multiset::{DirsMultiset, DirsMultisetIter}, dirs_multiset::{DirsMultiset, DirsMultisetIter},
dirstate_map::DirstateMap, dirstate_map::DirstateMap,
parsers::{pack_dirstate, parse_dirstate, PARENT_SIZE}, parsers::{pack_dirstate, parse_dirstate, PARENT_SIZE},
status::{status, StatusResult}, status::{status, StatusOptions, StatusResult},
CopyMap, CopyMapIter, DirstateEntry, DirstateParents, EntryState, CopyMap, CopyMapIter, DirstateEntry, DirstateParents, EntryState,
StateMap, StateMapIter, StateMap, StateMapIter,
}; };
mod filepatterns; mod filepatterns;
pub mod matchers; pub mod matchers;
pub mod revlog; pub mod revlog;
pub use revlog::*; pub use revlog::*;
#[cfg(feature = "with-re2")] #[cfg(feature = "with-re2")]

rust/hg-core/src/matchers.rs

let dirs = HashSet::new(); let dirs = HashSet::new();
let mut parents = HashSet::new(); let mut parents = HashSet::new();
parents.insert(HgPathBuf::new()); parents.insert(HgPathBuf::new());
parents.insert(HgPathBuf::from_bytes(b"g")); parents.insert(HgPathBuf::from_bytes(b"g"));
assert_eq!( assert_eq!(
roots_dirs_and_parents(&pats).unwrap(), roots_dirs_and_parents(&pats).unwrap(),
RootsDirsAndParents {roots, dirs, parents} RootsDirsAndParents {
roots,
dirs,
parents
}
); );
} }
#[test] #[test]
fn test_filematcher_visit_children_set() { fn test_filematcher_visit_children_set() {
// Visitchildrenset // Visitchildrenset
let files = vec![HgPath::new(b"dir/subdir/foo.txt")]; let files = vec![HgPath::new(b"dir/subdir/foo.txt")];
let matcher = FileMatcher::new(&files).unwrap(); let matcher = FileMatcher::new(&files).unwrap();