rust: Add Repo::dirstate_map and use it in `rhg status`

This moves low-level dirstate wrangling out of the status command and into
a more reusable location.

The open dirstate map is lazily initialized and kept on the Repo object,
for reuse by sub-sequent calls.

Differential Revision: https://phab.mercurial-scm.org/D11398

rust/hg-core/src/dirstate.rs

@@ -1,160 +1,160 b''
 // dirstate module
 //
 // Copyright 2019 Raphaël Gomès <[e-mail]>
 //
 // This software may be used and distributed according to the terms of the
 // GNU General Public License version 2 or any later version.
 
 use crate::dirstate_tree::on_disk::DirstateV2ParseError;
 use crate::errors::HgError;
 use crate::revlog::node::NULL_NODE;
 use crate::revlog::Node;
 use crate::utils::hg_path::{HgPath, HgPathBuf};
 use crate::FastHashMap;
 use bytes_cast::{unaligned, BytesCast};
 use std::convert::TryFrom;
 
 pub mod dirs_multiset;
 pub mod dirstate_map;
 pub mod parsers;
 pub mod status;
 
-#[derive(Debug, PartialEq, Clone, BytesCast)]
+#[derive(Debug, PartialEq, Copy, Clone, BytesCast)]
 #[repr(C)]
 pub struct DirstateParents {
     pub p1: Node,
     pub p2: Node,
 }
 
 impl DirstateParents {
     pub const NULL: Self = Self {
         p1: NULL_NODE,
         p2: NULL_NODE,
     };
 }
 
 /// The C implementation uses all signed types. This will be an issue
 /// either when 4GB+ source files are commonplace or in 2038, whichever
 /// comes first.
 #[derive(Debug, PartialEq, Copy, Clone)]
 pub struct DirstateEntry {
     pub state: EntryState,
     pub mode: i32,
     pub mtime: i32,
     pub size: i32,
 }
 
 impl DirstateEntry {
     pub fn is_non_normal(&self) -> bool {
         self.state != EntryState::Normal || self.mtime == MTIME_UNSET
     }
 
     pub fn is_from_other_parent(&self) -> bool {
         self.state == EntryState::Normal && self.size == SIZE_FROM_OTHER_PARENT
     }
 
     // TODO: other platforms
     #[cfg(unix)]
     pub fn mode_changed(
         &self,
         filesystem_metadata: &std::fs::Metadata,
     ) -> bool {
         use std::os::unix::fs::MetadataExt;
         const EXEC_BIT_MASK: u32 = 0o100;
         let dirstate_exec_bit = (self.mode as u32) & EXEC_BIT_MASK;
         let fs_exec_bit = filesystem_metadata.mode() & EXEC_BIT_MASK;
         dirstate_exec_bit != fs_exec_bit
     }
 
     /// Returns a `(state, mode, size, mtime)` tuple as for
     /// `DirstateMapMethods::debug_iter`.
     pub fn debug_tuple(&self) -> (u8, i32, i32, i32) {
         (self.state.into(), self.mode, self.size, self.mtime)
     }
 }
 
 #[derive(BytesCast)]
 #[repr(C)]
 struct RawEntry {
     state: u8,
     mode: unaligned::I32Be,
     size: unaligned::I32Be,
     mtime: unaligned::I32Be,
     length: unaligned::I32Be,
 }
 
 pub const V1_RANGEMASK: i32 = 0x7FFFFFFF;
 
 pub const MTIME_UNSET: i32 = -1;
 
 /// 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
 /// merge.
 pub const SIZE_FROM_OTHER_PARENT: i32 = -2;
 /// A special value used for internal representation of special case in
 /// dirstate v1 format.
 pub const SIZE_NON_NORMAL: i32 = -1;
 
 pub type StateMap = FastHashMap<HgPathBuf, DirstateEntry>;
 pub type StateMapIter<'a> = Box<
     dyn Iterator<
             Item = Result<(&'a HgPath, DirstateEntry), DirstateV2ParseError>,
         > + Send
         + 'a,
 >;
 
 pub type CopyMap = FastHashMap<HgPathBuf, HgPathBuf>;
 pub type CopyMapIter<'a> = Box<
     dyn Iterator<Item = Result<(&'a HgPath, &'a HgPath), DirstateV2ParseError>>
         + Send
         + 'a,
 >;
 
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 pub enum EntryState {
     Normal,
     Added,
     Removed,
     Merged,
     Unknown,
 }
 
 impl EntryState {
     pub fn is_tracked(self) -> bool {
         use EntryState::*;
         match self {
             Normal | Added | Merged => true,
             Removed | Unknown => false,
         }
     }
 }
 
 impl TryFrom<u8> for EntryState {
     type Error = HgError;
 
     fn try_from(value: u8) -> Result<Self, Self::Error> {
         match value {
             b'n' => Ok(EntryState::Normal),
             b'a' => Ok(EntryState::Added),
             b'r' => Ok(EntryState::Removed),
             b'm' => Ok(EntryState::Merged),
             b'?' => Ok(EntryState::Unknown),
             _ => Err(HgError::CorruptedRepository(format!(
                 "Incorrect dirstate entry state {}",
                 value
             ))),
         }
     }
 }
 
 impl Into<u8> for EntryState {
     fn into(self) -> u8 {
         match self {
             EntryState::Normal => b'n',
             EntryState::Added => b'a',
             EntryState::Removed => b'r',
             EntryState::Merged => b'm',
             EntryState::Unknown => b'?',
         }
     }
 }

rust/hg-core/src/repo.rs

@@ -1,246 +1,339 b''
 use crate::config::{Config, ConfigError, ConfigParseError};
+use crate::dirstate::DirstateParents;
+use crate::dirstate_tree::dirstate_map::DirstateMap;
+use crate::dirstate_tree::owning::OwningDirstateMap;
 use crate::errors::HgError;
+use crate::errors::HgResultExt;
 use crate::exit_codes;
 use crate::requirements;
 use crate::utils::files::get_path_from_bytes;
 use crate::utils::SliceExt;
 use crate::vfs::{is_dir, is_file, Vfs};
+use crate::DirstateError;
+use std::cell::{Cell, Ref, RefCell, RefMut};
 use std::collections::HashSet;
 use std::path::{Path, PathBuf};
 
 /// A repository on disk
 pub struct Repo {
     working_directory: PathBuf,
     dot_hg: PathBuf,
     store: PathBuf,
     requirements: HashSet<String>,
     config: Config,
+    // None means not known/initialized yet
+    dirstate_parents: Cell<Option<DirstateParents>>,
+    dirstate_map: RefCell<Option<OwningDirstateMap>>,
 }
 
 #[derive(Debug, derive_more::From)]
 pub enum RepoError {
     NotFound {
         at: PathBuf,
     },
     #[from]
     ConfigParseError(ConfigParseError),
     #[from]
     Other(HgError),
 }
 
 impl From<ConfigError> for RepoError {
     fn from(error: ConfigError) -> Self {
         match error {
             ConfigError::Parse(error) => error.into(),
             ConfigError::Other(error) => error.into(),
         }
     }
 }
 
 impl Repo {
     /// tries to find nearest repository root in current working directory or
     /// its ancestors
     pub fn find_repo_root() -> Result<PathBuf, RepoError> {
         let current_directory = crate::utils::current_dir()?;
         // ancestors() is inclusive: it first yields `current_directory`
         // as-is.
         for ancestor in current_directory.ancestors() {
             if is_dir(ancestor.join(".hg"))? {
                 return Ok(ancestor.to_path_buf());
             }
         }
         return Err(RepoError::NotFound {
             at: current_directory,
         });
     }
 
     /// Find a repository, either at the given path (which must contain a `.hg`
     /// sub-directory) or by searching the current directory and its
     /// ancestors.
     ///
     /// A method with two very different "modes" like this usually a code smell
     /// to make two methods instead, but in this case an `Option` is what rhg
     /// sub-commands get from Clap for the `-R` / `--repository` CLI argument.
     /// Having two methods would just move that `if` to almost all callers.
     pub fn find(
         config: &Config,
         explicit_path: Option<PathBuf>,
     ) -> Result<Self, RepoError> {
         if let Some(root) = explicit_path {
             if is_dir(root.join(".hg"))? {
                 Self::new_at_path(root.to_owned(), config)
             } else if is_file(&root)? {
                 Err(HgError::unsupported("bundle repository").into())
             } else {
                 Err(RepoError::NotFound {
                     at: root.to_owned(),
                 })
             }
         } else {
             let root = Self::find_repo_root()?;
             Self::new_at_path(root, config)
         }
     }
 
     /// To be called after checking that `.hg` is a sub-directory
     fn new_at_path(
         working_directory: PathBuf,
         config: &Config,
     ) -> Result<Self, RepoError> {
         let dot_hg = working_directory.join(".hg");
 
         let mut repo_config_files = Vec::new();
         repo_config_files.push(dot_hg.join("hgrc"));
         repo_config_files.push(dot_hg.join("hgrc-not-shared"));
 
         let hg_vfs = Vfs { base: &dot_hg };
         let mut reqs = requirements::load_if_exists(hg_vfs)?;
         let relative =
             reqs.contains(requirements::RELATIVE_SHARED_REQUIREMENT);
         let shared =
             reqs.contains(requirements::SHARED_REQUIREMENT) || relative;
 
         // From `mercurial/localrepo.py`:
         //
         // if .hg/requires contains the sharesafe requirement, it means
         // there exists a `.hg/store/requires` too and we should read it
         // NOTE: presence of SHARESAFE_REQUIREMENT imply that store requirement
         // is present. We never write SHARESAFE_REQUIREMENT for a repo if store
         // is not present, refer checkrequirementscompat() for that
         //
         // However, if SHARESAFE_REQUIREMENT is not present, it means that the
         // repository was shared the old way. We check the share source
         // .hg/requires for SHARESAFE_REQUIREMENT to detect whether the
         // current repository needs to be reshared
         let share_safe = reqs.contains(requirements::SHARESAFE_REQUIREMENT);
 
         let store_path;
         if !shared {
             store_path = dot_hg.join("store");
         } else {
             let bytes = hg_vfs.read("sharedpath")?;
             let mut shared_path =
                 get_path_from_bytes(bytes.trim_end_matches(|b| b == b'\n'))
                     .to_owned();
             if relative {
                 shared_path = dot_hg.join(shared_path)
             }
             if !is_dir(&shared_path)? {
                 return Err(HgError::corrupted(format!(
                     ".hg/sharedpath points to nonexistent directory {}",
                     shared_path.display()
                 ))
                 .into());
             }
 
             store_path = shared_path.join("store");
 
             let source_is_share_safe =
                 requirements::load(Vfs { base: &shared_path })?
                     .contains(requirements::SHARESAFE_REQUIREMENT);
 
             if share_safe && !source_is_share_safe {
                 return Err(match config
                     .get(b"share", b"safe-mismatch.source-not-safe")
                 {
                     Some(b"abort") | None => HgError::abort(
                         "abort: share source does not support share-safe requirement\n\
                         (see `hg help config.format.use-share-safe` for more information)",
                         exit_codes::ABORT,
                     ),
                     _ => HgError::unsupported("share-safe downgrade"),
                 }
                 .into());
             } else if source_is_share_safe && !share_safe {
                 return Err(
                     match config.get(b"share", b"safe-mismatch.source-safe") {
                         Some(b"abort") | None => HgError::abort(
                             "abort: version mismatch: source uses share-safe \
                             functionality while the current share does not\n\
                             (see `hg help config.format.use-share-safe` for more information)",
                         exit_codes::ABORT,
                         ),
                         _ => HgError::unsupported("share-safe upgrade"),
                     }
                     .into(),
                 );
             }
 
             if share_safe {
                 repo_config_files.insert(0, shared_path.join("hgrc"))
             }
         }
         if share_safe {
             reqs.extend(requirements::load(Vfs { base: &store_path })?);
         }
 
         let repo_config = if std::env::var_os("HGRCSKIPREPO").is_none() {
             config.combine_with_repo(&repo_config_files)?
         } else {
             config.clone()
         };
 
         let repo = Self {
             requirements: reqs,
             working_directory,
             store: store_path,
             dot_hg,
             config: repo_config,
+            dirstate_parents: Cell::new(None),
+            dirstate_map: RefCell::new(None),
         };
 
         requirements::check(&repo)?;
 
         Ok(repo)
     }
 
     pub fn working_directory_path(&self) -> &Path {
         &self.working_directory
     }
 
     pub fn requirements(&self) -> &HashSet<String> {
         &self.requirements
     }
 
     pub fn config(&self) -> &Config {
         &self.config
     }
 
     /// For accessing repository files (in `.hg`), except for the store
     /// (`.hg/store`).
     pub fn hg_vfs(&self) -> Vfs<'_> {
         Vfs { base: &self.dot_hg }
     }
 
     /// For accessing repository store files (in `.hg/store`)
     pub fn store_vfs(&self) -> Vfs<'_> {
         Vfs { base: &self.store }
     }
 
     /// For accessing the working copy
     pub fn working_directory_vfs(&self) -> Vfs<'_> {
         Vfs {
             base: &self.working_directory,
         }
     }
 
     pub fn has_dirstate_v2(&self) -> bool {
         self.requirements
             .contains(requirements::DIRSTATE_V2_REQUIREMENT)
     }
 
-    pub fn dirstate_parents(
-        &self,
-    ) -> Result<crate::dirstate::DirstateParents, HgError> {
-        let dirstate = self.hg_vfs().mmap_open("dirstate")?;
-        if dirstate.is_empty() {
-            return Ok(crate::dirstate::DirstateParents::NULL);
+    fn dirstate_file_contents(&self) -> Result<Vec<u8>, HgError> {
+        Ok(self
+            .hg_vfs()
+            .read("dirstate")
+            .io_not_found_as_none()?
+            .unwrap_or(Vec::new()))
+    }
+
+    pub fn dirstate_parents(&self) -> Result<DirstateParents, HgError> {
+        if let Some(parents) = self.dirstate_parents.get() {
+            return Ok(parents);
         }
-        let parents = if self.has_dirstate_v2() {
+        let dirstate = self.dirstate_file_contents()?;
+        let parents = if dirstate.is_empty() {
+            DirstateParents::NULL
+        } else if self.has_dirstate_v2() {
             crate::dirstate_tree::on_disk::read_docket(&dirstate)?.parents()
         } else {
             crate::dirstate::parsers::parse_dirstate_parents(&dirstate)?
                 .clone()
         };
+        self.dirstate_parents.set(Some(parents));
         Ok(parents)
     }
+
+    fn new_dirstate_map(&self) -> Result<OwningDirstateMap, DirstateError> {
+        let dirstate_file_contents = self.dirstate_file_contents()?;
+        if dirstate_file_contents.is_empty() {
+            self.dirstate_parents.set(Some(DirstateParents::NULL));
+            Ok(OwningDirstateMap::new_empty(Vec::new()))
+        } else if self.has_dirstate_v2() {
+            let docket = crate::dirstate_tree::on_disk::read_docket(
+                &dirstate_file_contents,
+            )?;
+            self.dirstate_parents.set(Some(docket.parents()));
+            let data_size = docket.data_size();
+            let metadata = docket.tree_metadata();
+            let mut map = if let Some(data_mmap) = self
+                .hg_vfs()
+                .mmap_open(docket.data_filename())
+                .io_not_found_as_none()?
+            {
+                OwningDirstateMap::new_empty(MmapWrapper(data_mmap))
+            } else {
+                OwningDirstateMap::new_empty(Vec::new())
+            };
+            let (on_disk, placeholder) = map.get_mut_pair();
+            *placeholder = DirstateMap::new_v2(on_disk, data_size, metadata)?;
+            Ok(map)
+        } else {
+            let mut map = OwningDirstateMap::new_empty(dirstate_file_contents);
+            let (on_disk, placeholder) = map.get_mut_pair();
+            let (inner, parents) = DirstateMap::new_v1(on_disk)?;
+            self.dirstate_parents
+                .set(Some(parents.unwrap_or(DirstateParents::NULL)));
+            *placeholder = inner;
+            Ok(map)
+        }
+    }
+
+    pub fn dirstate_map(
+        &self,
+    ) -> Result<Ref<OwningDirstateMap>, DirstateError> {
+        let mut borrowed = self.dirstate_map.borrow();
+        if borrowed.is_none() {
+            drop(borrowed);
+            // Only use `borrow_mut` if it is really needed to avoid panic in
+            // case there is another outstanding borrow but mutation is not
+            // needed.
+            *self.dirstate_map.borrow_mut() = Some(self.new_dirstate_map()?);
+            borrowed = self.dirstate_map.borrow()
+        }
+        Ok(Ref::map(borrowed, |option| option.as_ref().unwrap()))
+    }
+
+    pub fn dirstate_map_mut(
+        &self,
+    ) -> Result<RefMut<OwningDirstateMap>, DirstateError> {
+        let mut borrowed = self.dirstate_map.borrow_mut();
+        if borrowed.is_none() {
+            *borrowed = Some(self.new_dirstate_map()?);
+        }
+        Ok(RefMut::map(borrowed, |option| option.as_mut().unwrap()))
+    }
 }
+
+// TODO: remove this when https://github.com/RazrFalcon/memmap2-rs/pull/22 is on crates.io
+struct MmapWrapper(memmap2::Mmap);
+
+impl std::ops::Deref for MmapWrapper {
+    type Target = [u8];
+
+    fn deref(&self) -> &[u8] {
+        self.0.deref()
+    }
+}
+
+unsafe impl stable_deref_trait::StableDeref for MmapWrapper {}

rust/rhg/src/commands/status.rs

@@ -1,347 +1,305 b''
 // status.rs
 //
 // Copyright 2020, Georges Racinet <[e-mail]>
 //
 // This software may be used and distributed according to the terms of the
 // GNU General Public License version 2 or any later version.
 
 use crate::error::CommandError;
 use crate::ui::Ui;
 use clap::{Arg, SubCommand};
 use hg;
-use hg::dirstate_tree::dirstate_map::DirstateMap;
-use hg::dirstate_tree::on_disk;
-use hg::errors::HgResultExt;
+use hg::dirstate_tree::dispatch::DirstateMapMethods;
 use hg::errors::IoResultExt;
 use hg::matchers::AlwaysMatcher;
 use hg::operations::cat;
 use hg::repo::Repo;
 use hg::revlog::node::Node;
 use hg::utils::hg_path::{hg_path_to_os_string, HgPath};
 use hg::StatusError;
 use hg::{HgPathCow, StatusOptions};
 use log::{info, warn};
 use std::convert::TryInto;
 use std::fs;
 use std::io::BufReader;
 use std::io::Read;
 
 pub const HELP_TEXT: &str = "
 Show changed files in the working directory
 
 This is a pure Rust version of `hg status`.
 
 Some options might be missing, check the list below.
 ";
 
 pub fn args() -> clap::App<'static, 'static> {
     SubCommand::with_name("status")
         .alias("st")
         .about(HELP_TEXT)
         .arg(
             Arg::with_name("all")
                 .help("show status of all files")
                 .short("-A")
                 .long("--all"),
         )
         .arg(
             Arg::with_name("modified")
                 .help("show only modified files")
                 .short("-m")
                 .long("--modified"),
         )
         .arg(
             Arg::with_name("added")
                 .help("show only added files")
                 .short("-a")
                 .long("--added"),
         )
         .arg(
             Arg::with_name("removed")
                 .help("show only removed files")
                 .short("-r")
                 .long("--removed"),
         )
         .arg(
             Arg::with_name("clean")
                 .help("show only clean files")
                 .short("-c")
                 .long("--clean"),
         )
         .arg(
             Arg::with_name("deleted")
                 .help("show only deleted files")
                 .short("-d")
                 .long("--deleted"),
         )
         .arg(
             Arg::with_name("unknown")
                 .help("show only unknown (not tracked) files")
                 .short("-u")
                 .long("--unknown"),
         )
         .arg(
             Arg::with_name("ignored")
                 .help("show only ignored files")
                 .short("-i")
                 .long("--ignored"),
         )
 }
 
 /// Pure data type allowing the caller to specify file states to display
 #[derive(Copy, Clone, Debug)]
 pub struct DisplayStates {
     pub modified: bool,
     pub added: bool,
     pub removed: bool,
     pub clean: bool,
     pub deleted: bool,
     pub unknown: bool,
     pub ignored: bool,
 }
 
 pub const DEFAULT_DISPLAY_STATES: DisplayStates = DisplayStates {
     modified: true,
     added: true,
     removed: true,
     clean: false,
     deleted: true,
     unknown: true,
     ignored: false,
 };
 
 pub const ALL_DISPLAY_STATES: DisplayStates = DisplayStates {
     modified: true,
     added: true,
     removed: true,
     clean: true,
     deleted: true,
     unknown: true,
     ignored: true,
 };
 
 impl DisplayStates {
     pub fn is_empty(&self) -> bool {
         !(self.modified
             || self.added
             || self.removed
             || self.clean
             || self.deleted
             || self.unknown
             || self.ignored)
     }
 }
 
 pub fn run(invocation: &crate::CliInvocation) -> Result<(), CommandError> {
     let status_enabled_default = false;
     let status_enabled = invocation.config.get_option(b"rhg", b"status")?;
     if !status_enabled.unwrap_or(status_enabled_default) {
         return Err(CommandError::unsupported(
             "status is experimental in rhg (enable it with 'rhg.status = true' \
             or enable fallback with 'rhg.on-unsupported = fallback')"
         ));
     }
 
     let ui = invocation.ui;
     let args = invocation.subcommand_args;
     let display_states = if args.is_present("all") {
         // TODO when implementing `--quiet`: it excludes clean files
         // from `--all`
         ALL_DISPLAY_STATES
     } else {
         let requested = DisplayStates {
             modified: args.is_present("modified"),
             added: args.is_present("added"),
             removed: args.is_present("removed"),
             clean: args.is_present("clean"),
             deleted: args.is_present("deleted"),
             unknown: args.is_present("unknown"),
             ignored: args.is_present("ignored"),
         };
         if requested.is_empty() {
             DEFAULT_DISPLAY_STATES
         } else {
             requested
         }
     };
 
     let repo = invocation.repo?;
-    let dirstate_data_mmap;
-    let (mut dmap, parents) = if repo.has_dirstate_v2() {
-        let docket_data =
-            repo.hg_vfs().read("dirstate").io_not_found_as_none()?;
-        let parents;
-        let dirstate_data;
-        let data_size;
-        let docket;
-        let tree_metadata;
-        if let Some(docket_data) = &docket_data {
-            docket = on_disk::read_docket(docket_data)?;
-            tree_metadata = docket.tree_metadata();
-            parents = Some(docket.parents());
-            data_size = docket.data_size();
-            dirstate_data_mmap = repo
-                .hg_vfs()
-                .mmap_open(docket.data_filename())
-                .io_not_found_as_none()?;
-            dirstate_data = dirstate_data_mmap.as_deref().unwrap_or(b"");
-        } else {
-            parents = None;
-            tree_metadata = b"";
-            data_size = 0;
-            dirstate_data = b"";
-        }
-        let dmap =
-            DirstateMap::new_v2(dirstate_data, data_size, tree_metadata)?;
-        (dmap, parents)
-    } else {
-        dirstate_data_mmap =
-            repo.hg_vfs().mmap_open("dirstate").io_not_found_as_none()?;
-        let dirstate_data = dirstate_data_mmap.as_deref().unwrap_or(b"");
-        DirstateMap::new_v1(dirstate_data)?
-    };
+    let mut dmap = repo.dirstate_map_mut()?;
 
     let options = StatusOptions {
         // TODO should be provided by the dirstate parsing and
         // hence be stored on dmap. Using a value that assumes we aren't
         // below the time resolution granularity of the FS and the
         // dirstate.
         last_normal_time: 0,
         // we're currently supporting file systems with exec flags only
         // anyway
         check_exec: true,
         list_clean: display_states.clean,
         list_unknown: display_states.unknown,
         list_ignored: display_states.ignored,
         collect_traversed_dirs: false,
     };
     let ignore_file = repo.working_directory_vfs().join(".hgignore"); // TODO hardcoded
-    let (mut ds_status, pattern_warnings) = hg::dirstate_tree::status::status(
-        &mut dmap,
+    let (mut ds_status, pattern_warnings) = dmap.status(
         &AlwaysMatcher,
         repo.working_directory_path().to_owned(),
         vec![ignore_file],
         options,
     )?;
     if !pattern_warnings.is_empty() {
         warn!("Pattern warnings: {:?}", &pattern_warnings);
     }
 
     if !ds_status.bad.is_empty() {
         warn!("Bad matches {:?}", &(ds_status.bad))
     }
     if !ds_status.unsure.is_empty() {
         info!(
             "Files to be rechecked by retrieval from filelog: {:?}",
             &ds_status.unsure
         );
     }
     if !ds_status.unsure.is_empty()
         && (display_states.modified || display_states.clean)
     {
-        let p1: Node = parents
-            .expect(
-                "Dirstate with no parents should not list any file to
-            be rechecked for modifications",
-            )
-            .p1
-            .into();
+        let p1: Node = repo.dirstate_parents()?.p1.into();
         let p1_hex = format!("{:x}", p1);
         for to_check in ds_status.unsure {
             if cat_file_is_modified(repo, &to_check, &p1_hex)? {
                 if display_states.modified {
                     ds_status.modified.push(to_check);
                 }
             } else {
                 if display_states.clean {
                     ds_status.clean.push(to_check);
                 }
             }
         }
     }
     if display_states.modified {
         display_status_paths(ui, &mut ds_status.modified, b"M")?;
     }
     if display_states.added {
         display_status_paths(ui, &mut ds_status.added, b"A")?;
     }
     if display_states.removed {
         display_status_paths(ui, &mut ds_status.removed, b"R")?;
     }
     if display_states.deleted {
         display_status_paths(ui, &mut ds_status.deleted, b"!")?;
     }
     if display_states.unknown {
         display_status_paths(ui, &mut ds_status.unknown, b"?")?;
     }
     if display_states.ignored {
         display_status_paths(ui, &mut ds_status.ignored, b"I")?;
     }
     if display_states.clean {
         display_status_paths(ui, &mut ds_status.clean, b"C")?;
     }
     Ok(())
 }
 
 // Probably more elegant to use a Deref or Borrow trait rather than
 // harcode HgPathBuf, but probably not really useful at this point
 fn display_status_paths(
     ui: &Ui,
     paths: &mut [HgPathCow],
     status_prefix: &[u8],
 ) -> Result<(), CommandError> {
     paths.sort_unstable();
     for path in paths {
         // Same TODO as in commands::root
         let bytes: &[u8] = path.as_bytes();
         // TODO optim, probably lots of unneeded copies here, especially
         // if out stream is buffered
         ui.write_stdout(&[status_prefix, b" ", bytes, b"\n"].concat())?;
     }
     Ok(())
 }
 
 /// Check if a file is modified by comparing actual repo store and file system.
 ///
 /// This meant to be used for those that the dirstate cannot resolve, due
 /// to time resolution limits.
 ///
 /// TODO: detect permission bits and similar metadata modifications
 fn cat_file_is_modified(
     repo: &Repo,
     hg_path: &HgPath,
     rev: &str,
 ) -> Result<bool, CommandError> {
     // TODO CatRev expects &[HgPathBuf], something like
     // &[impl Deref<HgPath>] would be nicer and should avoid the copy
     let path_bufs = [hg_path.into()];
     // TODO IIUC CatRev returns a simple Vec<u8> for all files
     //      being able to tell them apart as (path, bytes) would be nicer
     //      and OPTIM would allow manifest resolution just once.
     let output = cat(repo, rev, &path_bufs).map_err(|e| (e, rev))?;
 
     let fs_path = repo
         .working_directory_vfs()
         .join(hg_path_to_os_string(hg_path).expect("HgPath conversion"));
     let hg_data_len: u64 = match output.concatenated.len().try_into() {
         Ok(v) => v,
         Err(_) => {
             // conversion of data length to u64 failed,
             // good luck for any file to have this content
             return Ok(true);
         }
     };
     let fobj = fs::File::open(&fs_path).when_reading_file(&fs_path)?;
     if fobj.metadata().map_err(|e| StatusError::from(e))?.len() != hg_data_len
     {
         return Ok(true);
     }
     for (fs_byte, hg_byte) in
         BufReader::new(fobj).bytes().zip(output.concatenated)
     {
         if fs_byte.map_err(|e| StatusError::from(e))? != hg_byte {
             return Ok(true);
         }
     }
     Ok(false)
 }