D11494: dirstate: Pass the final DirstateItem to _rustmap.addfile()
SimonSapin
phabricator at mercurial-scm.org
Thu Sep 23 17:43:55 UTC 2021
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
More information about the Mercurial-devel
mailing list