This is an archive of the discontinued Mercurial Phabricator instance.

Differential D12460

rust-dirstatemap: add `set_untracked` method
ClosedPublic

Authored by Alphare on Apr 6 2022, 10:11 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG95e304addfc1: rust-dirstatemap: add `set_untracked` method
rHG0e20d8410c2d: rust-dirstatemap: add `set_untracked` method
Summary

This is the new API that Python has already migrated to

Diff Detail

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

Event Timeline

Alphare created this revision.Apr 6 2022, 10:11 AM
marmoute added a subscriber: marmoute.Apr 8 2022, 7:28 AM
marmoute added inline comments.
rust/hg-core/src/dirstate_tree/dirstate_map.rs
782

If we know that an entry exist. why do we need a get_or_insert here?

792–794

Code lower in this patch seems to drop the node in the "added" case, how does this deal with the fact we keep touching it here ?

Alphare added inline comments.Apr 8 2022, 9:08 AM
rust/hg-core/src/dirstate_tree/dirstate_map.rs
782

See D12458. We've agreed to fix this in additional patches to reduce churn.

792–794

This is not called at all in the "added" case.

Alphare added a commit: rHG0e20d8410c2d: rust-dirstatemap: add `set_untracked` method.Apr 8 2022, 11:56 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG0e20d8410c2d: rust-dirstatemap: add `set_untracked` method (authored by Alphare). · Explain Why
This revision was automatically updated to reflect the committed changes.
baymax updated this revision to Diff 32932.Apr 8 2022, 12:29 PM

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

baymax updated this revision to Diff 33034.Apr 12 2022, 10:58 AM

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

baymax updated this revision to Diff 33116.Apr 12 2022, 12:06 PM

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

Revision Contents
Changeset List

Diff 33116

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

)?; )?;
if !had_entry { if !had_entry {
self.nodes_with_entry_count += 1 self.nodes_with_entry_count += 1
} }
node.data = NodeData::Entry(new_entry); node.data = NodeData::Entry(new_entry);
Ok(()) Ok(())
} }
/// It is the responsibility of the caller to know that there was an entry
/// there before. Does not handle the removal of copy source
fn set_untracked(
&mut self,
filename: &HgPath,
old_entry: DirstateEntry,
) -> Result<(), DirstateV2ParseError> {
let node = Self::get_or_insert_node(
marmouteUnsubmitted
Not Done

If we know that an entry exist. why do we need a get_or_insert here?

marmoute: If we know that an entry exist. why do we need a get**_or_insert** here?
AlphareAuthorUnsubmitted
Done

See D12458. We've agreed to fix this in additional patches to reduce churn.

Alphare: See D12458. We've agreed to fix this in additional patches to reduce churn.
self.on_disk,
&mut self.unreachable_bytes,
&mut self.root,
filename,
WithBasename::to_cow_owned,
|ancestor| {
ancestor.tracked_descendants_count = ancestor
.tracked_descendants_count
.checked_sub(1)
.expect("tracked_descendants_count should be >= 0");
},
)?;
marmouteUnsubmitted
Not Done

Code lower in this patch seems to drop the node in the "added" case, how does this deal with the fact we keep touching it here ?

marmoute: Code lower in this patch seems to drop the node in the "added" case, how does this deal with…
AlphareAuthorUnsubmitted
Done

This is not called at all in the "added" case.

Alphare: This is not called at all in the "added" case.
let mut new_entry = old_entry.clone();
new_entry.set_untracked();
node.data = NodeData::Entry(new_entry);
Ok(())
}
fn set_clean( fn set_clean(
&mut self, &mut self,
filename: &HgPath, filename: &HgPath,
old_entry: DirstateEntry, old_entry: DirstateEntry,
mode: u32, mode: u32,
size: u32, size: u32,
mtime: TruncatedTimestamp, mtime: TruncatedTimestamp,
) -> Result<(), DirstateError> { ) -> Result<(), DirstateError> {
pub fn set_tracked( pub fn set_tracked(
&mut self, &mut self,
filename: &HgPath, filename: &HgPath,
) -> Result<bool, DirstateV2ParseError> { ) -> Result<bool, DirstateV2ParseError> {
let old_entry_opt = self.get(filename)?; let old_entry_opt = self.get(filename)?;
self.with_dmap_mut(|map| map.set_tracked(filename, old_entry_opt)) self.with_dmap_mut(|map| map.set_tracked(filename, old_entry_opt))
} }
pub fn set_untracked(
&mut self,
filename: &HgPath,
) -> Result<bool, DirstateError> {
let old_entry_opt = self.get(filename)?;
match old_entry_opt {
None => Ok(false),
Some(old_entry) => {
if !old_entry.tracked() {
// `DirstateMap::set_untracked` is not a noop if
// already not tracked as it will decrement the
// tracked counters while going down.
return Ok(true);
}
if old_entry.added() {
// Untracking an "added" entry will just result in a
// worthless entry (and other parts of the code will
// complain about it), just drop it entirely.
self.drop_entry_and_copy_source(filename)?;
return Ok(true);
}
if !old_entry.p2_info() {
self.copy_map_remove(filename)?;
}
self.with_dmap_mut(|map| {
map.set_untracked(filename, old_entry)?;
Ok(true)
})
}
}
}
pub fn set_clean( pub fn set_clean(
&mut self, &mut self,
filename: &HgPath, filename: &HgPath,
mode: u32, mode: u32,
size: u32, size: u32,
mtime: TruncatedTimestamp, mtime: TruncatedTimestamp,
) -> Result<(), DirstateError> { ) -> Result<(), DirstateError> {
let old_entry = match self.get(filename)? { let old_entry = match self.get(filename)? {

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

let path = HgPath::new(bytes.data(py)); let path = HgPath::new(bytes.data(py));
let res = self.inner(py).borrow_mut().set_tracked(path); let res = self.inner(py).borrow_mut().set_tracked(path);
let was_tracked = res.or_else(|_| { let was_tracked = res.or_else(|_| {
Err(PyErr::new::<exc::OSError, _>(py, "Dirstate error".to_string())) Err(PyErr::new::<exc::OSError, _>(py, "Dirstate error".to_string()))
})?; })?;
Ok(was_tracked.to_py_object(py)) Ok(was_tracked.to_py_object(py))
} }
def set_untracked(&self, f: PyObject) -> PyResult<PyBool> {
let bytes = f.extract::<PyBytes>(py)?;
let path = HgPath::new(bytes.data(py));
let res = self.inner(py).borrow_mut().set_untracked(path);
let was_tracked = res.or_else(|_| {
Err(PyErr::new::<exc::OSError, _>(py, "Dirstate error".to_string()))
})?;
Ok(was_tracked.to_py_object(py))
}
def set_clean( def set_clean(
&self, &self,
f: PyObject, f: PyObject,
mode: u32, mode: u32,
size: u32, size: u32,
mtime: (i64, u32, bool) mtime: (i64, u32, bool)
) -> PyResult<PyNone> { ) -> PyResult<PyNone> {
let (mtime_s, mtime_ns, second_ambiguous) = mtime; let (mtime_s, mtime_ns, second_ambiguous) = mtime;