This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7869

rust-dirs-multiset: add `DirsChildrenMultiset`
ClosedPublic

Authored by Alphare on Jan 14 2020, 12:33 PM.

Details

Reviewers
kevincox
Group Reviewers
hg-reviewers
Commits
rHG0e9ac3968b56: rust-dirs-multiset: add `DirsChildrenMultiset`
rHG2dca0d76358c: rust-dirs-multiset: add `DirsChildrenMultiset`
Summary

In a future patch, this structure will be needed to store information needed by
the (also upcoming) IgnoreMatcher.

Diff Detail

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

Event Timeline

Alphare created this revision.Jan 14 2020, 12:33 PM
kevincox accepted this revision.Jan 15 2020, 10:06 AM
kevincox added inline comments.
rust/hg-core/src/dirstate/dirs_multiset.rs
170

I would put this check into a helper function.

rust/hg-core/src/utils/files.rs
86

This should probably be a helper on HgPath? It would be much easier to understand what it is doing if it had a name.

Alphare marked 2 inline comments as done.Jan 15 2020, 4:56 PM
Alphare updated this revision to Diff 19319.
Alphare updated this revision to Diff 19353.Jan 16 2020, 5:22 AM
kevincox accepted this revision.Jan 16 2020, 7:46 AM
kevincox added inline comments.
rust/hg-core/src/utils/hg_path.rs
194

"suffix" isn't clear to me. Maybe split_dir_from_file? That's a bit wordy but I feel that it gets the point across. Or split_file_name.

Alphare updated this revision to Diff 19358.Jan 16 2020, 7:48 AM
Alphare updated this revision to Diff 19383.Jan 16 2020, 4:50 PM
Alphare added a commit: rHG2dca0d76358c: rust-dirs-multiset: add `DirsChildrenMultiset`.Feb 6 2020, 11:47 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG2dca0d76358c: rust-dirs-multiset: add `DirsChildrenMultiset` (authored by Alphare). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 19383

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

// dirs_multiset.rs // dirs_multiset.rs
// //
// Copyright 2019 Raphaël Gomès <rgomes@octobus.net> // Copyright 2019 Raphaël Gomès <rgomes@octobus.net>
// //
// 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.
//! A multiset of directory names. //! A multiset of directory names.
//! //!
//! Used to counts the references to directories in a manifest or dirstate. //! Used to counts the references to directories in a manifest or dirstate.
use crate::utils::hg_path::{HgPath, HgPathBuf};
use crate::{ use crate::{
dirstate::EntryState, utils::files, DirstateEntry, DirstateMapError, dirstate::EntryState,
FastHashMap, utils::{
files,
hg_path::{HgPath, HgPathBuf},
},
DirstateEntry, DirstateMapError, FastHashMap,
}; };
use std::collections::hash_map::{self, Entry}; use std::collections::{hash_map, hash_map::Entry, HashMap, HashSet};
// could be encapsulated if we care API stability more seriously // could be encapsulated if we care API stability more seriously
pub type DirsMultisetIter<'a> = hash_map::Keys<'a, HgPathBuf, u32>; pub type DirsMultisetIter<'a> = hash_map::Keys<'a, HgPathBuf, u32>;
#[derive(PartialEq, Debug)] #[derive(PartialEq, Debug)]
pub struct DirsMultiset { pub struct DirsMultiset {
inner: FastHashMap<HgPathBuf, u32>, inner: FastHashMap<HgPathBuf, u32>,
} }
self.inner.keys() self.inner.keys()
} }
pub fn len(&self) -> usize { pub fn len(&self) -> usize {
self.inner.len() self.inner.len()
} }
} }
/// This is basically a reimplementation of `DirsMultiset` that stores the
/// children instead of just a count of them, plus a small optional
/// optimization to avoid some directories we don't need.
#[derive(PartialEq, Debug)]
pub struct DirsChildrenMultiset<'a> {
inner: FastHashMap<&'a HgPath, HashSet<&'a HgPath>>,
only_include: Option<HashSet<&'a HgPath>>,
}
impl<'a> DirsChildrenMultiset<'a> {
pub fn new(
paths: impl Iterator<Item = &'a HgPathBuf>,
only_include: Option<&'a HashSet<impl AsRef<HgPath> + 'a>>,
) -> Self {
let mut new = Self {
inner: HashMap::default(),
only_include: only_include
.map(|s| s.iter().map(|p| p.as_ref()).collect()),
};
for path in paths {
new.add_path(path)
}
new
}
fn add_path(&mut self, path: &'a (impl AsRef<HgPath> + 'a)) {
if path.as_ref().is_empty() {
return;
}
for (directory, basename) in files::find_dirs_with_base(path.as_ref())
{
if !self.is_dir_included(directory) {
continue;
}
self.inner
kevincoxUnsubmitted
Done

I would put this check into a helper function.

kevincox: I would put this check into a helper function.
.entry(directory)
.and_modify(|e| {
e.insert(basename);
})
.or_insert_with(|| {
let mut set = HashSet::new();
set.insert(basename);
set
});
}
}
fn is_dir_included(&self, dir: impl AsRef<HgPath>) -> bool {
match &self.only_include {
None => false,
Some(i) => i.contains(dir.as_ref()),
}
}
pub fn get(
&self,
path: impl AsRef<HgPath>,
) -> Option<&HashSet<&'a HgPath>> {
self.inner.get(path.as_ref())
}
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
#[test] #[test]
fn test_delete_path_path_not_found() { fn test_delete_path_path_not_found() {
let manifest: Vec<HgPathBuf> = vec![]; let manifest: Vec<HgPathBuf> = vec![];
let mut map = DirsMultiset::from_manifest(&manifest).unwrap(); let mut map = DirsMultiset::from_manifest(&manifest).unwrap();

rust/hg-core/src/utils/files.rs

// files.rs // files.rs
// //
// Copyright 2019 // Copyright 2019
// Raphaël Gomès <rgomes@octobus.net>, // Raphaël Gomès <rgomes@octobus.net>,
// Yuya Nishihara <yuya@tcha.org> // Yuya Nishihara <yuya@tcha.org>
// //
// 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.
//! Functions for fiddling with files. //! Functions for fiddling with files.
use crate::utils::hg_path::{HgPath, HgPathBuf}; use crate::utils::hg_path::{HgPath, HgPathBuf};
use std::iter::FusedIterator;
use crate::utils::replace_slice; use crate::utils::replace_slice;
use lazy_static::lazy_static; use lazy_static::lazy_static;
use std::fs::Metadata; use std::fs::Metadata;
use std::iter::FusedIterator;
use std::path::Path; use std::path::Path;
pub fn get_path_from_bytes(bytes: &[u8]) -> &Path { pub fn get_path_from_bytes(bytes: &[u8]) -> &Path {
let os_str; let os_str;
#[cfg(unix)] #[cfg(unix)]
{ {
use std::os::unix::ffi::OsStrExt; use std::os::unix::ffi::OsStrExt;
os_str = std::ffi::OsStr::from_bytes(bytes); os_str = std::ffi::OsStr::from_bytes(bytes);
None => None, None => None,
}; };
next next
} }
} }
impl<'a> FusedIterator for Ancestors<'a> {} impl<'a> FusedIterator for Ancestors<'a> {}
/// An iterator over repository path yielding itself and its ancestors.
#[derive(Copy, Clone, Debug)]
pub(crate) struct AncestorsWithBase<'a> {
next: Option<(&'a HgPath, &'a HgPath)>,
}
impl<'a> Iterator for AncestorsWithBase<'a> {
type Item = (&'a HgPath, &'a HgPath);
fn next(&mut self) -> Option<Self::Item> {
let next = self.next;
self.next = match self.next {
Some((s, _)) if s.is_empty() => None,
Some((s, _)) => Some(s.split_filename()),
None => None,
};
next
}
}
kevincoxUnsubmitted
Done

This should probably be a helper on HgPath? It would be much easier to understand what it is doing if it had a name.

kevincox: This should probably be a helper on `HgPath`? It would be much easier to understand what it is…
impl<'a> FusedIterator for AncestorsWithBase<'a> {}
/// Returns an iterator yielding ancestor directories of the given repository /// Returns an iterator yielding ancestor directories of the given repository
/// path. /// path.
/// ///
/// The path is separated by '/', and must not start with '/'. /// The path is separated by '/', and must not start with '/'.
/// ///
/// The path itself isn't included unless it is b"" (meaning the root /// The path itself isn't included unless it is b"" (meaning the root
/// directory.) /// directory.)
pub fn find_dirs<'a>(path: &'a HgPath) -> Ancestors<'a> { pub fn find_dirs<'a>(path: &'a HgPath) -> Ancestors<'a> {
let mut dirs = Ancestors { next: Some(path) }; let mut dirs = Ancestors { next: Some(path) };
if !path.is_empty() { if !path.is_empty() {
dirs.next(); // skip itself dirs.next(); // skip itself
} }
dirs dirs
} }
/// Returns an iterator yielding ancestor directories of the given repository
/// path.
///
/// The path is separated by '/', and must not start with '/'.
///
/// The path itself isn't included unless it is b"" (meaning the root
/// directory.)
pub(crate) fn find_dirs_with_base<'a>(
path: &'a HgPath,
) -> AncestorsWithBase<'a> {
let mut dirs = AncestorsWithBase {
next: Some((path, HgPath::new(b""))),
};
if !path.is_empty() {
dirs.next(); // skip itself
}
dirs
}
/// TODO more than ASCII? /// TODO more than ASCII?
pub fn normalize_case(path: &HgPath) -> HgPathBuf { pub fn normalize_case(path: &HgPath) -> HgPathBuf {
#[cfg(windows)] // NTFS compares via upper() #[cfg(windows)] // NTFS compares via upper()
return path.to_ascii_uppercase(); return path.to_ascii_uppercase();
#[cfg(unix)] #[cfg(unix)]
path.to_ascii_lowercase() path.to_ascii_lowercase()
} }
#[test] #[test]
fn find_dirs_empty() { fn find_dirs_empty() {
// looks weird, but mercurial.pathutil.finddirs(b"") yields b"" // looks weird, but mercurial.pathutil.finddirs(b"") yields b""
let mut dirs = super::find_dirs(HgPath::new(b"")); let mut dirs = super::find_dirs(HgPath::new(b""));
assert_eq!(dirs.next(), Some(HgPath::new(b""))); assert_eq!(dirs.next(), Some(HgPath::new(b"")));
assert_eq!(dirs.next(), None); assert_eq!(dirs.next(), None);
assert_eq!(dirs.next(), None); assert_eq!(dirs.next(), None);
} }
#[test]
fn test_find_dirs_with_base_some() {
let mut dirs = super::find_dirs_with_base(HgPath::new(b"foo/bar/baz"));
assert_eq!(
dirs.next(),
Some((HgPath::new(b"foo/bar"), HgPath::new(b"baz")))
);
assert_eq!(
dirs.next(),
Some((HgPath::new(b"foo"), HgPath::new(b"bar")))
);
assert_eq!(dirs.next(), Some((HgPath::new(b""), HgPath::new(b"foo"))));
assert_eq!(dirs.next(), None);
assert_eq!(dirs.next(), None);
}
#[test]
fn test_find_dirs_with_base_empty() {
let mut dirs = super::find_dirs_with_base(HgPath::new(b""));
assert_eq!(dirs.next(), Some((HgPath::new(b""), HgPath::new(b""))));
assert_eq!(dirs.next(), None);
assert_eq!(dirs.next(), None);
}
} }

rust/hg-core/src/utils/hg_path.rs

} }
pub fn trim_trailing_slash(&self) -> &Self { pub fn trim_trailing_slash(&self) -> &Self {
Self::new(if self.inner.last() == Some(&b'/') { Self::new(if self.inner.last() == Some(&b'/') {
&self.inner[..self.inner.len() - 1] &self.inner[..self.inner.len() - 1]
} else { } else {
&self.inner[..] &self.inner[..]
}) })
} }
/// Returns a tuple of slices `(base, filename)` resulting from the split
/// at the rightmost `/`, if any.
///
/// # Examples:
///
/// ```
/// use hg::utils::hg_path::HgPath;
///
/// let path = HgPath::new(b"cool/hg/path").split_filename();
/// assert_eq!(path, (HgPath::new(b"cool/hg"), HgPath::new(b"path")));
///
/// let path = HgPath::new(b"pathwithoutsep").split_filename();
/// assert_eq!(path, (HgPath::new(b""), HgPath::new(b"pathwithoutsep")));
/// ```
pub fn split_filename(&self) -> (&Self, &Self) {
kevincoxUnsubmitted
Not Done

"suffix" isn't clear to me. Maybe split_dir_from_file? That's a bit wordy but I feel that it gets the point across. Or split_file_name.

kevincox: "suffix" isn't clear to me. Maybe `split_dir_from_file`? That's a bit wordy but I feel that it…
match &self.inner.iter().rposition(|c| *c == b'/') {
None => (HgPath::new(""), &self),
Some(size) => (
HgPath::new(&self.inner[..*size]),
HgPath::new(&self.inner[*size + 1..]),
),
}
}
pub fn join<T: ?Sized + AsRef<Self>>(&self, other: &T) -> HgPathBuf { pub fn join<T: ?Sized + AsRef<Self>>(&self, other: &T) -> HgPathBuf {
let mut inner = self.inner.to_owned(); let mut inner = self.inner.to_owned();
if inner.len() != 0 && inner.last() != Some(&b'/') { if inner.len() != 0 && inner.last() != Some(&b'/') {
inner.push(b'/'); inner.push(b'/');
} }
inner.extend(other.as_ref().bytes()); inner.extend(other.as_ref().bytes());
HgPathBuf::from_bytes(&inner) HgPathBuf::from_bytes(&inner)
} }