This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10827

dirstate-v2: Write .hg/dirstate back to disk on directory cache changes
ClosedPublic

Authored by SimonSapin on Jun 1 2021, 12:51 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG04d1f17f49e7: dirstate-v2: Write .hg/dirstate back to disk on directory cache changes

Diff Detail

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

Event Timeline

SimonSapin created this revision.Jun 1 2021, 12:51 PM
SimonSapin added a commit: rHG04d1f17f49e7: dirstate-v2: Write .hg/dirstate back to disk on directory cache changes.Jun 1 2021, 3:35 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG04d1f17f49e7: dirstate-v2: Write .hg/dirstate back to disk on directory cache changes (authored by SimonSapin). · Explain Why
This revision was automatically updated to reflect the committed changes.
baymax updated this revision to Diff 28385.Jun 1 2021, 5:05 PM

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

Alphare added a subscriber: Alphare.Jun 2 2021, 4:11 AM
Alphare added inline comments.
rust/hg-core/src/dirstate_tree/status.rs
83

This does not seem semantically correct. The docstring for dirty says that it's any change to the dirstate that should be written, but this only concerns the directories cache.

SimonSapin added inline comments.Jun 2 2021, 4:17 AM
rust/hg-core/src/dirstate_tree/status.rs
83

Yes, and we want new cache entries to be written to disk?

Alphare added inline comments.Jun 2 2021, 4:19 AM
rust/hg-core/src/dirstate_tree/status.rs
83

Right, but the docstring for dirty implies that it covers any part of the dirstate that needs serializing to disk, while it is only for the cache.

Alphare added inline comments.Jun 2 2021, 5:24 AM
rust/hg-core/src/dirstate_tree/status.rs
83

I was completely confused, we don't update the dirstate for anything other than caches outside of a commit. Sorry for the noise. :)

Revision Contents
Changeset List

Diff 28385

mercurial/context.py

# it's in the dirstate. # it's in the dirstate.
deleted.append(f) deleted.append(f)
return modified, deleted, fixup return modified, deleted, fixup
def _poststatusfixup(self, status, fixup): def _poststatusfixup(self, status, fixup):
"""update dirstate for files that are actually clean""" """update dirstate for files that are actually clean"""
poststatus = self._repo.postdsstatus() poststatus = self._repo.postdsstatus()
if fixup or poststatus: if fixup or poststatus or self._repo.dirstate._dirty:
try: try:
oldid = self._repo.dirstate.identity() oldid = self._repo.dirstate.identity()
# updating the dirstate is optional # updating the dirstate is optional
# so we don't wait on the lock # so we don't wait on the lock
# wlock can invalidate the dirstate, so cache normal _after_ # wlock can invalidate the dirstate, so cache normal _after_
# taking the lock # taking the lock
with self._repo.wlock(False): with self._repo.wlock(False):

mercurial/dirstate.py

removed, removed,
deleted, deleted,
clean, clean,
ignored, ignored,
unknown, unknown,
warnings, warnings,
bad, bad,
traversed, traversed,
dirty,
) = rustmod.status( ) = rustmod.status(
self._map._rustmap, self._map._rustmap,
matcher, matcher,
self._rootdir, self._rootdir,
self._ignorefiles(), self._ignorefiles(),
self._checkexec, self._checkexec,
self._lastnormaltime, self._lastnormaltime,
bool(list_clean), bool(list_clean),
bool(list_ignored), bool(list_ignored),
bool(list_unknown), bool(list_unknown),
bool(matcher.traversedir), bool(matcher.traversedir),
) )
self._dirty |= dirty
if matcher.traversedir: if matcher.traversedir:
for dir in traversed: for dir in traversed:
matcher.traversedir(dir) matcher.traversedir(dir)
if self._ui.warn: if self._ui.warn:
for item in warnings: for item in warnings:
if isinstance(item, tuple): if isinstance(item, tuple):
file_path, syntax = item file_path, syntax = item

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

/// Either clean or modified, but we can’t tell from filesystem metadata /// Either clean or modified, but we can’t tell from filesystem metadata
/// alone. The file contents need to be read and compared with that in /// alone. The file contents need to be read and compared with that in
/// the parent. /// the parent.
pub unsure: Vec<HgPathCow<'a>>, pub unsure: Vec<HgPathCow<'a>>,
/// Only filled if `collect_traversed_dirs` is `true` /// Only filled if `collect_traversed_dirs` is `true`
pub traversed: Vec<HgPathCow<'a>>, pub traversed: Vec<HgPathCow<'a>>,
/// Whether `status()` made changed to the `DirstateMap` that should be
/// written back to disk
pub dirty: bool,
} }
#[derive(Debug, derive_more::From)] #[derive(Debug, derive_more::From)]
pub enum StatusError { pub enum StatusError {
/// Generic IO error /// Generic IO error
IO(std::io::Error), IO(std::io::Error),
/// An invalid path that cannot be represented in Mercurial was found /// An invalid path that cannot be represented in Mercurial was found
Path(HgPathError), Path(HgPathError),
removed, removed,
deleted, deleted,
clean, clean,
ignored, ignored,
unknown, unknown,
bad, bad,
unsure, unsure,
traversed, traversed,
dirty: false,
} }
} }
/// Get the status of files in the working directory. /// Get the status of files in the working directory.
/// ///
/// This is the current entry-point for `hg-core` and is realistically unusable /// This is the current entry-point for `hg-core` and is realistically unusable
/// outside of a Python context because its arguments need to provide a lot of /// outside of a Python context because its arguments need to provide a lot of
/// information that will not be necessary in the future. /// information that will not be necessary in the future.

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

has_ignored_ancestor, has_ignored_ancestor,
dmap.root.as_ref(), dmap.root.as_ref(),
hg_path, hg_path,
&root_dir, &root_dir,
root_dir_metadata, root_dir_metadata,
root_cached_mtime, root_cached_mtime,
is_at_repo_root, is_at_repo_root,
)?; )?;
let outcome = common.outcome.into_inner().unwrap(); let mut outcome = common.outcome.into_inner().unwrap();
let to_add = common.cached_directory_mtimes_to_add.into_inner().unwrap(); let to_add = common.cached_directory_mtimes_to_add.into_inner().unwrap();
outcome.dirty = !to_add.is_empty();
AlphareUnsubmitted
Not Done

This does not seem semantically correct. The docstring for dirty says that it's any change to the dirstate that should be written, but this only concerns the directories cache.

Alphare: This does not seem semantically correct. The docstring for `dirty` says that it's any change to…
SimonSapinAuthorUnsubmitted
Done

Yes, and we want new cache entries to be written to disk?

SimonSapin: Yes, and we want new cache entries to be written to disk?
AlphareUnsubmitted
Not Done

Right, but the docstring for dirty implies that it covers any part of the dirstate that needs serializing to disk, while it is only for the cache.

Alphare: Right, but the docstring for `dirty` implies that it covers any part of the dirstate that needs…
AlphareUnsubmitted
Not Done

I was completely confused, we don't update the dirstate for anything other than caches outside of a commit. Sorry for the noise. :)

Alphare: I was completely confused, we don't update the dirstate for anything other than caches outside…
for (path, mtime) in &to_add { for (path, mtime) in &to_add {
let node = DirstateMap::get_or_insert_node( let node = DirstateMap::get_or_insert_node(
dmap.on_disk, dmap.on_disk,
&mut dmap.root, &mut dmap.root,
path, path,
WithBasename::to_cow_owned, WithBasename::to_cow_owned,
|_| {}, |_| {},
)?; )?;

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

let removed = collect_pybytes_list(py, status_res.removed.as_ref()); let removed = collect_pybytes_list(py, status_res.removed.as_ref());
let deleted = collect_pybytes_list(py, status_res.deleted.as_ref()); let deleted = collect_pybytes_list(py, status_res.deleted.as_ref());
let clean = collect_pybytes_list(py, status_res.clean.as_ref()); let clean = collect_pybytes_list(py, status_res.clean.as_ref());
let ignored = collect_pybytes_list(py, status_res.ignored.as_ref()); let ignored = collect_pybytes_list(py, status_res.ignored.as_ref());
let unknown = collect_pybytes_list(py, status_res.unknown.as_ref()); let unknown = collect_pybytes_list(py, status_res.unknown.as_ref());
let unsure = collect_pybytes_list(py, status_res.unsure.as_ref()); let unsure = collect_pybytes_list(py, status_res.unsure.as_ref());
let bad = collect_bad_matches(py, status_res.bad.as_ref())?; let bad = collect_bad_matches(py, status_res.bad.as_ref())?;
let traversed = collect_pybytes_list(py, status_res.traversed.as_ref()); let traversed = collect_pybytes_list(py, status_res.traversed.as_ref());
let dirty = status_res.dirty.to_py_object(py);
let py_warnings = PyList::new(py, &[]); let py_warnings = PyList::new(py, &[]);
for warning in warnings.iter() { for warning in warnings.iter() {
// We use duck-typing on the Python side for dispatch, good enough for // We use duck-typing on the Python side for dispatch, good enough for
// now. // now.
match warning { match warning {
PatternFileWarning::InvalidSyntax(file, syn) => { PatternFileWarning::InvalidSyntax(file, syn) => {
py_warnings.append( py_warnings.append(
py, py,
removed.into_object(), removed.into_object(),
deleted.into_object(), deleted.into_object(),
clean.into_object(), clean.into_object(),
ignored.into_object(), ignored.into_object(),
unknown.into_object(), unknown.into_object(),
py_warnings.into_object(), py_warnings.into_object(),
bad.into_object(), bad.into_object(),
traversed.into_object(), traversed.into_object(),
dirty.into_object(),
][..], ][..],
)) ))
} }