rust/hg-core/src/dirstate/dirs_multiset.rs
43	You can replate the nested if with: if skip_state == Some(state) {
63	I would remove the `pos` argument. IIUC following two are currently identical. find_dir(path, n); find_dir(path[..n], n); If you remove the second argument then you can just always remove the last component of the path. This will also allow you to have a more clear doc comment. Right now I find it a little confusing.

rust/hg-core/src/dirstate/dirs_multiset.rs
43	Unless I'm missing something, this would render the `if` statement useless.

rust/hg-core/src/dirstate/dirs_multiset.rs
43	Maybe Kevin meant something like this? if skip_state == None \| skip_state == Some(state) { multiset.add_path(filename); }

rust/hg-core/src/dirstate/dirs_multiset.rs
43	No, you are right. I misread the condition.

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	15160		May 17 2019, 6:10 AM	★	★
Diff 2	15341		Jun 5 2019, 12:22 PM	★	★
Diff 3	15508		Jun 14 2019, 6:13 AM	★	★
Diff 4	15670		Jun 27 2019, 9:16 AM	★	★
Diff 5	15694	rHG2dcee6497b0b9a9d5b55d1c068b8702b883a857e	May 16 2019, 12:03 PM	★	★

Commit	Parents	Author	Summary	Date
		Raphaël Gomès		May 16 2019, 12:03 PM

Status	Author	Revision
Closed	Alphare	D6489 rust-regex: fix shortcut for exact matches
Closed	Alphare	D6485 rust-filepatterns: use bytes instead of String
Closed	Alphare	D6395 rust-dirstate: call new "dirs" rust implementation from Python
Closed	Alphare	D6394 rust-dirstate: add "dirs" rust-cpython binding
Closed	Alphare	D6393 rust-dirstate: add "dirs" Rust implementation
Closed	Kwan	D6496 phabricator: add commenting to phabsend for new/updated Diffs
Closed	Alphare	D6389 rust-dirstate: create dirstate submodule

Diff 15160

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

This file was added.

				// dirs_multiset.rs
				//
				// Copyright 2019 Raphaël Gomès <rgomes@octobus.net>
				//
				// This software may be used and distributed according to the terms of the
				// GNU General Public License version 2 or any later version.

				//! A multiset of directory names.
				//!
				//! Used to counts the references to directories in a manifest or dirstate.
				use std::collections::hash_map::Entry;
				use std::collections::HashMap;
				use std::ops::Deref;
				use {DirsIterable, DirstateEntry, DirstateMapError};

				#[derive(PartialEq, Debug)]
				pub struct DirsMultiset {
				inner: HashMap<Vec<u8>, u32>,
				}

				impl Deref for DirsMultiset {
				type Target = HashMap<Vec<u8>, u32>;

				fn deref(&self) -> &Self::Target {
				&self.inner
				}
				}

				impl DirsMultiset {
				/// Initializes the multiset from a dirstate or a manifest.
				///
				/// If `skip_state` is provided, skips dirstate entries with equal state.
				pub fn new(iterable: DirsIterable, skip_state: Option<i8>) -> Self {
				let mut multiset = DirsMultiset {
				inner: HashMap::new(),
				};

				match iterable {
				DirsIterable::Dirstate(vec) => {
				for (filename, DirstateEntry { state, .. }) in vec {
				// This `if` is optimized out of the loop
				if let Some(skip) = skip_state {
				if skip != state {
				kevincoxUnsubmitted Not Done You can replate the nested if with: if skip_state == Some(state) { kevincox: You can replate the nested if with: ``` if skip_state == Some(state) { ```
				AlphareAuthorUnsubmitted Done Unless I'm missing something, this would render the `if` statement useless. Alphare: Unless I'm missing something, this would render the `if` statement useless.
				martinvonzUnsubmitted Not Done Maybe Kevin meant something like this? if skip_state == None \| skip_state == Some(state) { multiset.add_path(filename); } martinvonz: Maybe Kevin meant something like this? ``` if skip_state == None \| skip_state == Some(state) {…
				kevincoxUnsubmitted Not Done No, you are right. I misread the condition. kevincox: No, you are right. I misread the condition.
				multiset.add_path(filename);
				}
				} else {
				multiset.add_path(filename);
				}
				}
				}
				DirsIterable::Manifest(vec) => {
				for filename in vec {
				multiset.add_path(filename);
				}
				}
				}

				multiset
				}

				/// Returns (maybe) a slice of path containing the next directory name
				/// without trailing slash, from right to left.
				fn find_dir(path: &[u8], mut pos: usize) -> Option<&[u8]> {
				kevincoxUnsubmitted Done I would remove the `pos` argument. IIUC following two are currently identical. find_dir(path, n); find_dir(path[..n], n); If you remove the second argument then you can just always remove the last component of the path. This will also allow you to have a more clear doc comment. Right now I find it a little confusing. kevincox: I would remove the `pos` argument. IIUC following two are currently identical. ``` find_dir…
				loop {
				if let Some(new_pos) = pos.checked_sub(1) {
				if path[new_pos] == b'/' {
				break Some(&path[..new_pos]);
				}
				pos = new_pos;
				} else {
				break None;
				}
				}
				}

				/// Increases the count of deepest directory contained in the path.
				///
				/// If the directory is not yet in the map, adds its parents.
				pub fn add_path(&mut self, path: Vec<u8>) {
				if path.is_empty() {
				return;
				}
				let mut pos = path.len();

				while let Some(subpath) = Self::find_dir(&path, pos) {
				if let Some(val) = self.inner.get_mut(subpath) {
				*val += 1;
				break;
				}
				self.inner.insert(subpath.to_owned(), 1);

				pos = subpath.len();
				}
				}

				/// Decreases the count of deepest directory contained in the path.
				///
				/// If it is the only reference, decreases all parents until one is
				/// removed.
				/// If the directory is not in the map, something horrible has happened.
				pub fn delete_path(
				&mut self,
				path: Vec<u8>,
				) -> Result<(), DirstateMapError> {
				if path.is_empty() {
				return Err(DirstateMapError::EmptyPath);
				}
				let mut pos = path.len();

				while let Some(subpath) = Self::find_dir(&path, pos) {
				match self.inner.entry(subpath.to_owned()) {
				Entry::Occupied(mut entry) => {
				let val = entry.get().clone();
				if val > 1 {
				entry.insert(val - 1);
				break;
				}
				entry.remove();
				}
				Entry::Vacant(_) => {
				return Err(DirstateMapError::PathNotFound(path.to_owned()))
				}
				};

				pos = subpath.len();
				}

				Ok(())
				}
				}

				#[cfg(test)]
				mod tests {
				use super::*;

				#[test]
				fn test_delete_path_path_not_found() {
				let mut map = DirsMultiset::new(DirsIterable::Manifest(vec![]), None);
				let path = b"doesnotexist/".to_vec();
				assert_eq!(
				Err(DirstateMapError::PathNotFound(path.to_owned())),
				map.delete_path(path)
				);
				}

				#[test]
				fn test_delete_path_empty_path() {
				let mut map = DirsMultiset::new(DirsIterable::Manifest(vec![]), None);
				let path = vec![];
				assert_eq!(Err(DirstateMapError::EmptyPath), map.delete_path(path));
				}

				#[test]
				fn test_delete_path_successful() {
				let mut map = DirsMultiset {
				inner: [("a", 3), ("a/b", 2), ("a/c", 1)]
				.iter()
				.map(\|(k, v)\| (k.as_bytes().to_vec(), *v))
				.collect(),
				};

				assert_eq!(Ok(()), map.delete_path(b"a/b/".to_vec()));
				assert_eq!(Ok(()), map.delete_path(b"a/b/".to_vec()));
				assert_eq!(
				Err(DirstateMapError::PathNotFound(b"a/b/".to_vec())),
				map.delete_path(b"a/b/".to_vec())
				);

				assert_eq!(2, *map.get(&b"a".to_vec()).unwrap());
				assert_eq!(1, *map.get(&b"a/c".to_vec()).unwrap());
				assert_eq!(Ok(()), map.delete_path(b"a/".to_vec()));

				assert_eq!(Ok(()), map.delete_path(b"a/c/".to_vec()));
				assert_eq!(
				Err(DirstateMapError::PathNotFound(b"a/c/".to_vec())),
				map.delete_path(b"a/c/".to_vec())
				);
				}

				#[test]
				fn test_add_path_empty_path() {
				let mut map = DirsMultiset::new(DirsIterable::Manifest(vec![]), None);
				let path = vec![];
				map.add_path(path);

				assert_eq!(0, map.len());
				}

				#[test]
				fn test_add_path_successful() {
				let mut map = DirsMultiset::new(DirsIterable::Manifest(vec![]), None);

				map.add_path(b"a/".to_vec());
				assert_eq!(1, *map.get(&b"a".to_vec()).unwrap());

				// Non directory should be ignored
				map.add_path(b"a".to_vec());
				assert_eq!(1, *map.get(&b"a".to_vec()).unwrap());

				// Non directory will still add its base
				map.add_path(b"a/b".to_vec());
				assert_eq!(2, *map.get(&b"a".to_vec()).unwrap());
				assert_eq!(1, map.len());

				// Duplicate path works
				map.add_path(b"a/".to_vec());
				assert_eq!(3, *map.get(&b"a".to_vec()).unwrap());

				// Nested dir adds to its base
				map.add_path(b"a/b/".to_vec());
				assert_eq!(4, *map.get(&b"a".to_vec()).unwrap());
				assert_eq!(1, *map.get(&b"a/b".to_vec()).unwrap());

				// but not its base's base, because it already existed
				map.add_path(b"a/b/c/".to_vec());
				assert_eq!(4, *map.get(&b"a".to_vec()).unwrap());
				assert_eq!(2, *map.get(&b"a/b".to_vec()).unwrap());

				map.add_path(b"a/c/".to_vec());
				assert_eq!(1, *map.get(&b"a/c".to_vec()).unwrap());

				let expected = DirsMultiset {
				inner: [("a", 5), ("a/b", 2), ("a/b/c", 1), ("a/c", 1)]
				.iter()
				.map(\|(k, v)\| (k.as_bytes().to_vec(), *v))
				.collect(),
				};
				assert_eq!(map, expected);
				}

				#[test]
				fn test_dirsmultiset_new_empty() {
				use DirsIterable::{Dirstate, Manifest};

				let new = DirsMultiset::new(Manifest(vec![]), None);
				let expected = DirsMultiset {
				inner: HashMap::new(),
				};
				assert_eq!(expected, new);

				let new = DirsMultiset::new(Dirstate(vec![]), None);
				let expected = DirsMultiset {
				inner: HashMap::new(),
				};
				assert_eq!(expected, new);
				}

				#[test]
				fn test_dirsmultiset_new_no_skip() {
				use DirsIterable::{Dirstate, Manifest};

				let input_vec = ["a/", "b/", "a/c", "a/d/"]
				.iter()
				.map(\|e\| e.as_bytes().to_vec())
				.collect();
				let expected_inner = [("a", 3), ("b", 1), ("a/d", 1)]
				.iter()
				.map(\|(k, v)\| (k.as_bytes().to_vec(), *v))
				.collect();

				let new = DirsMultiset::new(Manifest(input_vec), None);
				let expected = DirsMultiset {
				inner: expected_inner,
				};
				assert_eq!(expected, new);

				let input_map = ["a/", "b/", "a/c", "a/d/"]
				.iter()
				.map(\|f\| {
				(
				f.as_bytes().to_vec(),
				DirstateEntry {
				state: 0,
				mode: 0,
				mtime: 0,
				size: 0,
				},
				)
				})
				.collect();
				let expected_inner = [("a", 3), ("b", 1), ("a/d", 1)]
				.iter()
				.map(\|(k, v)\| (k.as_bytes().to_vec(), *v))
				.collect();

				let new = DirsMultiset::new(Dirstate(input_map), None);
				let expected = DirsMultiset {
				inner: expected_inner,
				};
				assert_eq!(expected, new);
				}

				#[test]
				fn test_dirsmultiset_new_skip() {
				use DirsIterable::{Dirstate, Manifest};

				let input_vec = ["a/", "b/", "a/c", "a/d/"]
				.iter()
				.map(\|e\| e.as_bytes().to_vec())
				.collect();
				let expected_inner = [("a", 3), ("b", 1), ("a/d", 1)]
				.iter()
				.map(\|(k, v)\| (k.as_bytes().to_vec(), *v))
				.collect();

				let new = DirsMultiset::new(Manifest(input_vec), Some('n' as i8));
				let expected = DirsMultiset {
				inner: expected_inner,
				};
				// Skip does not affect a manifest
				assert_eq!(expected, new);

				let input_map =
				[("a/", 'n'), ("a/b/", 'n'), ("a/c", 'r'), ("a/d/", 'm')]
				.iter()
				.map(\|(f, state)\| {
				(
				f.as_bytes().to_vec(),
				DirstateEntry {
				state: *state as i8,
				mode: 0,
				mtime: 0,
				size: 0,
				},
				)
				})
				.collect();

				// "a" incremented with "a/c" and "a/d/"
				let expected_inner = [("a", 2), ("a/d", 1)]
				.iter()
				.map(\|(k, v)\| (k.as_bytes().to_vec(), *v))
				.collect();

				let new = DirsMultiset::new(Dirstate(input_map), Some('n' as i8));
				let expected = DirsMultiset {
				inner: expected_inner,
				};
				assert_eq!(expected, new);
				}

				}

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

				pub mod dirs_multiset;
	pub mod parsers;			pub mod parsers;

	#[derive(Debug, PartialEq, Copy, Clone)]			#[derive(Debug, PartialEq, Copy, Clone)]
	pub struct DirstateParents<'a> {			pub struct DirstateParents<'a> {
	pub p1: &'a [u8],			pub p1: &'a [u8],
	pub p2: &'a [u8],			pub p2: &'a [u8],
	}			}


	#[derive(Debug, PartialEq)]			#[derive(Debug, PartialEq)]
	pub struct CopyVecEntry<'a> {			pub struct CopyVecEntry<'a> {
	pub path: &'a [u8],			pub path: &'a [u8],
	pub copy_path: &'a [u8],			pub copy_path: &'a [u8],
	}			}

	pub type CopyVec<'a> = Vec<CopyVecEntry<'a>>;			pub type CopyVec<'a> = Vec<CopyVecEntry<'a>>;

				/// The Python implementation passes either a mapping (dirstate) or a flat
				/// iterable (manifest)
				pub enum DirsIterable {
				Dirstate(DirstateVec),
				Manifest(Vec<Vec<u8>>),
				}

rust/hg-core/src/lib.rs


	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,
	parsers::{pack_dirstate, parse_dirstate},			parsers::{pack_dirstate, parse_dirstate},
	CopyVec, CopyVecEntry, DirstateEntry, DirstateParents, DirstateVec,			CopyVec, CopyVecEntry, DirsIterable, DirstateEntry, DirstateParents,
				DirstateVec,
	};			};
	mod filepatterns;			mod filepatterns;

	pub use filepatterns::{			pub use filepatterns::{
	build_single_regex, read_pattern_file, PatternSyntax, PatternTuple,			build_single_regex, read_pattern_file, PatternSyntax, PatternTuple,
	};			};

	/// Mercurial revision numbers			/// Mercurial revision numbers

	#[derive(Debug, PartialEq)]			#[derive(Debug, PartialEq)]
	pub enum DirstatePackError {			pub enum DirstatePackError {
	CorruptedEntry(String),			CorruptedEntry(String),
	CorruptedParent,			CorruptedParent,
	BadSize(usize, usize),			BadSize(usize, usize),
	}			}

				#[derive(Debug, PartialEq)]
				pub enum DirstateMapError {
				PathNotFound(Vec<u8>),
				EmptyPath,
				}

	impl From<std::io::Error> for DirstatePackError {			impl From<std::io::Error> for DirstatePackError {
	fn from(e: std::io::Error) -> Self {			fn from(e: std::io::Error) -> Self {
	DirstatePackError::CorruptedEntry(e.to_string())			DirstatePackError::CorruptedEntry(e.to_string())
	}			}
	}			}

	impl From<std::io::Error> for DirstateParseError {			impl From<std::io::Error> for DirstateParseError {
	fn from(e: std::io::Error) -> Self {			fn from(e: std::io::Error) -> Self {

		Path
A	M	rust/hg-core/src/dirstate/dirs_multiset.rs (342 lines)
M		rust/hg-core/src/dirstate/mod.rs (8 lines)
M		rust/hg-core/src/lib.rs (10 lines)

This is an archive of the discontinued Mercurial Phabricator instance.

rust-dirstate: add "dirs" Rust implementation
ClosedPublic

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 15160

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

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

rust/hg-core/src/lib.rs

This is an archive of the discontinued Mercurial Phabricator instance.

rust-dirstate: add "dirs" Rust implementationClosedPublic

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 15160

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

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

rust/hg-core/src/lib.rs

rust-dirstate: add "dirs" Rust implementation
ClosedPublic

Revision Contents
Changeset List