SimonSapin created this revision. Herald added a reviewer: hg-reviewers. Herald added a subscriber: mercurial-patches.
REVISION SUMMARY Now that the Python DirstateItem class wraps a Rust DirstateEntry value, use that value directly instead of converting through v1 data + 5 booleans. REPOSITORY rHG Mercurial BRANCH default REVISION DETAIL https://phab.mercurial-scm.org/D11494 AFFECTED FILES mercurial/dirstatemap.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/dirstate_map.rs rust/hg-cpython/src/dirstate/item.rs CHANGE DETAILS 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 @@ -146,6 +146,36 @@ DirstateItem::create_instance(py, Cell::new(entry)) } + @classmethod + def new_added(_cls) -> PyResult<Self> { + let entry = DirstateEntry::new_added(); + DirstateItem::create_instance(py, Cell::new(entry)) + } + + @classmethod + def new_merged(_cls) -> PyResult<Self> { + let entry = DirstateEntry::new_merged(); + DirstateItem::create_instance(py, Cell::new(entry)) + } + + @classmethod + def new_from_p2(_cls) -> PyResult<Self> { + let entry = DirstateEntry::new_from_p2(); + DirstateItem::create_instance(py, Cell::new(entry)) + } + + @classmethod + def new_possibly_dirty(_cls) -> PyResult<Self> { + let entry = DirstateEntry::new_possibly_dirty(); + DirstateItem::create_instance(py, Cell::new(entry)) + } + + @classmethod + def new_normal(_cls, mode: i32, size: i32, mtime: i32) -> PyResult<Self> { + let entry = DirstateEntry::new_normal(mode, size, mtime); + DirstateItem::create_instance(py, Cell::new(entry)) + } + def set_clean( &self, mode: i32, 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 @@ -27,8 +27,6 @@ }; use hg::{ dirstate::parsers::Timestamp, - dirstate::MTIME_UNSET, - dirstate::SIZE_NON_NORMAL, dirstate_tree::dirstate_map::DirstateMap as TreeDirstateMap, dirstate_tree::dispatch::DirstateMapMethods, dirstate_tree::on_disk::DirstateV2ParseError, @@ -145,50 +143,16 @@ def addfile( &self, - f: PyObject, - mode: PyObject, - size: PyObject, - mtime: PyObject, - added: PyObject, - merged: PyObject, - from_p2: PyObject, - possibly_dirty: PyObject, - ) -> PyResult<PyObject> { - let f = f.extract::<PyBytes>(py)?; + f: PyBytes, + item: DirstateItem, + ) -> PyResult<PyNone> { let filename = HgPath::new(f.data(py)); - let mode = if mode.is_none(py) { - // fallback default value - 0 - } else { - mode.extract(py)? - }; - let size = if size.is_none(py) { - // fallback default value - SIZE_NON_NORMAL - } else { - size.extract(py)? - }; - let mtime = if mtime.is_none(py) { - // fallback default value - MTIME_UNSET - } else { - mtime.extract(py)? - }; - let entry = DirstateEntry::new_for_add_file(mode, size, mtime); - let added = added.extract::<PyBool>(py)?.is_true(); - let merged = merged.extract::<PyBool>(py)?.is_true(); - let from_p2 = from_p2.extract::<PyBool>(py)?.is_true(); - let possibly_dirty = possibly_dirty.extract::<PyBool>(py)?.is_true(); - self.inner(py).borrow_mut().add_file( - filename, - entry, - added, - merged, - from_p2, - possibly_dirty - ).and(Ok(py.None())).or_else(|e: DirstateError| { - Err(PyErr::new::<exc::ValueError, _>(py, e.to_string())) - }) + let entry = item.get_entry(py); + self.inner(py) + .borrow_mut() + .add_file(filename, entry) + .map_err(|e |dirstate_error(py, e))?; + Ok(PyNone) } def removefile( 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 @@ -32,19 +32,8 @@ &mut self, filename: &HgPath, entry: DirstateEntry, - added: bool, - merged: bool, - from_p2: bool, - possibly_dirty: bool, ) -> Result<(), DirstateError> { - self.get_mut().add_file( - filename, - entry, - added, - merged, - from_p2, - possibly_dirty, - ) + self.get_mut().add_file(filename, entry) } fn remove_file( 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 @@ -50,10 +50,6 @@ &mut self, filename: &HgPath, entry: DirstateEntry, - added: bool, - merged: bool, - from_p2: bool, - possibly_dirty: bool, ) -> Result<(), DirstateError>; /// Mark a file as "removed" (as in `hg rm`). @@ -326,12 +322,8 @@ &mut self, filename: &HgPath, entry: DirstateEntry, - added: bool, - merged: bool, - from_p2: bool, - possibly_dirty: bool, ) -> Result<(), DirstateError> { - self.add_file(filename, entry, added, merged, from_p2, possibly_dirty) + self.add_file(filename, entry) } fn remove_file( 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 @@ -11,10 +11,8 @@ use crate::dirstate::parsers::packed_entry_size; use crate::dirstate::parsers::parse_dirstate_entries; use crate::dirstate::parsers::Timestamp; -use crate::dirstate::MTIME_UNSET; use crate::dirstate::SIZE_FROM_OTHER_PARENT; use crate::dirstate::SIZE_NON_NORMAL; -use crate::dirstate::V1_RANGEMASK; use crate::matchers::Matcher; use crate::utils::hg_path::{HgPath, HgPathBuf}; use crate::CopyMapIter; @@ -771,45 +769,8 @@ &mut self, filename: &HgPath, entry: DirstateEntry, - added: bool, - merged: bool, - from_p2: bool, - possibly_dirty: bool, ) -> Result<(), DirstateError> { - let state; - let size; - let mtime; - if added { - assert!(!possibly_dirty); - assert!(!from_p2); - state = EntryState::Added; - size = SIZE_NON_NORMAL; - mtime = MTIME_UNSET; - } else if merged { - assert!(!possibly_dirty); - assert!(!from_p2); - state = EntryState::Merged; - size = SIZE_FROM_OTHER_PARENT; - mtime = MTIME_UNSET; - } else if from_p2 { - assert!(!possibly_dirty); - state = EntryState::Normal; - size = SIZE_FROM_OTHER_PARENT; - mtime = MTIME_UNSET; - } else if possibly_dirty { - state = EntryState::Normal; - size = SIZE_NON_NORMAL; - mtime = MTIME_UNSET; - } else { - state = EntryState::Normal; - size = entry.size() & V1_RANGEMASK; - mtime = entry.mtime() & V1_RANGEMASK; - } - let mode = entry.mode(); - let entry = DirstateEntry::from_v1_data(state, mode, size, mtime); - let old_state = self.get(filename)?.map(|e| e.state()); - Ok(self.add_or_remove_file(filename, old_state, entry)?) } 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 @@ -83,12 +83,7 @@ mtime: 0, } } else { - Self { - flags: Flags::WDIR_TRACKED | Flags::P1_TRACKED, - mode, - size, - mtime, - } + Self::new_normal(mode, size, mtime) } } EntryState::Added => Self::new_added(), @@ -111,7 +106,7 @@ } } - fn new_from_p2() -> Self { + pub fn new_from_p2() -> Self { Self { // might be missing P1_TRACKED flags: Flags::WDIR_TRACKED | Flags::P2_TRACKED | Flags::CLEAN_P2, @@ -121,7 +116,7 @@ } } - fn new_possibly_dirty() -> Self { + pub fn new_possibly_dirty() -> Self { Self { flags: Flags::WDIR_TRACKED | Flags::P1_TRACKED @@ -132,7 +127,7 @@ } } - fn new_added() -> Self { + pub fn new_added() -> Self { Self { flags: Flags::WDIR_TRACKED, mode: 0, @@ -141,7 +136,7 @@ } } - fn new_merged() -> Self { + pub fn new_merged() -> Self { Self { flags: Flags::WDIR_TRACKED | Flags::P1_TRACKED // might not be true because of rename ? @@ -153,6 +148,15 @@ } } + pub fn new_normal(mode: i32, size: i32, mtime: i32) -> Self { + Self { + flags: Flags::WDIR_TRACKED | Flags::P1_TRACKED, + mode, + size, + mtime, + } + } + /// Creates a new entry in "removed" state. /// /// `size` is expected to be zero, `SIZE_NON_NORMAL`, or @@ -161,14 +165,6 @@ Self::from_v1_data(EntryState::Removed, 0, size, 0) } - /// TODO: refactor `DirstateMap::add_file` to not take a `DirstateEntry` - /// parameter and remove this constructor - pub fn new_for_add_file(mode: i32, size: i32, mtime: i32) -> Self { - // XXX Arbitrary default value since the value is determined later - let state = EntryState::Normal; - Self::from_v1_data(state, mode, size, mtime) - } - pub fn tracked(&self) -> bool { self.flags.contains(Flags::WDIR_TRACKED) } 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 @@ -8,10 +8,8 @@ use crate::dirstate::parsers::Timestamp; use crate::{ dirstate::EntryState, - dirstate::MTIME_UNSET, dirstate::SIZE_FROM_OTHER_PARENT, dirstate::SIZE_NON_NORMAL, - dirstate::V1_RANGEMASK, pack_dirstate, parse_dirstate, utils::hg_path::{HgPath, HgPathBuf}, CopyMap, DirsMultiset, DirstateEntry, DirstateError, DirstateParents, @@ -73,44 +71,7 @@ &mut self, filename: &HgPath, entry: DirstateEntry, - // XXX once the dust settle this should probably become an enum - added: bool, - merged: bool, - from_p2: bool, - possibly_dirty: bool, ) -> Result<(), DirstateError> { - let state; - let size; - let mtime; - if added { - assert!(!possibly_dirty); - assert!(!from_p2); - state = EntryState::Added; - size = SIZE_NON_NORMAL; - mtime = MTIME_UNSET; - } else if merged { - assert!(!possibly_dirty); - assert!(!from_p2); - state = EntryState::Merged; - size = SIZE_FROM_OTHER_PARENT; - mtime = MTIME_UNSET; - } else if from_p2 { - assert!(!possibly_dirty); - state = EntryState::Normal; - size = SIZE_FROM_OTHER_PARENT; - mtime = MTIME_UNSET; - } else if possibly_dirty { - state = EntryState::Normal; - size = SIZE_NON_NORMAL; - mtime = MTIME_UNSET; - } else { - state = EntryState::Normal; - size = entry.size() & V1_RANGEMASK; - mtime = entry.mtime() & V1_RANGEMASK; - } - let mode = entry.mode(); - let entry = DirstateEntry::from_v1_data(state, mode, size, mtime); - let old_state = self.get(filename).map(|e| e.state()); if old_state.is_none() || old_state == Some(EntryState::Removed) { if let Some(ref mut dirs) = self.dirs { @@ -417,10 +378,6 @@ map.add_file( HgPath::new(b"meh"), DirstateEntry::from_v1_data(EntryState::Normal, 1337, 1337, 1337), - false, - false, - false, - false, ) .unwrap(); diff --git a/mercurial/dirstatemap.py b/mercurial/dirstatemap.py --- a/mercurial/dirstatemap.py +++ b/mercurial/dirstatemap.py @@ -561,17 +561,26 @@ from_p2=False, possibly_dirty=False, ): - ret = self._rustmap.addfile( - f, - mode, - size, - mtime, - added, - merged, - from_p2, - possibly_dirty, - ) if added: + assert not possibly_dirty + assert not from_p2 + item = DirstateItem.new_added() + elif merged: + assert not possibly_dirty + assert not from_p2 + item = DirstateItem.new_merged() + elif from_p2: + assert not possibly_dirty + item = DirstateItem.new_from_p2() + elif possibly_dirty: + item = DirstateItem.new_possibly_dirty() + else: + size = size & rangemask + mtime = mtime & rangemask + item = DirstateItem.new_normal(mode, size, mtime) + ret = self._rustmap.addfile(f, item) + if added: + # FIXME: is this dead code? _rustmap.addfile always returns None self.copymap.pop(f, None) return ret To: SimonSapin, #hg-reviewers Cc: mercurial-patches, mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
