[Request] [+---- ] D11507: dirstate: drop all logic around the "non-normal" sets
SimonSapin
phabricator at mercurial-scm.org
Tue Sep 28 19:56:01 UTC 2021
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.
REVISION SUMMARY
The dirstate has a lot of code to compute a set of all "non-normal" and
"from_other_parent" entries.
This is all used in a single location, when `setparent` is called and moved from
a merge to a non merge. At that time, any "merge related" information has to be
dropped. This is mostly useful for command like `graft` or `shelve` that move to
a single-parent state -before- the commit. Otherwise the commit will already
have removed all traces of the merge information in the dirstate (e.g. for a
regular merges).
The bookkeeping for these set is quite invasive. And it seems simpler to just
drop it and do the full computation in the single location where we actually use
it (since we have to do the computation at least once anyway).
This simplify the code a lot, and clarify why since kind of computation is
needed.
The possible drawback from before are:
- if the operation happens in a loop, we will end up doing it multiple time,
- the C code to detect entry of interest have been dropped, for now. It will be re-introduced later, with a processing code directly in C for even faster operation.
I did not touch the Rust related code as I expect that Simon Sapin will take of
it in a larger scale refactoring.
REPOSITORY
rHG Mercurial
BRANCH
default
REVISION DETAIL
https://phab.mercurial-scm.org/D11507
AFFECTED FILES
contrib/dirstatenonnormalcheck.py
mercurial/cext/parsers.c
mercurial/dirstatemap.py
mercurial/pure/parsers.py
rust/hg-core/src/dirstate/dirstate_map.rs
rust/hg-core/src/dirstate/entry.rs
rust/hg-core/src/dirstate_tree/dirstate_map.rs
rust/hg-core/src/dirstate_tree/dispatch.rs
rust/hg-core/src/dirstate_tree/owning_dispatch.rs
rust/hg-cpython/src/dirstate.rs
rust/hg-cpython/src/dirstate/dirstate_map.rs
rust/hg-cpython/src/dirstate/item.rs
rust/hg-cpython/src/dirstate/non_normal_entries.rs
tests/test-dirstate-nonnormalset.t
CHANGE DETAILS
diff --git a/tests/test-dirstate-nonnormalset.t b/tests/test-dirstate-nonnormalset.t
deleted file mode 100644
--- a/tests/test-dirstate-nonnormalset.t
+++ /dev/null
@@ -1,22 +0,0 @@
- $ cat >> $HGRCPATH << EOF
- > [command-templates]
- > log="{rev}:{node|short} ({phase}) [{tags} {bookmarks}] {desc|firstline}\n"
- > [extensions]
- > dirstateparanoidcheck = $TESTDIR/../contrib/dirstatenonnormalcheck.py
- > [experimental]
- > nonnormalparanoidcheck = True
- > [devel]
- > all-warnings=True
- > EOF
- $ mkcommit() {
- > echo "$1" > "$1"
- > hg add "$1"
- > hg ci -m "add $1"
- > }
-
- $ hg init testrepo
- $ cd testrepo
- $ mkcommit a
- $ mkcommit b
- $ mkcommit c
- $ hg status
diff --git a/rust/hg-cpython/src/dirstate/non_normal_entries.rs b/rust/hg-cpython/src/dirstate/non_normal_entries.rs
deleted file mode 100644
--- a/rust/hg-cpython/src/dirstate/non_normal_entries.rs
+++ /dev/null
@@ -1,83 +0,0 @@
-// non_normal_other_parent_entries.rs
-//
-// Copyright 2020 Raphaël Gomès <rgomes at 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.
-
-use cpython::{
- exc::NotImplementedError, CompareOp, ObjectProtocol, PyBytes, PyClone,
- PyErr, PyObject, PyResult, PyString, Python, PythonObject, ToPyObject,
- UnsafePyLeaked,
-};
-
-use crate::dirstate::dirstate_map::v2_error;
-use crate::dirstate::DirstateMap;
-use hg::dirstate_tree::on_disk::DirstateV2ParseError;
-use hg::utils::hg_path::HgPath;
-use std::cell::RefCell;
-
-py_class!(pub class NonNormalEntries |py| {
- data dmap: DirstateMap;
-
- def __contains__(&self, key: PyObject) -> PyResult<bool> {
- self.dmap(py).non_normal_entries_contains(py, key)
- }
- def remove(&self, key: PyObject) -> PyResult<PyObject> {
- self.dmap(py).non_normal_entries_remove(py, key)
- }
- def add(&self, key: PyObject) -> PyResult<PyObject> {
- self.dmap(py).non_normal_entries_add(py, key)
- }
- def discard(&self, key: PyObject) -> PyResult<PyObject> {
- self.dmap(py).non_normal_entries_discard(py, key)
- }
- def __richcmp__(&self, other: PyObject, op: CompareOp) -> PyResult<bool> {
- match op {
- CompareOp::Eq => self.is_equal_to(py, other),
- CompareOp::Ne => Ok(!self.is_equal_to(py, other)?),
- _ => Err(PyErr::new::<NotImplementedError, _>(py, ""))
- }
- }
- def __repr__(&self) -> PyResult<PyString> {
- self.dmap(py).non_normal_entries_display(py)
- }
-
- def __iter__(&self) -> PyResult<NonNormalEntriesIterator> {
- self.dmap(py).non_normal_entries_iter(py)
- }
-});
-
-impl NonNormalEntries {
- pub fn from_inner(py: Python, dm: DirstateMap) -> PyResult<Self> {
- Self::create_instance(py, dm)
- }
-
- fn is_equal_to(&self, py: Python, other: PyObject) -> PyResult<bool> {
- for item in other.iter(py)? {
- if !self.dmap(py).non_normal_entries_contains(py, item?)? {
- return Ok(false);
- }
- }
- Ok(true)
- }
-
- fn translate_key(
- py: Python,
- key: Result<&HgPath, DirstateV2ParseError>,
- ) -> PyResult<Option<PyBytes>> {
- let key = key.map_err(|e| v2_error(py, e))?;
- Ok(Some(PyBytes::new(py, key.as_bytes())))
- }
-}
-
-type NonNormalEntriesIter<'a> = Box<
- dyn Iterator<Item = Result<&'a HgPath, DirstateV2ParseError>> + Send + 'a,
->;
-
-py_shared_iterator!(
- NonNormalEntriesIterator,
- UnsafePyLeaked<NonNormalEntriesIter<'static>>,
- NonNormalEntries::translate_key,
- Option<PyBytes>
-);
diff --git a/rust/hg-cpython/src/dirstate/item.rs b/rust/hg-cpython/src/dirstate/item.rs
--- a/rust/hg-cpython/src/dirstate/item.rs
+++ b/rust/hg-cpython/src/dirstate/item.rs
@@ -95,16 +95,6 @@
Ok(self.entry(py).get().from_p2_removed())
}
- @property
- def dm_nonnormal(&self) -> PyResult<bool> {
- Ok(self.entry(py).get().is_non_normal())
- }
-
- @property
- def dm_otherparent(&self) -> PyResult<bool> {
- Ok(self.entry(py).get().is_from_other_parent())
- }
-
def v1_state(&self) -> PyResult<PyBytes> {
let (state, _mode, _size, _mtime) = self.entry(py).get().v1_data();
let state_byte: u8 = state.into();
diff --git a/rust/hg-cpython/src/dirstate/dirstate_map.rs b/rust/hg-cpython/src/dirstate/dirstate_map.rs
--- a/rust/hg-cpython/src/dirstate/dirstate_map.rs
+++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs
@@ -13,16 +13,12 @@
use cpython::{
exc, PyBool, PyBytes, PyClone, PyDict, PyErr, PyList, PyNone, PyObject,
- PyResult, PySet, PyString, Python, PythonObject, ToPyObject,
- UnsafePyLeaked,
+ PyResult, Python, PythonObject, ToPyObject, UnsafePyLeaked,
};
use crate::{
dirstate::copymap::{CopyMap, CopyMapItemsIterator, CopyMapKeysIterator},
dirstate::item::DirstateItem,
- dirstate::non_normal_entries::{
- NonNormalEntries, NonNormalEntriesIterator,
- },
pybytes_deref::PyBytesDeref,
};
use hg::{
@@ -185,100 +181,6 @@
Ok(PyNone)
}
- def other_parent_entries(&self) -> PyResult<PyObject> {
- let mut inner_shared = self.inner(py).borrow_mut();
- let set = PySet::empty(py)?;
- for path in inner_shared.iter_other_parent_paths() {
- let path = path.map_err(|e| v2_error(py, e))?;
- set.add(py, PyBytes::new(py, path.as_bytes()))?;
- }
- Ok(set.into_object())
- }
-
- def non_normal_entries(&self) -> PyResult<NonNormalEntries> {
- NonNormalEntries::from_inner(py, self.clone_ref(py))
- }
-
- def non_normal_entries_contains(&self, key: PyObject) -> PyResult<bool> {
- let key = key.extract::<PyBytes>(py)?;
- self.inner(py)
- .borrow_mut()
- .non_normal_entries_contains(HgPath::new(key.data(py)))
- .map_err(|e| v2_error(py, e))
- }
-
- def non_normal_entries_display(&self) -> PyResult<PyString> {
- let mut inner = self.inner(py).borrow_mut();
- let paths = inner
- .iter_non_normal_paths()
- .collect::<Result<Vec<_>, _>>()
- .map_err(|e| v2_error(py, e))?;
- let formatted = format!("NonNormalEntries: {}", hg::utils::join_display(paths, ", "));
- Ok(PyString::new(py, &formatted))
- }
-
- def non_normal_entries_remove(&self, key: PyObject) -> PyResult<PyObject> {
- let key = key.extract::<PyBytes>(py)?;
- let key = key.data(py);
- let was_present = self
- .inner(py)
- .borrow_mut()
- .non_normal_entries_remove(HgPath::new(key));
- if !was_present {
- let msg = String::from_utf8_lossy(key);
- Err(PyErr::new::<exc::KeyError, _>(py, msg))
- } else {
- Ok(py.None())
- }
- }
-
- def non_normal_entries_discard(&self, key: PyObject) -> PyResult<PyObject>
- {
- let key = key.extract::<PyBytes>(py)?;
- self
- .inner(py)
- .borrow_mut()
- .non_normal_entries_remove(HgPath::new(key.data(py)));
- Ok(py.None())
- }
-
- def non_normal_entries_add(&self, key: PyObject) -> PyResult<PyObject> {
- let key = key.extract::<PyBytes>(py)?;
- self
- .inner(py)
- .borrow_mut()
- .non_normal_entries_add(HgPath::new(key.data(py)));
- Ok(py.None())
- }
-
- def non_normal_or_other_parent_paths(&self) -> PyResult<PyList> {
- let mut inner = self.inner(py).borrow_mut();
-
- let ret = PyList::new(py, &[]);
- for filename in inner.non_normal_or_other_parent_paths() {
- let filename = filename.map_err(|e| v2_error(py, e))?;
- let as_pystring = PyBytes::new(py, filename.as_bytes());
- ret.append(py, as_pystring.into_object());
- }
- Ok(ret)
- }
-
- def non_normal_entries_iter(&self) -> PyResult<NonNormalEntriesIterator> {
- // Make sure the sets are defined before we no longer have a mutable
- // reference to the dmap.
- self.inner(py)
- .borrow_mut()
- .set_non_normal_other_parent_entries(false);
-
- let leaked_ref = self.inner(py).leak_immutable();
-
- NonNormalEntriesIterator::from_inner(py, unsafe {
- leaked_ref.map(py, |o| {
- o.iter_non_normal_paths_panic()
- })
- })
- }
-
def hastrackeddir(&self, d: PyObject) -> PyResult<PyBool> {
let d = d.extract::<PyBytes>(py)?;
Ok(self.inner(py).borrow_mut()
diff --git a/rust/hg-cpython/src/dirstate.rs b/rust/hg-cpython/src/dirstate.rs
--- a/rust/hg-cpython/src/dirstate.rs
+++ b/rust/hg-cpython/src/dirstate.rs
@@ -13,7 +13,6 @@
mod dirs_multiset;
mod dirstate_map;
mod item;
-mod non_normal_entries;
mod status;
use self::item::DirstateItem;
use crate::{
diff --git a/rust/hg-core/src/dirstate_tree/owning_dispatch.rs b/rust/hg-core/src/dirstate_tree/owning_dispatch.rs
--- a/rust/hg-core/src/dirstate_tree/owning_dispatch.rs
+++ b/rust/hg-core/src/dirstate_tree/owning_dispatch.rs
@@ -51,56 +51,6 @@
self.get_mut().drop_entry_and_copy_source(filename)
}
- fn non_normal_entries_contains(
- &mut self,
- key: &HgPath,
- ) -> Result<bool, DirstateV2ParseError> {
- self.get_mut().non_normal_entries_contains(key)
- }
-
- fn non_normal_entries_remove(&mut self, key: &HgPath) -> bool {
- self.get_mut().non_normal_entries_remove(key)
- }
-
- fn non_normal_entries_add(&mut self, key: &HgPath) {
- self.get_mut().non_normal_entries_add(key)
- }
-
- fn non_normal_or_other_parent_paths(
- &mut self,
- ) -> Box<dyn Iterator<Item = Result<&HgPath, DirstateV2ParseError>> + '_>
- {
- self.get_mut().non_normal_or_other_parent_paths()
- }
-
- fn set_non_normal_other_parent_entries(&mut self, force: bool) {
- self.get_mut().set_non_normal_other_parent_entries(force)
- }
-
- fn iter_non_normal_paths(
- &mut self,
- ) -> Box<
- dyn Iterator<Item = Result<&HgPath, DirstateV2ParseError>> + Send + '_,
- > {
- self.get_mut().iter_non_normal_paths()
- }
-
- fn iter_non_normal_paths_panic(
- &self,
- ) -> Box<
- dyn Iterator<Item = Result<&HgPath, DirstateV2ParseError>> + Send + '_,
- > {
- self.get().iter_non_normal_paths_panic()
- }
-
- fn iter_other_parent_paths(
- &mut self,
- ) -> Box<
- dyn Iterator<Item = Result<&HgPath, DirstateV2ParseError>> + Send + '_,
- > {
- self.get_mut().iter_other_parent_paths()
- }
-
fn has_tracked_dir(
&mut self,
directory: &HgPath,
diff --git a/rust/hg-core/src/dirstate_tree/dispatch.rs b/rust/hg-core/src/dirstate_tree/dispatch.rs
--- a/rust/hg-core/src/dirstate_tree/dispatch.rs
+++ b/rust/hg-core/src/dirstate_tree/dispatch.rs
@@ -67,82 +67,6 @@
filename: &HgPath,
) -> Result<(), DirstateError>;
- /// Return whether the map has an "non-normal" entry for the given
- /// filename. That is, any entry with a `state` other than
- /// `EntryState::Normal` or with an ambiguous `mtime`.
- fn non_normal_entries_contains(
- &mut self,
- key: &HgPath,
- ) -> Result<bool, DirstateV2ParseError>;
-
- /// Mark the given path as "normal" file. This is only relevant in the flat
- /// dirstate map where there is a separate `HashSet` that needs to be kept
- /// up to date.
- /// Returns whether the key was present in the set.
- fn non_normal_entries_remove(&mut self, key: &HgPath) -> bool;
-
- /// Mark the given path as "non-normal" file.
- /// This is only relevant in the flat dirstate map where there is a
- /// separate `HashSet` that needs to be kept up to date.
- fn non_normal_entries_add(&mut self, key: &HgPath);
-
- /// Return an iterator of paths whose respective entry are either
- /// "non-normal" (see `non_normal_entries_contains`) or "from other
- /// parent".
- ///
- /// If that information is cached, create the cache as needed.
- ///
- /// "From other parent" is defined as `state == Normal && size == -2`.
- ///
- /// Because parse errors can happen during iteration, the iterated items
- /// are `Result`s.
- fn non_normal_or_other_parent_paths(
- &mut self,
- ) -> Box<dyn Iterator<Item = Result<&HgPath, DirstateV2ParseError>> + '_>;
-
- /// Create the cache for `non_normal_or_other_parent_paths` if needed.
- ///
- /// If `force` is true, the cache is re-created even if it already exists.
- fn set_non_normal_other_parent_entries(&mut self, force: bool);
-
- /// Return an iterator of paths whose respective entry are "non-normal"
- /// (see `non_normal_entries_contains`).
- ///
- /// If that information is cached, create the cache as needed.
- ///
- /// Because parse errors can happen during iteration, the iterated items
- /// are `Result`s.
- fn iter_non_normal_paths(
- &mut self,
- ) -> Box<
- dyn Iterator<Item = Result<&HgPath, DirstateV2ParseError>> + Send + '_,
- >;
-
- /// Same as `iter_non_normal_paths`, but takes `&self` instead of `&mut
- /// self`.
- ///
- /// Panics if a cache is necessary but does not exist yet.
- fn iter_non_normal_paths_panic(
- &self,
- ) -> Box<
- dyn Iterator<Item = Result<&HgPath, DirstateV2ParseError>> + Send + '_,
- >;
-
- /// Return an iterator of paths whose respective entry are "from other
- /// parent".
- ///
- /// If that information is cached, create the cache as needed.
- ///
- /// "From other parent" is defined as `state == Normal && size == -2`.
- ///
- /// Because parse errors can happen during iteration, the iterated items
- /// are `Result`s.
- fn iter_other_parent_paths(
- &mut self,
- ) -> Box<
- dyn Iterator<Item = Result<&HgPath, DirstateV2ParseError>> + Send + '_,
- >;
-
/// Returns whether the sub-tree rooted at the given directory contains any
/// tracked file.
///
@@ -330,66 +254,6 @@
self.drop_entry_and_copy_source(filename)
}
- fn non_normal_entries_contains(
- &mut self,
- key: &HgPath,
- ) -> Result<bool, DirstateV2ParseError> {
- let (non_normal, _other_parent) =
- self.get_non_normal_other_parent_entries();
- Ok(non_normal.contains(key))
- }
-
- fn non_normal_entries_remove(&mut self, key: &HgPath) -> bool {
- self.non_normal_entries_remove(key)
- }
-
- fn non_normal_entries_add(&mut self, key: &HgPath) {
- self.non_normal_entries_add(key)
- }
-
- fn non_normal_or_other_parent_paths(
- &mut self,
- ) -> Box<dyn Iterator<Item = Result<&HgPath, DirstateV2ParseError>> + '_>
- {
- let (non_normal, other_parent) =
- self.get_non_normal_other_parent_entries();
- Box::new(non_normal.union(other_parent).map(|p| Ok(&**p)))
- }
-
- fn set_non_normal_other_parent_entries(&mut self, force: bool) {
- self.set_non_normal_other_parent_entries(force)
- }
-
- fn iter_non_normal_paths(
- &mut self,
- ) -> Box<
- dyn Iterator<Item = Result<&HgPath, DirstateV2ParseError>> + Send + '_,
- > {
- let (non_normal, _other_parent) =
- self.get_non_normal_other_parent_entries();
- Box::new(non_normal.iter().map(|p| Ok(&**p)))
- }
-
- fn iter_non_normal_paths_panic(
- &self,
- ) -> Box<
- dyn Iterator<Item = Result<&HgPath, DirstateV2ParseError>> + Send + '_,
- > {
- let (non_normal, _other_parent) =
- self.get_non_normal_other_parent_entries_panic();
- Box::new(non_normal.iter().map(|p| Ok(&**p)))
- }
-
- fn iter_other_parent_paths(
- &mut self,
- ) -> Box<
- dyn Iterator<Item = Result<&HgPath, DirstateV2ParseError>> + Send + '_,
- > {
- let (_non_normal, other_parent) =
- self.get_non_normal_other_parent_entries();
- Box::new(other_parent.iter().map(|p| Ok(&**p)))
- }
-
fn has_tracked_dir(
&mut self,
directory: &HgPath,
@@ -406,7 +270,7 @@
parents: DirstateParents,
now: Timestamp,
) -> Result<Vec<u8>, DirstateError> {
- self.pack(parents, now)
+ Ok(self.pack(parents, now)?)
}
fn pack_v2(
diff --git a/rust/hg-core/src/dirstate_tree/dirstate_map.rs b/rust/hg-core/src/dirstate_tree/dirstate_map.rs
--- a/rust/hg-core/src/dirstate_tree/dirstate_map.rs
+++ b/rust/hg-core/src/dirstate_tree/dirstate_map.rs
@@ -701,27 +701,6 @@
Ok(())
}
- /// Return a faillilble iterator of full paths of nodes that have an
- /// `entry` for which the given `predicate` returns true.
- ///
- /// Fallibility means that each iterator item is a `Result`, which may
- /// indicate a parse error of the on-disk dirstate-v2 format. Such errors
- /// should only happen if Mercurial is buggy or a repository is corrupted.
- fn filter_full_paths<'tree>(
- &'tree self,
- predicate: impl Fn(&DirstateEntry) -> bool + 'tree,
- ) -> impl Iterator<Item = Result<&HgPath, DirstateV2ParseError>> + 'tree
- {
- filter_map_results(self.iter_nodes(), move |node| {
- if let Some(entry) = node.entry()? {
- if predicate(&entry) {
- return Ok(Some(node.full_path(self.on_disk)?));
- }
- }
- Ok(None)
- })
- }
-
fn count_dropped_path(unreachable_bytes: &mut u32, path: &Cow<HgPath>) {
if let Cow::Borrowed(path) = path {
*unreachable_bytes += path.len() as u32
@@ -917,69 +896,6 @@
Ok(())
}
- fn non_normal_entries_contains(
- &mut self,
- key: &HgPath,
- ) -> Result<bool, DirstateV2ParseError> {
- Ok(if let Some(node) = self.get_node(key)? {
- node.entry()?.map_or(false, |entry| entry.is_non_normal())
- } else {
- false
- })
- }
-
- fn non_normal_entries_remove(&mut self, key: &HgPath) -> bool {
- // Do nothing, this `DirstateMap` does not have a separate "non normal
- // entries" set that need to be kept up to date.
- if let Ok(Some(v)) = self.get(key) {
- return v.is_non_normal();
- }
- false
- }
-
- fn non_normal_entries_add(&mut self, _key: &HgPath) {
- // Do nothing, this `DirstateMap` does not have a separate "non normal
- // entries" set that need to be kept up to date
- }
-
- fn non_normal_or_other_parent_paths(
- &mut self,
- ) -> Box<dyn Iterator<Item = Result<&HgPath, DirstateV2ParseError>> + '_>
- {
- Box::new(self.filter_full_paths(|entry| {
- entry.is_non_normal() || entry.is_from_other_parent()
- }))
- }
-
- fn set_non_normal_other_parent_entries(&mut self, _force: bool) {
- // Do nothing, this `DirstateMap` does not have a separate "non normal
- // entries" and "from other parent" sets that need to be recomputed
- }
-
- fn iter_non_normal_paths(
- &mut self,
- ) -> Box<
- dyn Iterator<Item = Result<&HgPath, DirstateV2ParseError>> + Send + '_,
- > {
- self.iter_non_normal_paths_panic()
- }
-
- fn iter_non_normal_paths_panic(
- &self,
- ) -> Box<
- dyn Iterator<Item = Result<&HgPath, DirstateV2ParseError>> + Send + '_,
- > {
- Box::new(self.filter_full_paths(|entry| entry.is_non_normal()))
- }
-
- fn iter_other_parent_paths(
- &mut self,
- ) -> Box<
- dyn Iterator<Item = Result<&HgPath, DirstateV2ParseError>> + Send + '_,
- > {
- Box::new(self.filter_full_paths(|entry| entry.is_from_other_parent()))
- }
-
fn has_tracked_dir(
&mut self,
directory: &HgPath,
diff --git a/rust/hg-core/src/dirstate/entry.rs b/rust/hg-core/src/dirstate/entry.rs
--- a/rust/hg-core/src/dirstate/entry.rs
+++ b/rust/hg-core/src/dirstate/entry.rs
@@ -294,11 +294,7 @@
(self.state().into(), self.mode(), self.size(), self.mtime())
}
- pub fn is_non_normal(&self) -> bool {
- self.state() != EntryState::Normal || self.mtime() == MTIME_UNSET
- }
-
- pub fn is_from_other_parent(&self) -> bool {
+ pub(crate) fn is_from_other_parent(&self) -> bool {
self.state() == EntryState::Normal
&& self.size() == SIZE_FROM_OTHER_PARENT
}
diff --git a/rust/hg-core/src/dirstate/dirstate_map.rs b/rust/hg-core/src/dirstate/dirstate_map.rs
--- a/rust/hg-core/src/dirstate/dirstate_map.rs
+++ b/rust/hg-core/src/dirstate/dirstate_map.rs
@@ -6,6 +6,7 @@
// GNU General Public License version 2 or any later version.
use crate::dirstate::parsers::Timestamp;
+use crate::errors::HgError;
use crate::{
dirstate::EntryState,
dirstate::SIZE_FROM_OTHER_PARENT,
@@ -16,7 +17,6 @@
StateMap,
};
use micro_timer::timed;
-use std::collections::HashSet;
use std::iter::FromIterator;
use std::ops::Deref;
@@ -26,8 +26,6 @@
pub copy_map: CopyMap,
pub dirs: Option<DirsMultiset>,
pub all_dirs: Option<DirsMultiset>,
- non_normal_set: Option<HashSet<HgPathBuf>>,
- other_parent_set: Option<HashSet<HgPathBuf>>,
}
/// Should only really be used in python interface code, for clarity
@@ -58,8 +56,6 @@
pub fn clear(&mut self) {
self.state_map = StateMap::default();
self.copy_map.clear();
- self.non_normal_set = None;
- self.other_parent_set = None;
}
pub fn set_entry(&mut self, filename: &HgPath, entry: DirstateEntry) {
@@ -84,18 +80,6 @@
}
}
self.state_map.insert(filename.to_owned(), entry.to_owned());
-
- if entry.is_non_normal() {
- self.get_non_normal_other_parent_entries()
- .0
- .insert(filename.to_owned());
- }
-
- if entry.is_from_other_parent() {
- self.get_non_normal_other_parent_entries()
- .1
- .insert(filename.to_owned());
- }
Ok(())
}
@@ -126,9 +110,6 @@
{
// other parent
size = SIZE_FROM_OTHER_PARENT;
- self.get_non_normal_other_parent_entries()
- .1
- .insert(filename.to_owned());
}
}
}
@@ -148,9 +129,6 @@
self.state_map
.insert(filename.to_owned(), DirstateEntry::new_removed(size));
- self.get_non_normal_other_parent_entries()
- .0
- .insert(filename.to_owned());
Ok(())
}
@@ -173,92 +151,11 @@
all_dirs.delete_path(filename)?;
}
}
- self.get_non_normal_other_parent_entries()
- .0
- .remove(filename);
-
self.copy_map.remove(filename);
Ok(())
}
- pub fn non_normal_entries_remove(
- &mut self,
- key: impl AsRef<HgPath>,
- ) -> bool {
- self.get_non_normal_other_parent_entries()
- .0
- .remove(key.as_ref())
- }
-
- pub fn non_normal_entries_add(&mut self, key: impl AsRef<HgPath>) {
- self.get_non_normal_other_parent_entries()
- .0
- .insert(key.as_ref().into());
- }
-
- pub fn non_normal_entries_union(
- &mut self,
- other: HashSet<HgPathBuf>,
- ) -> Vec<HgPathBuf> {
- self.get_non_normal_other_parent_entries()
- .0
- .union(&other)
- .map(ToOwned::to_owned)
- .collect()
- }
-
- pub fn get_non_normal_other_parent_entries(
- &mut self,
- ) -> (&mut HashSet<HgPathBuf>, &mut HashSet<HgPathBuf>) {
- self.set_non_normal_other_parent_entries(false);
- (
- self.non_normal_set.as_mut().unwrap(),
- self.other_parent_set.as_mut().unwrap(),
- )
- }
-
- /// Useful to get immutable references to those sets in contexts where
- /// you only have an immutable reference to the `DirstateMap`, like when
- /// sharing references with Python.
- ///
- /// TODO, get rid of this along with the other "setter/getter" stuff when
- /// a nice typestate plan is defined.
- ///
- /// # Panics
- ///
- /// Will panic if either set is `None`.
- pub fn get_non_normal_other_parent_entries_panic(
- &self,
- ) -> (&HashSet<HgPathBuf>, &HashSet<HgPathBuf>) {
- (
- self.non_normal_set.as_ref().unwrap(),
- self.other_parent_set.as_ref().unwrap(),
- )
- }
-
- pub fn set_non_normal_other_parent_entries(&mut self, force: bool) {
- if !force
- && self.non_normal_set.is_some()
- && self.other_parent_set.is_some()
- {
- return;
- }
- let mut non_normal = HashSet::new();
- let mut other_parent = HashSet::new();
-
- for (filename, entry) in self.state_map.iter() {
- if entry.is_non_normal() {
- non_normal.insert(filename.to_owned());
- }
- if entry.is_from_other_parent() {
- other_parent.insert(filename.to_owned());
- }
- }
- self.non_normal_set = Some(non_normal);
- self.other_parent_set = Some(other_parent);
- }
-
/// Both of these setters and their uses appear to be the simplest way to
/// emulate a Python lazy property, but it is ugly and unidiomatic.
/// TODO One day, rewriting this struct using the typestate might be a
@@ -326,12 +223,8 @@
&mut self,
parents: DirstateParents,
now: Timestamp,
- ) -> Result<Vec<u8>, DirstateError> {
- let packed =
- pack_dirstate(&mut self.state_map, &self.copy_map, parents, now)?;
-
- self.set_non_normal_other_parent_entries(true);
- Ok(packed)
+ ) -> Result<Vec<u8>, HgError> {
+ pack_dirstate(&mut self.state_map, &self.copy_map, parents, now)
}
}
@@ -366,49 +259,5 @@
.unwrap();
assert_eq!(1, map.len());
- assert_eq!(0, map.get_non_normal_other_parent_entries().0.len());
- assert_eq!(0, map.get_non_normal_other_parent_entries().1.len());
- }
-
- #[test]
- fn test_non_normal_other_parent_entries() {
- let mut map: DirstateMap = [
- (b"f1", (EntryState::Removed, 1337, 1337, 1337)),
- (b"f2", (EntryState::Normal, 1337, 1337, -1)),
- (b"f3", (EntryState::Normal, 1337, 1337, 1337)),
- (b"f4", (EntryState::Normal, 1337, -2, 1337)),
- (b"f5", (EntryState::Added, 1337, 1337, 1337)),
- (b"f6", (EntryState::Added, 1337, 1337, -1)),
- (b"f7", (EntryState::Merged, 1337, 1337, -1)),
- (b"f8", (EntryState::Merged, 1337, 1337, 1337)),
- (b"f9", (EntryState::Merged, 1337, -2, 1337)),
- (b"fa", (EntryState::Added, 1337, -2, 1337)),
- (b"fb", (EntryState::Removed, 1337, -2, 1337)),
- ]
- .iter()
- .map(|(fname, (state, mode, size, mtime))| {
- (
- HgPathBuf::from_bytes(fname.as_ref()),
- DirstateEntry::from_v1_data(*state, *mode, *size, *mtime),
- )
- })
- .collect();
-
- let mut non_normal = [
- b"f1", b"f2", b"f4", b"f5", b"f6", b"f7", b"f8", b"f9", b"fa",
- b"fb",
- ]
- .iter()
- .map(|x| HgPathBuf::from_bytes(x.as_ref()))
- .collect();
-
- let mut other_parent = HashSet::new();
- other_parent.insert(HgPathBuf::from_bytes(b"f4"));
- let entries = map.get_non_normal_other_parent_entries();
-
- assert_eq!(
- (&mut non_normal, &mut other_parent),
- (entries.0, entries.1)
- );
}
}
diff --git a/mercurial/pure/parsers.py b/mercurial/pure/parsers.py
--- a/mercurial/pure/parsers.py
+++ b/mercurial/pure/parsers.py
@@ -361,22 +361,6 @@
"""
return self.removed and self._merged
- @property
- def dm_nonnormal(self):
- """True is the entry is non-normal in the dirstatemap sense
-
- There is no reason for any code, but the dirstatemap one to use this.
- """
- return self.v1_state() != b'n' or self.v1_mtime() == AMBIGUOUS_TIME
-
- @property
- def dm_otherparent(self):
- """True is the entry is `otherparent` in the dirstatemap sense
-
- There is no reason for any code, but the dirstatemap one to use this.
- """
- return self.v1_size() == FROM_P2
-
def v1_state(self):
"""return a "state" suitable for v1 serialization"""
if not (self._p1_tracked or self._p2_tracked or self._wc_tracked):
diff --git a/mercurial/dirstatemap.py b/mercurial/dirstatemap.py
--- a/mercurial/dirstatemap.py
+++ b/mercurial/dirstatemap.py
@@ -62,12 +62,6 @@
The dirstate also provides the following views onto the state:
- - `nonnormalset` is a set of the filenames that have state other
- than 'normal', or are normal but have an mtime of -1 ('normallookup').
-
- - `otherparentset` is a set of the filenames that are marked as coming
- from the second parent when the dirstate is currently being merged.
-
- `filefoldmap` is a dict mapping normalized filenames to the denormalized
form that they appear as in the dirstate.
@@ -112,8 +106,6 @@
util.clearcachedproperty(self, b"_alldirs")
util.clearcachedproperty(self, b"filefoldmap")
util.clearcachedproperty(self, b"dirfoldmap")
- util.clearcachedproperty(self, b"nonnormalset")
- util.clearcachedproperty(self, b"otherparentset")
def items(self):
return pycompat.iteritems(self._map)
@@ -185,7 +177,6 @@
size = size & rangemask
entry.set_clean(mode, size, mtime)
self.copymap.pop(filename, None)
- self.nonnormalset.discard(filename)
def reset_state(
self,
@@ -218,7 +209,6 @@
if not (p1_tracked or p2_tracked or wc_tracked):
old_entry = self._map.pop(filename, None)
self._dirs_decr(filename, old_entry=old_entry)
- self.nonnormalset.discard(filename)
self.copymap.pop(filename, None)
return
elif merged:
@@ -271,14 +261,6 @@
possibly_dirty=possibly_dirty,
parentfiledata=parentfiledata,
)
- if entry.dm_nonnormal:
- self.nonnormalset.add(filename)
- else:
- self.nonnormalset.discard(filename)
- if entry.dm_otherparent:
- self.otherparentset.add(filename)
- else:
- self.otherparentset.discard(filename)
self._map[filename] = entry
def set_tracked(self, filename):
@@ -297,8 +279,6 @@
parentfiledata=None,
)
self._map[filename] = entry
- if entry.dm_nonnormal:
- self.nonnormalset.add(filename)
new = True
elif not entry.tracked:
self._dirs_incr(filename, entry)
@@ -321,29 +301,11 @@
if not entry.merged:
self.copymap.pop(f, None)
if entry.added:
- self.nonnormalset.discard(f)
self._map.pop(f, None)
else:
- self.nonnormalset.add(f)
- if entry.from_p2:
- self.otherparentset.add(f)
entry.set_untracked()
return True
- def nonnormalentries(self):
- '''Compute the nonnormal dirstate entries from the dmap'''
- try:
- return parsers.nonnormalotherparententries(self._map)
- except AttributeError:
- nonnorm = set()
- otherparent = set()
- for fname, e in pycompat.iteritems(self._map):
- if e.dm_nonnormal:
- nonnorm.add(fname)
- if e.from_p2:
- otherparent.add(fname)
- return nonnorm, otherparent
-
@propertycache
def filefoldmap(self):
"""Returns a dictionary mapping normalized case paths to their
@@ -433,13 +395,7 @@
self._dirtyparents = True
copies = {}
if fold_p2:
- candidatefiles = self.non_normal_or_other_parent_paths()
-
- for f in candidatefiles:
- s = self.get(f)
- if s is None:
- continue
-
+ for f, s in pycompat.iteritems(self._map):
# Discard "merged" markers when moving away from a merge state
if s.merged or s.from_p2:
source = self.copymap.pop(f, None)
@@ -504,22 +460,6 @@
)
st.close()
self._dirtyparents = False
- self.nonnormalset, self.otherparentset = self.nonnormalentries()
-
- @propertycache
- def nonnormalset(self):
- nonnorm, otherparents = self.nonnormalentries()
- self.otherparentset = otherparents
- return nonnorm
-
- @propertycache
- def otherparentset(self):
- nonnorm, otherparents = self.nonnormalentries()
- self.nonnormalset = nonnorm
- return otherparents
-
- def non_normal_or_other_parent_paths(self):
- return self.nonnormalset.union(self.otherparentset)
@propertycache
def identity(self):
@@ -643,7 +583,6 @@
elif (p1_tracked or p2_tracked) and not wc_tracked:
# XXX might be merged and removed ?
self[filename] = DirstateItem.from_v1_data(b'r', 0, 0, 0)
- self.nonnormalset.add(filename)
elif clean_p2 and wc_tracked:
if p1_tracked or self.get(filename) is not None:
# XXX the `self.get` call is catching some case in
@@ -670,7 +609,6 @@
raise error.ProgrammingError(msg)
mode, size, mtime = parentfiledata
self.addfile(filename, mode=mode, size=size, mtime=mtime)
- self.nonnormalset.discard(filename)
else:
assert False, 'unreachable'
@@ -710,9 +648,6 @@
def removefile(self, *args, **kwargs):
return self._rustmap.removefile(*args, **kwargs)
- def nonnormalentries(self):
- return self._rustmap.nonnormalentries()
-
def get(self, *args, **kwargs):
return self._rustmap.get(*args, **kwargs)
@@ -790,13 +725,17 @@
self._dirtyparents = True
copies = {}
if fold_p2:
- candidatefiles = self.non_normal_or_other_parent_paths()
-
- for f in candidatefiles:
- s = self.get(f)
- if s is None:
- continue
-
+ # Collect into an intermediate list to avoid a `RuntimeError`
+ # exception due to mutation during iteration.
+ # TODO: move this the whole loop to Rust where `iter_mut`
+ # enables in-place mutation of elements of a collection while
+ # iterating it, without mutating the collection itself.
+ candidatefiles = [
+ (f, s)
+ for f, s in self._rustmap.items()
+ if s.merged or s.from_p2
+ ]
+ for f, s in candidatefiles:
# Discard "merged" markers when moving away from a merge state
if s.merged:
source = self.copymap.get(f)
@@ -965,19 +904,6 @@
self._rustmap
return self.identity
- @property
- def nonnormalset(self):
- nonnorm = self._rustmap.non_normal_entries()
- return nonnorm
-
- @propertycache
- def otherparentset(self):
- otherparents = self._rustmap.other_parent_entries()
- return otherparents
-
- def non_normal_or_other_parent_paths(self):
- return self._rustmap.non_normal_or_other_parent_paths()
-
@propertycache
def dirfoldmap(self):
f = {}
diff --git a/mercurial/cext/parsers.c b/mercurial/cext/parsers.c
--- a/mercurial/cext/parsers.c
+++ b/mercurial/cext/parsers.c
@@ -661,24 +661,6 @@
}
};
-static PyObject *dm_nonnormal(dirstateItemObject *self)
-{
- if ((dirstate_item_c_v1_state(self) != 'n') ||
- (dirstate_item_c_v1_mtime(self) == ambiguous_time)) {
- Py_RETURN_TRUE;
- } else {
- Py_RETURN_FALSE;
- }
-};
-static PyObject *dm_otherparent(dirstateItemObject *self)
-{
- if (dirstate_item_c_v1_mtime(self) == dirstate_v1_from_p2) {
- Py_RETURN_TRUE;
- } else {
- Py_RETURN_FALSE;
- }
-};
-
static PyGetSetDef dirstate_item_getset[] = {
{"mode", (getter)dirstate_item_get_mode, NULL, "mode", NULL},
{"size", (getter)dirstate_item_get_size, NULL, "size", NULL},
@@ -693,8 +675,6 @@
"from_p2_removed", NULL},
{"from_p2", (getter)dirstate_item_get_from_p2, NULL, "from_p2", NULL},
{"removed", (getter)dirstate_item_get_removed, NULL, "removed", NULL},
- {"dm_nonnormal", (getter)dm_nonnormal, NULL, "dm_nonnormal", NULL},
- {"dm_otherparent", (getter)dm_otherparent, NULL, "dm_otherparent", NULL},
{NULL} /* Sentinel */
};
@@ -831,70 +811,6 @@
}
/*
- * Build a set of non-normal and other parent entries from the dirstate dmap
- */
-static PyObject *nonnormalotherparententries(PyObject *self, PyObject *args)
-{
- PyObject *dmap, *fname, *v;
- PyObject *nonnset = NULL, *otherpset = NULL, *result = NULL;
- Py_ssize_t pos;
-
- if (!PyArg_ParseTuple(args, "O!:nonnormalentries", &PyDict_Type,
- &dmap)) {
- goto bail;
- }
-
- nonnset = PySet_New(NULL);
- if (nonnset == NULL) {
- goto bail;
- }
-
- otherpset = PySet_New(NULL);
- if (otherpset == NULL) {
- goto bail;
- }
-
- pos = 0;
- while (PyDict_Next(dmap, &pos, &fname, &v)) {
- dirstateItemObject *t;
- if (!dirstate_tuple_check(v)) {
- PyErr_SetString(PyExc_TypeError,
- "expected a dirstate tuple");
- goto bail;
- }
- t = (dirstateItemObject *)v;
-
- if (dirstate_item_c_from_p2(t)) {
- if (PySet_Add(otherpset, fname) == -1) {
- goto bail;
- }
- }
- if (!(t->flags & dirstate_flag_wc_tracked) ||
- !(t->flags &
- (dirstate_flag_p1_tracked | dirstate_flag_p2_tracked)) ||
- (t->flags &
- (dirstate_flag_possibly_dirty | dirstate_flag_merged))) {
- if (PySet_Add(nonnset, fname) == -1) {
- goto bail;
- }
- }
- }
-
- result = Py_BuildValue("(OO)", nonnset, otherpset);
- if (result == NULL) {
- goto bail;
- }
- Py_DECREF(nonnset);
- Py_DECREF(otherpset);
- return result;
-bail:
- Py_XDECREF(nonnset);
- Py_XDECREF(otherpset);
- Py_XDECREF(result);
- return NULL;
-}
-
-/*
* Efficiently pack a dirstate object into its on-disk format.
*/
static PyObject *pack_dirstate(PyObject *self, PyObject *args)
@@ -1226,9 +1142,6 @@
static PyMethodDef methods[] = {
{"pack_dirstate", pack_dirstate, METH_VARARGS, "pack a dirstate\n"},
- {"nonnormalotherparententries", nonnormalotherparententries, METH_VARARGS,
- "create a set containing non-normal and other parent entries of given "
- "dirstate\n"},
{"parse_dirstate", parse_dirstate, METH_VARARGS, "parse a dirstate\n"},
{"parse_index2", (PyCFunction)parse_index2, METH_VARARGS | METH_KEYWORDS,
"parse a revlog index\n"},
diff --git a/contrib/dirstatenonnormalcheck.py b/contrib/dirstatenonnormalcheck.py
deleted file mode 100644
--- a/contrib/dirstatenonnormalcheck.py
+++ /dev/null
@@ -1,81 +0,0 @@
-# dirstatenonnormalcheck.py - extension to check the consistency of the
-# dirstate's non-normal map
-#
-# For most operations on dirstate, this extensions checks that the nonnormalset
-# contains the right entries.
-# It compares the nonnormal file to a nonnormalset built from the map of all
-# the files in the dirstate to check that they contain the same files.
-
-from __future__ import absolute_import
-
-from mercurial import (
- dirstate,
- extensions,
- pycompat,
-)
-
-
-def nonnormalentries(dmap):
- """Compute nonnormal entries from dirstate's dmap"""
- res = set()
- for f, e in dmap.iteritems():
- if e.state != b'n' or e.mtime == -1:
- res.add(f)
- return res
-
-
-INCONSISTENCY_MESSAGE = b"""%s call to %s
- inconsistency in nonnormalset
- result from dirstatemap: %s
- expected nonnormalset: %s
-"""
-
-
-def checkconsistency(ui, orig, dmap, _nonnormalset, label):
- """Compute nonnormalset from dmap, check that it matches _nonnormalset"""
- nonnormalcomputedmap = nonnormalentries(dmap)
- if _nonnormalset != nonnormalcomputedmap:
- b_orig = pycompat.sysbytes(repr(orig))
- b_nonnormal = pycompat.sysbytes(repr(_nonnormalset))
- b_nonnormalcomputed = pycompat.sysbytes(repr(nonnormalcomputedmap))
- msg = INCONSISTENCY_MESSAGE % (
- label,
- b_orig,
- b_nonnormal,
- b_nonnormalcomputed,
- )
- ui.develwarn(msg, config=b'dirstate')
-
-
-def _checkdirstate(orig, self, *args, **kwargs):
- """Check nonnormal set consistency before and after the call to orig"""
- checkconsistency(
- self._ui, orig, self._map, self._map.nonnormalset, b"before"
- )
- r = orig(self, *args, **kwargs)
- checkconsistency(
- self._ui, orig, self._map, self._map.nonnormalset, b"after"
- )
- return r
-
-
-def extsetup(ui):
- """Wrap functions modifying dirstate to check nonnormalset consistency"""
- dirstatecl = dirstate.dirstate
- devel = ui.configbool(b'devel', b'all-warnings')
- paranoid = ui.configbool(b'experimental', b'nonnormalparanoidcheck')
- if devel:
- extensions.wrapfunction(dirstatecl, '_writedirstate', _checkdirstate)
- if paranoid:
- # We don't do all these checks when paranoid is disable as it would
- # make the extension run very slowly on large repos
- extensions.wrapfunction(dirstatecl, 'write', _checkdirstate)
- extensions.wrapfunction(dirstatecl, 'set_tracked', _checkdirstate)
- extensions.wrapfunction(dirstatecl, 'set_untracked', _checkdirstate)
- extensions.wrapfunction(
- dirstatecl, 'set_possibly_dirty', _checkdirstate
- )
- extensions.wrapfunction(
- dirstatecl, 'update_file_p1', _checkdirstate
- )
- extensions.wrapfunction(dirstatecl, 'update_file', _checkdirstate)
To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mercurial-scm.org/pipermail/mercurial-patches/attachments/20210928/6fa800b3/attachment-0001.html>
More information about the Mercurial-patches
mailing list