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
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.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
200

"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 19957

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