This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11840

rhg: Update the dirstate on disk after status
ClosedPublic

Authored by SimonSapin on Dec 2 2021, 11:48 AM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG000130cfafb6: rhg: Update the dirstate on disk after status

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

SimonSapin created this revision.Dec 2 2021, 11:48 AM
Alphare requested changes to this revision.Dec 3 2021, 9:12 AM
Alphare added a subscriber: Alphare.
Alphare added inline comments.
rust/rhg/src/commands/status.rs
476

I think we should add a comment here explaining why this racy code is not racy (because it's gated behind the mtime_boundary and any change that is caught in this boundary is discarded from this anyway).

This revision now requires changes to proceed.Dec 3 2021, 9:12 AM
SimonSapin updated this revision to Diff 31321.Dec 6 2021, 5:03 AM
SimonSapin updated this revision to Diff 31326.Dec 6 2021, 6:48 PM
SimonSapin added inline comments.Dec 7 2021, 4:07 AM
rust/rhg/src/commands/status.rs
476

This was actually missing all of the mtime_boundary logic since I had ported to Rust an earlier version of the Python code, without the recent changes that added mtime_boundary during ambiguous file resolution. This is fixed in the latest revision of this patch.

Alphare accepted this revision.Dec 7 2021, 9:59 AM
This revision is now accepted and ready to land.Dec 7 2021, 9:59 AM
SimonSapin added a commit: rHG000130cfafb6: rhg: Update the dirstate on disk after status.Dec 7 2021, 10:05 AM
Closed by commit rHG000130cfafb6: rhg: Update the dirstate on disk after status (authored by SimonSapin). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 31355

mercurial/dirstateutils/timestamp.py

return timestamp((secs, subsec_nanos, False)) return timestamp((secs, subsec_nanos, False))
def reliable_mtime_of(stat_result, present_mtime): def reliable_mtime_of(stat_result, present_mtime):
"""same as `mtime_of`, but return None if the date might be ambiguous """same as `mtime_of`, but return None if the date might be ambiguous
A modification time is reliable if it is older than "present_time" (or A modification time is reliable if it is older than "present_time" (or
sufficiently in the futur). sufficiently in the future).
Otherwise a concurrent modification might happens with the same mtime. Otherwise a concurrent modification might happens with the same mtime.
""" """
file_mtime = mtime_of(stat_result) file_mtime = mtime_of(stat_result)
file_second = file_mtime[0] file_second = file_mtime[0]
file_ns = file_mtime[1] file_ns = file_mtime[1]
boundary_second = present_mtime[0] boundary_second = present_mtime[0]
boundary_ns = present_mtime[1] boundary_ns = present_mtime[1]

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

Ok(Self::new_truncate(seconds, nanoseconds, false)) Ok(Self::new_truncate(seconds, nanoseconds, false))
} }
#[cfg(not(unix))] #[cfg(not(unix))]
{ {
metadata.modified().map(Self::from) metadata.modified().map(Self::from)
} }
} }
/// Returns whether this timestamp is reliable as the "mtime" of a file.
///
/// A modification time is reliable if it is older than `boundary` (or
/// sufficiently in the future).
///
/// Otherwise a concurrent modification might happens with the same mtime.
pub fn is_reliable_mtime(&self, boundary: &Self) -> bool {
// If the mtime of the ambiguous file is younger (or equal) to the
// starting point of the `status` walk, we cannot garantee that
// another, racy, write will not happen right after with the same mtime
// and we cannot cache the information.
//
// However if the mtime is far away in the future, this is likely some
// mismatch between the current clock and previous file system
// operation. So mtime more than one days in the future are considered
// fine.
if self.truncated_seconds == boundary.truncated_seconds {
self.nanoseconds != 0
&& boundary.nanoseconds != 0
&& self.nanoseconds < boundary.nanoseconds
} else {
// `truncated_seconds` is less than 2**31,
// so this does not overflow `u32`:
let one_day_later = boundary.truncated_seconds + 24 * 3600;
self.truncated_seconds < boundary.truncated_seconds
|| self.truncated_seconds > one_day_later
}
}
/// The lower 31 bits of the number of seconds since the epoch. /// The lower 31 bits of the number of seconds since the epoch.
pub fn truncated_seconds(&self) -> u32 { pub fn truncated_seconds(&self) -> u32 {
self.truncated_seconds self.truncated_seconds
} }
/// The sub-second component of this timestamp, in nanoseconds. /// The sub-second component of this timestamp, in nanoseconds.
/// Always in the `0 .. 1_000_000_000` range. /// Always in the `0 .. 1_000_000_000` range.
/// ///
} }
} }
}; };
Self::new_truncate(seconds, nanoseconds, false) Self::new_truncate(seconds, nanoseconds, false)
} }
} }
const NSEC_PER_SEC: u32 = 1_000_000_000; const NSEC_PER_SEC: u32 = 1_000_000_000;
const RANGE_MASK_31BIT: u32 = 0x7FFF_FFFF; pub const RANGE_MASK_31BIT: u32 = 0x7FFF_FFFF;
pub const MTIME_UNSET: i32 = -1; pub const MTIME_UNSET: i32 = -1;
/// A `DirstateEntry` with a size of `-2` means that it was merged from the /// A `DirstateEntry` with a size of `-2` means that it was merged from the
/// other parent. This allows revert to pick the right status back during a /// other parent. This allows revert to pick the right status back during a
/// merge. /// merge.
pub const SIZE_FROM_OTHER_PARENT: i32 = -2; pub const SIZE_FROM_OTHER_PARENT: i32 = -2;
/// A special value used for internal representation of special case in /// A special value used for internal representation of special case in

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

pub list_ignored: bool, pub list_ignored: bool,
/// Whether to collect traversed dirs for applying a callback later. /// Whether to collect traversed dirs for applying a callback later.
/// Used by `hg purge` for example. /// Used by `hg purge` for example.
pub collect_traversed_dirs: bool, pub collect_traversed_dirs: bool,
} }
#[derive(Debug, Default)] #[derive(Debug, Default)]
pub struct DirstateStatus<'a> { pub struct DirstateStatus<'a> {
/// The current time at the start of the `status()` algorithm, as measured
/// and possibly truncated by the filesystem.
pub filesystem_time_at_status_start: Option<std::time::SystemTime>,
/// Tracked files whose contents have changed since the parent revision /// Tracked files whose contents have changed since the parent revision
pub modified: Vec<HgPathCow<'a>>, pub modified: Vec<HgPathCow<'a>>,
/// Newly-tracked files that were not present in the parent /// Newly-tracked files that were not present in the parent
pub added: Vec<HgPathCow<'a>>, pub added: Vec<HgPathCow<'a>>,
/// Previously-tracked files that have been (re)moved with an hg command /// Previously-tracked files that have been (re)moved with an hg command
pub removed: Vec<HgPathCow<'a>>, pub removed: Vec<HgPathCow<'a>>,

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

let new_hash = *hasher.finalize().as_ref(); let new_hash = *hasher.finalize().as_ref();
let changed = new_hash != dmap.ignore_patterns_hash; let changed = new_hash != dmap.ignore_patterns_hash;
dmap.ignore_patterns_hash = new_hash; dmap.ignore_patterns_hash = new_hash;
(ignore_fn, warnings, Some(changed)) (ignore_fn, warnings, Some(changed))
} else { } else {
(Box::new(|&_| true), vec![], None) (Box::new(|&_| true), vec![], None)
}; };
let filesystem_time_at_status_start = filesystem_now(&root_dir).ok();
let outcome = DirstateStatus {
filesystem_time_at_status_start,
..Default::default()
};
let common = StatusCommon { let common = StatusCommon {
dmap, dmap,
options, options,
matcher, matcher,
ignore_fn, ignore_fn,
outcome: Default::default(), outcome: Mutex::new(outcome),
ignore_patterns_have_changed: patterns_changed, ignore_patterns_have_changed: patterns_changed,
new_cachable_directories: Default::default(), new_cachable_directories: Default::default(),
outated_cached_directories: Default::default(), outated_cached_directories: Default::default(),
filesystem_time_at_status_start: filesystem_now(&root_dir).ok(), filesystem_time_at_status_start,
}; };
let is_at_repo_root = true; let is_at_repo_root = true;
let hg_path = &BorrowedPath::OnDisk(HgPath::new("")); let hg_path = &BorrowedPath::OnDisk(HgPath::new(""));
let has_ignored_ancestor = false; let has_ignored_ancestor = false;
let root_cached_mtime = None; let root_cached_mtime = None;
let root_dir_metadata = None; let root_dir_metadata = None;
// If the path we have for the repository root is a symlink, do follow it. // If the path we have for the repository root is a symlink, do follow it.
// (As opposed to symlinks within the working directory which are not // (As opposed to symlinks within the working directory which are not

rust/rhg/src/commands/status.rs

// status.rs // status.rs
// //
// Copyright 2020, Georges Racinet <georges.racinets@octobus.net> // Copyright 2020, Georges Racinet <georges.racinets@octobus.net>
// //
// This software may be used and distributed according to the terms of the // This software may be used and distributed according to the terms of the
// GNU General Public License version 2 or any later version. // GNU General Public License version 2 or any later version.
use crate::error::CommandError; use crate::error::CommandError;
use crate::ui::Ui; use crate::ui::Ui;
use crate::utils::path_utils::relativize_paths; use crate::utils::path_utils::relativize_paths;
use clap::{Arg, SubCommand}; use clap::{Arg, SubCommand};
use format_bytes::format_bytes; use format_bytes::format_bytes;
use hg; use hg;
use hg::config::Config; use hg::config::Config;
use hg::dirstate::has_exec_bit; use hg::dirstate::has_exec_bit;
use hg::errors::HgError; use hg::dirstate::TruncatedTimestamp;
use hg::dirstate::RANGE_MASK_31BIT;
use hg::errors::{HgError, IoResultExt};
use hg::lock::LockError;
use hg::manifest::Manifest; use hg::manifest::Manifest;
use hg::matchers::AlwaysMatcher; use hg::matchers::AlwaysMatcher;
use hg::repo::Repo; use hg::repo::Repo;
use hg::utils::files::get_bytes_from_os_string; use hg::utils::files::get_bytes_from_os_string;
use hg::utils::hg_path::{hg_path_to_os_string, HgPath}; use hg::utils::hg_path::{hg_path_to_path_buf, HgPath};
use hg::{HgPathCow, StatusOptions}; use hg::{HgPathCow, StatusOptions};
use log::{info, warn}; use log::{info, warn};
use std::io;
pub const HELP_TEXT: &str = " pub const HELP_TEXT: &str = "
Show changed files in the working directory Show changed files in the working directory
This is a pure Rust version of `hg status`. This is a pure Rust version of `hg status`.
Some options might be missing, check the list below. Some options might be missing, check the list below.
"; ";
warn!("Bad matches {:?}", &(ds_status.bad)) warn!("Bad matches {:?}", &(ds_status.bad))
} }
if !ds_status.unsure.is_empty() { if !ds_status.unsure.is_empty() {
info!( info!(
"Files to be rechecked by retrieval from filelog: {:?}", "Files to be rechecked by retrieval from filelog: {:?}",
&ds_status.unsure &ds_status.unsure
); );
} }
let mut fixup = Vec::new();
if !ds_status.unsure.is_empty() if !ds_status.unsure.is_empty()
&& (display_states.modified || display_states.clean) && (display_states.modified || display_states.clean)
{ {
let p1 = repo.dirstate_parents()?.p1; let p1 = repo.dirstate_parents()?.p1;
let manifest = repo.manifest_for_node(p1).map_err(|e| { let manifest = repo.manifest_for_node(p1).map_err(|e| {
CommandError::from((e, &*format!("{:x}", p1.short()))) CommandError::from((e, &*format!("{:x}", p1.short())))
})?; })?;
for to_check in ds_status.unsure { for to_check in ds_status.unsure {
if unsure_is_modified(repo, &manifest, &to_check)? { if unsure_is_modified(repo, &manifest, &to_check)? {
if display_states.modified { if display_states.modified {
ds_status.modified.push(to_check); ds_status.modified.push(to_check);
} }
} else { } else {
if display_states.clean { if display_states.clean {
ds_status.clean.push(to_check); ds_status.clean.push(to_check.clone());
} }
fixup.push(to_check.into_owned())
} }
} }
} }
if display_states.modified { if display_states.modified {
display_status_paths( display_status_paths(
ui, ui,
repo, repo,
config, config,
ui, ui,
repo, repo,
config, config,
no_status, no_status,
&mut ds_status.clean, &mut ds_status.clean,
b"C", b"C",
)?; )?;
} }
let mut dirstate_write_needed = ds_status.dirty;
let filesystem_time_at_status_start = ds_status
.filesystem_time_at_status_start
.map(TruncatedTimestamp::from);
if (fixup.is_empty() || filesystem_time_at_status_start.is_none())
&& !dirstate_write_needed
{
// Nothing to update
return Ok(());
}
// Update the dirstate on disk if we can
let with_lock_result =
repo.try_with_wlock_no_wait(|| -> Result<(), CommandError> {
if let Some(mtime_boundary) = filesystem_time_at_status_start {
for hg_path in fixup {
use std::os::unix::fs::MetadataExt;
let fs_path = hg_path_to_path_buf(&hg_path)
.expect("HgPath conversion");
// Specifically do not reuse `fs_metadata` from
// `unsure_is_clean` which was needed before reading
// contents. Here we access metadata again after reading
// content, in case it changed in the meantime.
let fs_metadata = repo
.working_directory_vfs()
.symlink_metadata(&fs_path)?;
let mtime = TruncatedTimestamp::for_mtime_of(&fs_metadata)
.when_reading_file(&fs_path)?;
if mtime.is_reliable_mtime(&mtime_boundary) {
let mode = fs_metadata.mode();
let size = fs_metadata.len() as u32 & RANGE_MASK_31BIT;
let mut entry = dmap
.get(&hg_path)?
.expect("ambiguous file not in dirstate");
entry.set_clean(mode, size, mtime);
dmap.add_file(&hg_path, entry)?;
dirstate_write_needed = true
}
}
}
drop(dmap); // Avoid "already mutably borrowed" RefCell panics
if dirstate_write_needed {
repo.write_dirstate()?
}
Ok(())
});
match with_lock_result {
Ok(closure_result) => closure_result?,
Err(LockError::AlreadyHeld) => {
// Not updating the dirstate is not ideal but not critical:
// don’t keep our caller waiting until some other Mercurial
// process releases the lock.
}
Err(LockError::Other(HgError::IoError { error, .. }))
if error.kind() == io::ErrorKind::PermissionDenied =>
{
// `hg status` on a read-only repository is fine
}
Err(LockError::Other(error)) => {
// Report other I/O errors
Err(error)?
}
}
Ok(()) Ok(())
} }
// Probably more elegant to use a Deref or Borrow trait rather than // Probably more elegant to use a Deref or Borrow trait rather than
// harcode HgPathBuf, but probably not really useful at this point // harcode HgPathBuf, but probably not really useful at this point
fn display_status_paths( fn display_status_paths(
ui: &Ui, ui: &Ui,
repo: &Repo, repo: &Repo,
/// This meant to be used for those that the dirstate cannot resolve, due /// This meant to be used for those that the dirstate cannot resolve, due
/// to time resolution limits. /// to time resolution limits.
fn unsure_is_modified( fn unsure_is_modified(
repo: &Repo, repo: &Repo,
manifest: &Manifest, manifest: &Manifest,
hg_path: &HgPath, hg_path: &HgPath,
) -> Result<bool, HgError> { ) -> Result<bool, HgError> {
let vfs = repo.working_directory_vfs(); let vfs = repo.working_directory_vfs();
let fs_path = hg_path_to_os_string(hg_path).expect("HgPath conversion"); let fs_path = hg_path_to_path_buf(hg_path).expect("HgPath conversion");
let fs_metadata = vfs.symlink_metadata(&fs_path)?; let fs_metadata = vfs.symlink_metadata(&fs_path)?;
let is_symlink = fs_metadata.file_type().is_symlink(); let is_symlink = fs_metadata.file_type().is_symlink();
// TODO: Also account for `FALLBACK_SYMLINK` and `FALLBACK_EXEC` from the // TODO: Also account for `FALLBACK_SYMLINK` and `FALLBACK_EXEC` from the
// dirstate // dirstate
let fs_flags = if is_symlink { let fs_flags = if is_symlink {
Some(b'l') Some(b'l')
} else if has_exec_bit(&fs_metadata) { } else if has_exec_bit(&fs_metadata) {
Some(b'x') Some(b'x')
})?; })?;
let contents_in_p1 = filelog_entry.data()?; let contents_in_p1 = filelog_entry.data()?;
let fs_contents = if is_symlink { let fs_contents = if is_symlink {
get_bytes_from_os_string(vfs.read_link(fs_path)?.into_os_string()) get_bytes_from_os_string(vfs.read_link(fs_path)?.into_os_string())
} else { } else {
vfs.read(fs_path)? vfs.read(fs_path)?
}; };
return Ok(contents_in_p1 != &*fs_contents); Ok(contents_in_p1 != &*fs_contents)
} }
AlphareUnsubmitted
Not Done

I think we should add a comment here explaining why this racy code is not racy (because it's gated behind the mtime_boundary and any change that is caught in this boundary is discarded from this anyway).

Alphare: I think we should add a comment here explaining why this racy code is not racy (because it's…
SimonSapinAuthorUnsubmitted
Done

This was actually missing all of the mtime_boundary logic since I had ported to Rust an earlier version of the Python code, without the recent changes that added mtime_boundary during ambiguous file resolution. This is fixed in the latest revision of this patch.

SimonSapin: This was actually missing all of the `mtime_boundary` logic since I had ported to Rust an…

tests/test-backout.t

$ echo 'newly added' > c $ echo 'newly added' > c
$ hg add c $ hg add c
$ hg remove b $ hg remove b
$ hg commit -d '5 0' -m 'prepare for subsequent backout' $ hg commit -d '5 0' -m 'prepare for subsequent backout'
$ touch -t 200001010000 c $ touch -t 200001010000 c
$ hg status -A $ hg status -A
C c C c
$ hg debugstate --no-dates $ hg debugstate --no-dates
n 644 12 set c (no-rhg !) n 644 12 set c
n 0 -1 unset c (rhg known-bad-output !)
$ hg backout -d '6 0' -m 'to be rollback-ed soon' -r . $ hg backout -d '6 0' -m 'to be rollback-ed soon' -r .
removing c removing c
adding b adding b
changeset 6:4bfec048029d backs out changeset 5:fac0b729a654 changeset 6:4bfec048029d backs out changeset 5:fac0b729a654
$ hg rollback -q $ hg rollback -q
$ hg status -A $ hg status -A
A b A b
R c R c

tests/test-dirstate-race2.t

#testcases dirstate-v1 dirstate-v2 #testcases dirstate-v1 dirstate-v2
#if dirstate-v2 #if dirstate-v2
$ cat >> $HGRCPATH << EOF $ cat >> $HGRCPATH << EOF
> [format] > [format]
> exp-rc-dirstate-v2=1 > exp-rc-dirstate-v2=1
> [storage] > [storage]
> dirstate-v2.slow-path=allow > dirstate-v2.slow-path=allow
> EOF > EOF
#endif #endif
TODO: fix rhg bugs that make this test fail when status is enabled
$ unset RHG_STATUS
Checking the size/permissions/file-type of files stored in the Checking the size/permissions/file-type of files stored in the
dirstate after an update where the files are changed concurrently dirstate after an update where the files are changed concurrently
outside of hg's control. outside of hg's control.
$ hg init repo $ hg init repo
$ cd repo $ cd repo
$ echo a > a $ echo a > a
$ hg commit -qAm _ $ hg commit -qAm _

tests/test-dirstate.t

#testcases dirstate-v1 dirstate-v2 #testcases dirstate-v1 dirstate-v2
#if dirstate-v2 #if dirstate-v2
$ cat >> $HGRCPATH << EOF $ cat >> $HGRCPATH << EOF
> [format] > [format]
> exp-rc-dirstate-v2=1 > exp-rc-dirstate-v2=1
> [storage] > [storage]
> dirstate-v2.slow-path=allow > dirstate-v2.slow-path=allow
> EOF > EOF
#endif #endif
TODO: fix rhg bugs that make this test fail when status is enabled
$ unset RHG_STATUS
------ Test dirstate._dirs refcounting ------ Test dirstate._dirs refcounting
$ hg init t $ hg init t
$ cd t $ cd t
$ mkdir -p a/b/c/d $ mkdir -p a/b/c/d
$ touch a/b/c/d/x $ touch a/b/c/d/x
$ touch a/b/c/d/y $ touch a/b/c/d/y
$ touch a/b/c/d/z $ touch a/b/c/d/z