( )⚙ D10967 dirstate: use a `added` parameter to _addpath

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10967

dirstate: use a `added` parameter to _addpath
ClosedPublic

Authored by marmoute on Jul 4 2021, 5:51 PM.

Details

Reviewers
SimonSapin
pulkit
Group Reviewers
hg-reviewers
Commits
rHGfe4641cf9b72: dirstate: use a `added` parameter to _addpath
Summary

My next target are the explicit state byte, so let start with abstracting
their initial storage.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

marmoute created this revision.Jul 4 2021, 5:51 PM
SimonSapin accepted this revision.Jul 5 2021, 12:52 AM
SimonSapin added a subscriber: SimonSapin.
SimonSapin added inline comments.
rust/hg-cpython/src/dirstate/dirstate_map.rs
122–123

I’m not a fan of this, but I see that it changes soon anyway in other patches

pulkit accepted this revision.Jul 6 2021, 12:43 PM
This revision is now accepted and ready to land.Jul 6 2021, 12:43 PM
baymax updated this revision to Diff 28869.Jul 6 2021, 2:09 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

marmoute added a commit: rHGfe4641cf9b72: dirstate: use a `added` parameter to _addpath.Jul 6 2021, 3:10 PM
Closed by commit rHGfe4641cf9b72: dirstate: use a `added` parameter to _addpath (authored by marmoute). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 28788

mercurial/dirstate.py

return self._map.copymap.get(file, None) return self._map.copymap.get(file, None)
def copies(self): def copies(self):
return self._map.copymap return self._map.copymap
def _addpath( def _addpath(
self, self,
f, f,
state, state=None,
mode, mode=0,
size=None, size=None,
mtime=None, mtime=None,
added=False,
from_p2=False, from_p2=False,
possibly_dirty=False, possibly_dirty=False,
): ):
entry = self._map.get(f) entry = self._map.get(f)
if state == b'a' or entry is not None and entry.removed: if added or entry is not None and entry.removed:
scmutil.checkfilename(f) scmutil.checkfilename(f)
if self._map.hastrackeddir(f): if self._map.hastrackeddir(f):
msg = _(b'directory %r already in dirstate') msg = _(b'directory %r already in dirstate')
msg %= pycompat.bytestr(f) msg %= pycompat.bytestr(f)
raise error.Abort(msg) raise error.Abort(msg)
# shadows # shadows
for d in pathutil.finddirs(f): for d in pathutil.finddirs(f):
if self._map.hastrackeddir(d): if self._map.hastrackeddir(d):
break break
entry = self._map.get(d) entry = self._map.get(d)
if entry is not None and not entry.removed: if entry is not None and not entry.removed:
msg = _(b'file %r in dirstate clashes with %r') msg = _(b'file %r in dirstate clashes with %r')
msg %= (pycompat.bytestr(d), pycompat.bytestr(f)) msg %= (pycompat.bytestr(d), pycompat.bytestr(f))
raise error.Abort(msg) raise error.Abort(msg)
self._dirty = True self._dirty = True
self._updatedfiles.add(f) self._updatedfiles.add(f)
self._map.addfile( self._map.addfile(
f, f,
state=state, state=state,
mode=mode, mode=mode,
size=size, size=size,
mtime=mtime, mtime=mtime,
added=added,
from_p2=from_p2, from_p2=from_p2,
possibly_dirty=possibly_dirty, possibly_dirty=possibly_dirty,
) )
def normal(self, f, parentfiledata=None): def normal(self, f, parentfiledata=None):
"""Mark a file normal and clean. """Mark a file normal and clean.
parentfiledata: (mode, size, mtime) of the clean file parentfiledata: (mode, size, mtime) of the clean file
self._addpath(f, b'm', 0, from_p2=True) self._addpath(f, b'm', 0, from_p2=True)
else: else:
# add-like # add-like
self._addpath(f, b'n', 0, from_p2=True) self._addpath(f, b'n', 0, from_p2=True)
self._map.copymap.pop(f, None) self._map.copymap.pop(f, None)
def add(self, f): def add(self, f):
'''Mark a file added.''' '''Mark a file added.'''
self._addpath(f, b'a', 0) self._addpath(f, added=True)
self._map.copymap.pop(f, None) self._map.copymap.pop(f, None)
def remove(self, f): def remove(self, f):
'''Mark a file removed.''' '''Mark a file removed.'''
self._dirty = True self._dirty = True
self._updatedfiles.add(f) self._updatedfiles.add(f)
self._map.removefile(f, in_merge=self.in_merge) self._map.removefile(f, in_merge=self.in_merge)

mercurial/dirstatemap.py

def preload(self): def preload(self):
"""Loads the underlying data, if it's not already loaded""" """Loads the underlying data, if it's not already loaded"""
self._map self._map
def addfile( def addfile(
self, self,
f, f,
state, state=None,
mode, mode=0,
size=None, size=None,
mtime=None, mtime=None,
added=False,
from_p2=False, from_p2=False,
possibly_dirty=False, possibly_dirty=False,
): ):
"""Add a tracked file to the dirstate.""" """Add a tracked file to the dirstate."""
if state == b'a': if added:
assert not possibly_dirty assert not possibly_dirty
assert not from_p2 assert not from_p2
state = b'a'
size = NONNORMAL size = NONNORMAL
mtime = AMBIGUOUS_TIME mtime = AMBIGUOUS_TIME
elif from_p2: elif from_p2:
assert not possibly_dirty assert not possibly_dirty
size = FROM_P2 size = FROM_P2
mtime = AMBIGUOUS_TIME mtime = AMBIGUOUS_TIME
elif possibly_dirty: elif possibly_dirty:
size = NONNORMAL size = NONNORMAL
mtime = AMBIGUOUS_TIME mtime = AMBIGUOUS_TIME
else: else:
assert state != b'a'
assert size != FROM_P2 assert size != FROM_P2
assert size != NONNORMAL assert size != NONNORMAL
size = size & rangemask size = size & rangemask
mtime = mtime & rangemask mtime = mtime & rangemask
assert state is not None
assert size is not None assert size is not None
assert mtime is not None assert mtime is not None
old_entry = self.get(f) old_entry = self.get(f)
if ( if (
old_entry is None or old_entry.removed old_entry is None or old_entry.removed
) and "_dirs" in self.__dict__: ) and "_dirs" in self.__dict__:
self._dirs.addpath(f) self._dirs.addpath(f)
if old_entry is None and "_alldirs" in self.__dict__: if old_entry is None and "_alldirs" in self.__dict__:
b"experimental", b"experimental",
b"dirstate-tree.in-memory", b"dirstate-tree.in-memory",
False, False,
) )
def addfile( def addfile(
self, self,
f, f,
state, state=None,
mode, mode=0,
size=None, size=None,
mtime=None, mtime=None,
added=False,
from_p2=False, from_p2=False,
possibly_dirty=False, possibly_dirty=False,
): ):
return self._rustmap.addfile( return self._rustmap.addfile(
f, f,
state, state,
mode, mode,
size, size,
mtime, mtime,
added,
from_p2, from_p2,
possibly_dirty, possibly_dirty,
) )
def removefile(self, *args, **kwargs): def removefile(self, *args, **kwargs):
return self._rustmap.removefile(*args, **kwargs) return self._rustmap.removefile(*args, **kwargs)
def dropfile(self, *args, **kwargs): def dropfile(self, *args, **kwargs):

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

} }
/// Add a tracked file to the dirstate /// Add a tracked file to the dirstate
pub fn add_file( pub fn add_file(
&mut self, &mut self,
filename: &HgPath, filename: &HgPath,
entry: DirstateEntry, entry: DirstateEntry,
// XXX once the dust settle this should probably become an enum // XXX once the dust settle this should probably become an enum
added: bool,
from_p2: bool, from_p2: bool,
possibly_dirty: bool, possibly_dirty: bool,
) -> Result<(), DirstateError> { ) -> Result<(), DirstateError> {
let mut entry = entry; let mut entry = entry;
if entry.state == EntryState::Added { if added {
assert!(!possibly_dirty); assert!(!possibly_dirty);
assert!(!from_p2); assert!(!from_p2);
entry.state = EntryState::Added;
entry.size = SIZE_NON_NORMAL; entry.size = SIZE_NON_NORMAL;
entry.mtime = MTIME_UNSET; entry.mtime = MTIME_UNSET;
} else if from_p2 { } else if from_p2 {
assert!(!possibly_dirty); assert!(!possibly_dirty);
entry.size = SIZE_FROM_OTHER_PARENT; entry.size = SIZE_FROM_OTHER_PARENT;
entry.mtime = MTIME_UNSET; entry.mtime = MTIME_UNSET;
} else if possibly_dirty { } else if possibly_dirty {
entry.size = SIZE_NON_NORMAL; entry.size = SIZE_NON_NORMAL;
DirstateEntry { DirstateEntry {
state: EntryState::Normal, state: EntryState::Normal,
mode: 1337, mode: 1337,
mtime: 1337, mtime: 1337,
size: 1337, size: 1337,
}, },
false, false,
false, false,
false,
) )
.unwrap(); .unwrap();
assert_eq!(1, map.len()); 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().0.len());
assert_eq!(0, map.get_non_normal_other_parent_entries().1.len()); assert_eq!(0, map.get_non_normal_other_parent_entries().1.len());
} }

rust/hg-core/src/dirstate_tree/dirstate_map.rs

self.nodes_with_entry_count = 0; self.nodes_with_entry_count = 0;
self.nodes_with_copy_source_count = 0; self.nodes_with_copy_source_count = 0;
} }
fn add_file( fn add_file(
&mut self, &mut self,
filename: &HgPath, filename: &HgPath,
entry: DirstateEntry, entry: DirstateEntry,
added: bool,
from_p2: bool, from_p2: bool,
possibly_dirty: bool, possibly_dirty: bool,
) -> Result<(), DirstateError> { ) -> Result<(), DirstateError> {
let mut entry = entry; let mut entry = entry;
if entry.state == EntryState::Added { if added {
assert!(!possibly_dirty); assert!(!possibly_dirty);
assert!(!from_p2); assert!(!from_p2);
entry.state = EntryState::Added;
entry.size = SIZE_NON_NORMAL; entry.size = SIZE_NON_NORMAL;
entry.mtime = MTIME_UNSET; entry.mtime = MTIME_UNSET;
} else if from_p2 { } else if from_p2 {
assert!(!possibly_dirty); assert!(!possibly_dirty);
entry.size = SIZE_FROM_OTHER_PARENT; entry.size = SIZE_FROM_OTHER_PARENT;
entry.mtime = MTIME_UNSET; entry.mtime = MTIME_UNSET;
} else if possibly_dirty { } else if possibly_dirty {
entry.size = SIZE_NON_NORMAL; entry.size = SIZE_NON_NORMAL;

rust/hg-core/src/dirstate_tree/dispatch.rs

/// `old_state` is the state in the entry that `get` would have returned /// `old_state` is the state in the entry that `get` would have returned
/// before this call, or `EntryState::Unknown` if there was no such entry. /// before this call, or `EntryState::Unknown` if there was no such entry.
/// ///
/// `entry.state` should never be `EntryState::Unknown`. /// `entry.state` should never be `EntryState::Unknown`.
fn add_file( fn add_file(
&mut self, &mut self,
filename: &HgPath, filename: &HgPath,
entry: DirstateEntry, entry: DirstateEntry,
added: bool,
from_p2: bool, from_p2: bool,
possibly_dirty: bool, possibly_dirty: bool,
) -> Result<(), DirstateError>; ) -> Result<(), DirstateError>;
/// Mark a file as "removed" (as in `hg rm`). /// Mark a file as "removed" (as in `hg rm`).
/// ///
/// `old_state` is the state in the entry that `get` would have returned /// `old_state` is the state in the entry that `get` would have returned
/// before this call, or `EntryState::Unknown` if there was no such entry. /// before this call, or `EntryState::Unknown` if there was no such entry.
fn clear(&mut self) { fn clear(&mut self) {
self.clear() self.clear()
} }
fn add_file( fn add_file(
&mut self, &mut self,
filename: &HgPath, filename: &HgPath,
entry: DirstateEntry, entry: DirstateEntry,
added: bool,
from_p2: bool, from_p2: bool,
possibly_dirty: bool, possibly_dirty: bool,
) -> Result<(), DirstateError> { ) -> Result<(), DirstateError> {
self.add_file(filename, entry, from_p2, possibly_dirty) self.add_file(filename, entry, added, from_p2, possibly_dirty)
} }
fn remove_file( fn remove_file(
&mut self, &mut self,
filename: &HgPath, filename: &HgPath,
in_merge: bool, in_merge: bool,
) -> Result<(), DirstateError> { ) -> Result<(), DirstateError> {
self.remove_file(filename, in_merge) self.remove_file(filename, in_merge)

rust/hg-cpython/src/dirstate/dirstate_map.rs

def addfile( def addfile(
&self, &self,
f: PyObject, f: PyObject,
state: PyObject, state: PyObject,
mode: PyObject, mode: PyObject,
size: PyObject, size: PyObject,
mtime: PyObject, mtime: PyObject,
added: PyObject,
from_p2: PyObject, from_p2: PyObject,
possibly_dirty: PyObject, possibly_dirty: PyObject,
) -> PyResult<PyObject> { ) -> PyResult<PyObject> {
let f = f.extract::<PyBytes>(py)?; let f = f.extract::<PyBytes>(py)?;
let filename = HgPath::new(f.data(py)); let filename = HgPath::new(f.data(py));
let state = state.extract::<PyBytes>(py)?.data(py)[0] let state = if state.is_none(py) {
// Arbitrary default value
EntryState::Normal
SimonSapinUnsubmitted
Not Done

I’m not a fan of this, but I see that it changes soon anyway in other patches

SimonSapin: I’m not a fan of this, but I see that it changes soon anyway in other patches
} else {
state.extract::<PyBytes>(py)?.data(py)[0]
.try_into() .try_into()
.map_err(|e: HgError| { .map_err(|e: HgError| {
PyErr::new::<exc::ValueError, _>(py, e.to_string()) PyErr::new::<exc::ValueError, _>(py, e.to_string())
})?; })?
let mode = mode.extract(py)?; };
let mode = if mode.is_none(py) {
// fallback default value
0
} else {
mode.extract(py)?
};
let size = if size.is_none(py) { let size = if size.is_none(py) {
// fallback default value // fallback default value
SIZE_NON_NORMAL SIZE_NON_NORMAL
} else { } else {
size.extract(py)? size.extract(py)?
}; };
let mtime = if mtime.is_none(py) { let mtime = if mtime.is_none(py) {
// fallback default value // fallback default value
MTIME_UNSET MTIME_UNSET
} else { } else {
mtime.extract(py)? mtime.extract(py)?
}; };
let entry = DirstateEntry { let entry = DirstateEntry {
state: state, state: state,
mode: mode, mode: mode,
size: size, size: size,
mtime: mtime, mtime: mtime,
}; };
let added = added.extract::<PyBool>(py)?.is_true();
let from_p2 = from_p2.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(); let possibly_dirty = possibly_dirty.extract::<PyBool>(py)?.is_true();
self.inner(py).borrow_mut().add_file( self.inner(py).borrow_mut().add_file(
filename, filename,
entry, entry,
added,
from_p2, from_p2,
possibly_dirty possibly_dirty
).and(Ok(py.None())).or_else(|e: DirstateError| { ).and(Ok(py.None())).or_else(|e: DirstateError| {
Err(PyErr::new::<exc::ValueError, _>(py, e.to_string())) Err(PyErr::new::<exc::ValueError, _>(py, e.to_string()))
}) })
} }
def removefile( def removefile(

rust/hg-cpython/src/dirstate/dispatch.rs

fn clear(&mut self) { fn clear(&mut self) {
self.get_mut().clear() self.get_mut().clear()
} }
fn add_file( fn add_file(
&mut self, &mut self,
filename: &HgPath, filename: &HgPath,
entry: DirstateEntry, entry: DirstateEntry,
added: bool,
from_p2: bool, from_p2: bool,
possibly_dirty: bool, possibly_dirty: bool,
) -> Result<(), DirstateError> { ) -> Result<(), DirstateError> {
self.get_mut() self.get_mut().add_file(
.add_file(filename, entry, from_p2, possibly_dirty) filename,
entry,
added,
from_p2,
possibly_dirty,
)
} }
fn remove_file( fn remove_file(
&mut self, &mut self,
filename: &HgPath, filename: &HgPath,
in_merge: bool, in_merge: bool,
) -> Result<(), DirstateError> { ) -> Result<(), DirstateError> {
self.get_mut().remove_file(filename, in_merge) self.get_mut().remove_file(filename, in_merge)