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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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
783

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
783

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
783

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 32892

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…
)?;
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

.or_else(|_| { .or_else(|_| {
Err(PyErr::new::<exc::OSError, _>( Err(PyErr::new::<exc::OSError, _>(
py, py,
"Dirstate error".to_string(), "Dirstate error".to_string(),
)) ))
})?.to_py_object(py)) })?.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
);
self.inner(py).borrow_mut()
.set_clean(
HgPath::new(f.extract::<PyBytes>(py)?.data(py)),
mode,
size,
timestamp,
)
.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)>)>,