This is an archive of the discontinued Mercurial Phabricator instance.

Differential D12456

rust-dirstatemap: add `set_clean` method
ClosedPublic

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

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG0813b32a51fc: rust-dirstatemap: add `set_clean` method
rHGd4a96f211898: rust-dirstatemap: add `set_clean` method
Summary

This is the new dirstate API that has already been moved to in Python.

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:19 AM

looks overall good, but I have a small question.

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

Why can we have an empty closure here ?

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

set_clean can't be called on a fully untracked node (because of the check in the public function), but could be theoretically be called on a removed file, which would change the tracked_descendants_count variable. It just happens that this does not happen in the code, so I probably forgot to check this.

I'll update this, thanks.

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

If this cannot be called on a fully untracked node, it seems like we should not use a _or_insert method.

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

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

baymax updated this revision to Diff 33030.Apr 12 2022, 10:57 AM

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

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

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

Revision Contents
Changeset List

Diff 33111

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

use crate::dirstate::StateMapIter; use crate::dirstate::StateMapIter;
use crate::dirstate::TruncatedTimestamp; use crate::dirstate::TruncatedTimestamp;
use crate::dirstate::SIZE_FROM_OTHER_PARENT; use crate::dirstate::SIZE_FROM_OTHER_PARENT;
use crate::dirstate::SIZE_NON_NORMAL; use crate::dirstate::SIZE_NON_NORMAL;
use crate::matchers::Matcher; use crate::matchers::Matcher;
use crate::utils::hg_path::{HgPath, HgPathBuf}; use crate::utils::hg_path::{HgPath, HgPathBuf};
use crate::DirstateEntry; use crate::DirstateEntry;
use crate::DirstateError; use crate::DirstateError;
use crate::DirstateMapError;
use crate::DirstateParents; use crate::DirstateParents;
use crate::DirstateStatus; use crate::DirstateStatus;
use crate::EntryState; use crate::EntryState;
use crate::FastHashbrownMap as FastHashMap; use crate::FastHashbrownMap as FastHashMap;
use crate::PatternFileWarning; use crate::PatternFileWarning;
use crate::StatusError; use crate::StatusError;
use crate::StatusOptions; use crate::StatusOptions;
)?; )?;
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(())
} }
fn set_clean(
&mut self,
filename: &HgPath,
old_entry: DirstateEntry,
mode: u32,
size: u32,
mtime: TruncatedTimestamp,
) -> Result<(), DirstateError> {
let node = Self::get_or_insert_node(
self.on_disk,
&mut self.unreachable_bytes,
&mut self.root,
filename,
WithBasename::to_cow_owned,
|ancestor| {
marmouteUnsubmitted
Not Done

Why can we have an empty closure here ?

marmoute: Why can we have an empty closure here ?
AlphareAuthorUnsubmitted
Done

set_clean can't be called on a fully untracked node (because of the check in the public function), but could be theoretically be called on a removed file, which would change the tracked_descendants_count variable. It just happens that this does not happen in the code, so I probably forgot to check this.

I'll update this, thanks.

Alphare: `set_clean` can't be called on a fully untracked node (because of the check in the public…
marmouteUnsubmitted
Not Done

If this cannot be called on a fully untracked node, it seems like we should not use a _or_insert method.

marmoute: If this cannot be called on a fully untracked node, it seems like we should not use a…
if !old_entry.tracked() {
ancestor.tracked_descendants_count += 1;
}
},
)?;
let mut new_entry = old_entry.clone();
new_entry.set_clean(mode, size, mtime);
node.data = NodeData::Entry(new_entry);
Ok(())
}
fn iter_nodes<'tree>( fn iter_nodes<'tree>(
&'tree self, &'tree self,
) -> impl Iterator< ) -> impl Iterator<
Item = Result<NodeRef<'tree, 'on_disk>, DirstateV2ParseError>, Item = Result<NodeRef<'tree, 'on_disk>, DirstateV2ParseError>,
> + 'tree { > + 'tree {
// Depth first tree traversal. // Depth first tree traversal.
// //
// If we could afford internal iteration and recursion, // If we could afford internal iteration and recursion,
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_clean(
&mut self,
filename: &HgPath,
mode: u32,
size: u32,
mtime: TruncatedTimestamp,
) -> Result<(), DirstateError> {
let old_entry = match self.get(filename)? {
None => {
return Err(
DirstateMapError::PathNotFound(filename.into()).into()
)
}
Some(e) => e,
};
self.copy_map_remove(filename)?;
self.with_dmap_mut(|map| {
map.set_clean(filename, old_entry, mode, size, mtime)
})
}
pub fn reset_state( pub fn reset_state(
&mut self, &mut self,
filename: &HgPath, filename: &HgPath,
wc_tracked: bool, wc_tracked: bool,
p1_tracked: bool, p1_tracked: bool,
p2_info: bool, p2_info: bool,
has_meaningful_mtime: bool, has_meaningful_mtime: bool,
parent_file_data_opt: Option<ParentFileData>, parent_file_data_opt: Option<ParentFileData>,

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_clean(
&self,
f: PyObject,
mode: u32,
size: u32,
mtime: (i64, u32, bool)
) -> PyResult<PyNone> {
let (mtime_s, mtime_ns, second_ambiguous) = mtime;
let timestamp = TruncatedTimestamp::new_truncate(
mtime_s, mtime_ns, second_ambiguous
);
let bytes = f.extract::<PyBytes>(py)?;
let path = HgPath::new(bytes.data(py));
let res = self.inner(py).borrow_mut().set_clean(
path, mode, size, timestamp,
);
res.or_else(|_| {
Err(PyErr::new::<exc::OSError, _>(py, "Dirstate error".to_string()))
})?;
Ok(PyNone)
}
def reset_state( def reset_state(
&self, &self,
f: PyObject, f: PyObject,
wc_tracked: bool, wc_tracked: bool,
p1_tracked: bool, p1_tracked: bool,
p2_info: bool, p2_info: bool,
has_meaningful_mtime: bool, has_meaningful_mtime: bool,
parentfiledata: Option<(u32, u32, Option<(i64, u32, bool)>)>, parentfiledata: Option<(u32, u32, Option<(i64, u32, bool)>)>,