rhg: Reuse manifest when checking status of multiple ambiguous files

When `rhg status` cannot determine whether a file is clean based on mtime and
size alone, it needs to compare its contents with those found in the parent
commit. Previously, rhg would find the (same) manifest of that commit again
for every such file. This is lifted out of the loop and reused.

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

rust/hg-core/src/operations/cat.rs

@@ -1,71 +1,71 b''
 // list_tracked_files.rs
 //
 // Copyright 2020 Antoine Cezar <[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::repo::Repo;
 use crate::revlog::revlog::RevlogError;
 use crate::revlog::Node;
 
 use crate::utils::hg_path::HgPathBuf;
 
 pub struct CatOutput {
     /// Whether any file in the manifest matched the paths given as CLI
     /// arguments
     pub found_any: bool,
     /// The contents of matching files, in manifest order
     pub concatenated: Vec<u8>,
     /// Which of the CLI arguments did not match any manifest file
     pub missing: Vec<HgPathBuf>,
     /// The node ID that the given revset was resolved to
     pub node: Node,
 }
 
 /// Output the given revision of files
 ///
 /// * `root`: Repository root
 /// * `rev`: The revision to cat the files from.
 /// * `files`: The files to output.
 pub fn cat<'a>(
     repo: &Repo,
     revset: &str,
     files: &'a [HgPathBuf],
 ) -> Result<CatOutput, RevlogError> {
     let rev = crate::revset::resolve_single(revset, repo)?;
-    let manifest = repo.manifest(rev)?;
+    let manifest = repo.manifest_for_rev(rev)?;
     let node = *repo
         .changelog()?
         .node_from_rev(rev)
         .expect("should succeed when repo.manifest did");
     let mut bytes = vec![];
     let mut matched = vec![false; files.len()];
     let mut found_any = false;
 
     for (manifest_file, node_bytes) in manifest.files_with_nodes() {
         for (cat_file, is_matched) in files.iter().zip(&mut matched) {
             if cat_file.as_bytes() == manifest_file.as_bytes() {
                 *is_matched = true;
                 found_any = true;
                 let file_log = repo.filelog(manifest_file)?;
                 let file_node = Node::from_hex_for_repo(node_bytes)?;
                 let entry = file_log.get_node(file_node)?;
                 bytes.extend(entry.data()?)
             }
         }
     }
 
     let missing: Vec<_> = files
         .iter()
         .zip(&matched)
         .filter(|pair| !*pair.1)
         .map(|pair| pair.0.clone())
         .collect();
     Ok(CatOutput {
         found_any,
         concatenated: bytes,
         missing,
         node,
     })
 }

rust/hg-core/src/operations/list_tracked_files.rs

@@ -1,82 +1,82 b''
 // list_tracked_files.rs
 //
 // Copyright 2020 Antoine Cezar <[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::parsers::parse_dirstate_entries;
 use crate::dirstate_tree::on_disk::{for_each_tracked_path, read_docket};
 use crate::errors::HgError;
 use crate::repo::Repo;
 use crate::revlog::manifest::Manifest;
 use crate::revlog::revlog::RevlogError;
 use crate::utils::hg_path::HgPath;
 use crate::DirstateError;
 use rayon::prelude::*;
 
 /// List files under Mercurial control in the working directory
 /// by reading the dirstate
 pub struct Dirstate {
     /// The `dirstate` content.
     content: Vec<u8>,
     v2_metadata: Option<Vec<u8>>,
 }
 
 impl Dirstate {
     pub fn new(repo: &Repo) -> Result<Self, HgError> {
         let mut content = repo.hg_vfs().read("dirstate")?;
         let v2_metadata = if repo.has_dirstate_v2() {
             let docket = read_docket(&content)?;
             let meta = docket.tree_metadata().to_vec();
             content = repo.hg_vfs().read(docket.data_filename())?;
             Some(meta)
         } else {
             None
         };
         Ok(Self {
             content,
             v2_metadata,
         })
     }
 
     pub fn tracked_files(&self) -> Result<Vec<&HgPath>, DirstateError> {
         let mut files = Vec::new();
         if !self.content.is_empty() {
             if let Some(meta) = &self.v2_metadata {
                 for_each_tracked_path(&self.content, meta, |path| {
                     files.push(path)
                 })?
             } else {
                 let _parents = parse_dirstate_entries(
                     &self.content,
                     |path, entry, _copy_source| {
                         if entry.state.is_tracked() {
                             files.push(path)
                         }
                         Ok(())
                     },
                 )?;
             }
         }
         files.par_sort_unstable();
         Ok(files)
     }
 }
 
 /// List files under Mercurial control at a given revision.
 pub fn list_rev_tracked_files(
     repo: &Repo,
     revset: &str,
 ) -> Result<FilesForRev, RevlogError> {
     let rev = crate::revset::resolve_single(revset, repo)?;
-    Ok(FilesForRev(repo.manifest(rev)?))
+    Ok(FilesForRev(repo.manifest_for_rev(rev)?))
 }
 
 pub struct FilesForRev(Manifest);
 
 impl FilesForRev {
     pub fn iter(&self) -> impl Iterator<Item = &HgPath> {
         self.0.files()
     }
 }

rust/hg-core/src/repo.rs

@@ -1,409 +1,419 b''
 use crate::changelog::Changelog;
 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::manifest::{Manifest, Manifestlog};
-use crate::requirements;
 use crate::revlog::filelog::Filelog;
 use crate::revlog::revlog::RevlogError;
 use crate::utils::files::get_path_from_bytes;
 use crate::utils::hg_path::HgPath;
 use crate::utils::SliceExt;
 use crate::vfs::{is_dir, is_file, Vfs};
-use crate::{exit_codes, Node};
+use crate::{requirements, NodePrefix};
 use crate::{DirstateError, Revision};
 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: LazyCell<OwningDirstateMap, DirstateError>,
     changelog: LazyCell<Changelog, HgError>,
     manifestlog: LazyCell<Manifestlog, HgError>,
 }
 
 #[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: LazyCell::new(Self::new_dirstate_map),
             changelog: LazyCell::new(Changelog::open),
             manifestlog: LazyCell::new(Manifestlog::open),
         };
 
         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)
     }
 
     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 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> {
         self.dirstate_map.get_or_init(self)
     }
 
     pub fn dirstate_map_mut(
         &self,
     ) -> Result<RefMut<OwningDirstateMap>, DirstateError> {
         self.dirstate_map.get_mut_or_init(self)
     }
 
     pub fn changelog(&self) -> Result<Ref<Changelog>, HgError> {
         self.changelog.get_or_init(self)
     }
 
     pub fn changelog_mut(&self) -> Result<RefMut<Changelog>, HgError> {
         self.changelog.get_mut_or_init(self)
     }
 
     pub fn manifestlog(&self) -> Result<Ref<Manifestlog>, HgError> {
         self.manifestlog.get_or_init(self)
     }
 
     pub fn manifestlog_mut(&self) -> Result<RefMut<Manifestlog>, HgError> {
         self.manifestlog.get_mut_or_init(self)
     }
 
+    /// Returns the manifest of the given node ID
+    pub fn manifest_for_node(
+        &self,
+        node: impl Into<NodePrefix>,
+    ) -> Result<Manifest, RevlogError> {
+        self.manifestlog()?.get_node(
+            self.changelog()?
+                .get_node(node.into())?
+                .manifest_node()?
+                .into(),
+        )
+    }
+
     /// Returns the manifest of the given revision
-    pub fn manifest(
+    pub fn manifest_for_rev(
         &self,
         revision: Revision,
     ) -> Result<Manifest, RevlogError> {
-        let changelog = self.changelog()?;
-        let manifest = self.manifestlog()?;
-        let changelog_entry = changelog.get_rev(revision)?;
-        let manifest_node =
-            Node::from_hex_for_repo(&changelog_entry.manifest_node()?)?;
-        manifest.get_node(manifest_node.into())
+        self.manifestlog()?.get_node(
+            self.changelog()?.get_rev(revision)?.manifest_node()?.into(),
+        )
     }
 
     pub fn filelog(&self, path: &HgPath) -> Result<Filelog, HgError> {
         Filelog::open(self, path)
     }
 }
 
 /// Lazily-initialized component of `Repo` with interior mutability
 ///
 /// This differs from `OnceCell` in that the value can still be "deinitialized"
 /// later by setting its inner `Option` to `None`.
 struct LazyCell<T, E> {
     value: RefCell<Option<T>>,
     // `Fn`s that don’t capture environment are zero-size, so this box does
     // not allocate:
     init: Box<dyn Fn(&Repo) -> Result<T, E>>,
 }
 
 impl<T, E> LazyCell<T, E> {
     fn new(init: impl Fn(&Repo) -> Result<T, E> + 'static) -> Self {
         Self {
             value: RefCell::new(None),
             init: Box::new(init),
         }
     }
 
     fn get_or_init(&self, repo: &Repo) -> Result<Ref<T>, E> {
         let mut borrowed = self.value.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.value.borrow_mut() = Some((self.init)(repo)?);
             borrowed = self.value.borrow()
         }
         Ok(Ref::map(borrowed, |option| option.as_ref().unwrap()))
     }
 
     pub fn get_mut_or_init(&self, repo: &Repo) -> Result<RefMut<T>, E> {
         let mut borrowed = self.value.borrow_mut();
         if borrowed.is_none() {
             *borrowed = Some((self.init)(repo)?);
         }
         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/hg-core/src/revlog/changelog.rs

@@ -1,65 +1,67 b''
 use crate::errors::HgError;
 use crate::repo::Repo;
 use crate::revlog::revlog::{Revlog, RevlogError};
 use crate::revlog::Revision;
 use crate::revlog::{Node, NodePrefix};
 
 /// A specialized `Revlog` to work with `changelog` data format.
 pub struct Changelog {
     /// The generic `revlog` format.
     pub(crate) revlog: Revlog,
 }
 
 impl Changelog {
     /// Open the `changelog` of a repository given by its root.
     pub fn open(repo: &Repo) -> Result<Self, HgError> {
         let revlog = Revlog::open(repo, "00changelog.i", None)?;
         Ok(Self { revlog })
     }
 
     /// Return the `ChangelogEntry` a given node id.
     pub fn get_node(
         &self,
         node: NodePrefix,
     ) -> Result<ChangelogEntry, RevlogError> {
         let rev = self.revlog.get_node_rev(node)?;
         self.get_rev(rev)
     }
 
     /// Return the `ChangelogEntry` of a given node revision.
     pub fn get_rev(
         &self,
         rev: Revision,
     ) -> Result<ChangelogEntry, RevlogError> {
         let bytes = self.revlog.get_rev_data(rev)?;
         Ok(ChangelogEntry { bytes })
     }
 
     pub fn node_from_rev(&self, rev: Revision) -> Option<&Node> {
         Some(self.revlog.index.get_entry(rev)?.hash())
     }
 }
 
 /// `Changelog` entry which knows how to interpret the `changelog` data bytes.
 #[derive(Debug)]
 pub struct ChangelogEntry {
     /// The data bytes of the `changelog` entry.
     bytes: Vec<u8>,
 }
 
 impl ChangelogEntry {
     /// Return an iterator over the lines of the entry.
     pub fn lines(&self) -> impl Iterator<Item = &[u8]> {
         self.bytes
             .split(|b| b == &b'\n')
             .filter(|line| !line.is_empty())
     }
 
     /// Return the node id of the `manifest` referenced by this `changelog`
     /// entry.
-    pub fn manifest_node(&self) -> Result<&[u8], RevlogError> {
-        self.lines()
-            .next()
-            .ok_or_else(|| HgError::corrupted("empty changelog entry").into())
+    pub fn manifest_node(&self) -> Result<Node, HgError> {
+        Node::from_hex_for_repo(
+            self.lines()
+                .next()
+                .ok_or_else(|| HgError::corrupted("empty changelog entry"))?,
+        )
     }
 }

rust/hg-core/src/revlog/manifest.rs

@@ -1,71 +1,84 b''
 use crate::errors::HgError;
 use crate::repo::Repo;
 use crate::revlog::revlog::{Revlog, RevlogError};
-use crate::revlog::NodePrefix;
 use crate::revlog::Revision;
+use crate::revlog::{Node, NodePrefix};
 use crate::utils::hg_path::HgPath;
 
 /// A specialized `Revlog` to work with `manifest` data format.
 pub struct Manifestlog {
     /// The generic `revlog` format.
     revlog: Revlog,
 }
 
 impl Manifestlog {
     /// Open the `manifest` of a repository given by its root.
     pub fn open(repo: &Repo) -> Result<Self, HgError> {
         let revlog = Revlog::open(repo, "00manifest.i", None)?;
         Ok(Self { revlog })
     }
 
     /// Return the `ManifestEntry` of a given node id.
     pub fn get_node(&self, node: NodePrefix) -> Result<Manifest, RevlogError> {
         let rev = self.revlog.get_node_rev(node)?;
         self.get_rev(rev)
     }
 
     /// Return the `ManifestEntry` of a given node revision.
     pub fn get_rev(&self, rev: Revision) -> Result<Manifest, RevlogError> {
         let bytes = self.revlog.get_rev_data(rev)?;
         Ok(Manifest { bytes })
     }
 }
 
 /// `Manifestlog` entry which knows how to interpret the `manifest` data bytes.
 #[derive(Debug)]
 pub struct Manifest {
     bytes: Vec<u8>,
 }
 
 impl Manifest {
     /// Return an iterator over the lines of the entry.
     pub fn lines(&self) -> impl Iterator<Item = &[u8]> {
         self.bytes
             .split(|b| b == &b'\n')
             .filter(|line| !line.is_empty())
     }
 
     /// Return an iterator over the files of the entry.
     pub fn files(&self) -> impl Iterator<Item = &HgPath> {
         self.lines().filter(|line| !line.is_empty()).map(|line| {
             let pos = line
                 .iter()
                 .position(|x| x == &b'\0')
                 .expect("manifest line should contain \\0");
             HgPath::new(&line[..pos])
         })
     }
 
     /// Return an iterator over the files of the entry.
     pub fn files_with_nodes(&self) -> impl Iterator<Item = (&HgPath, &[u8])> {
         self.lines().filter(|line| !line.is_empty()).map(|line| {
             let pos = line
                 .iter()
                 .position(|x| x == &b'\0')
                 .expect("manifest line should contain \\0");
             let hash_start = pos + 1;
             let hash_end = hash_start + 40;
             (HgPath::new(&line[..pos]), &line[hash_start..hash_end])
         })
     }
+
+    /// If the given path is in this manifest, return its filelog node ID
+    pub fn find_file(&self, path: &HgPath) -> Result<Option<Node>, HgError> {
+        // TODO: use binary search instead of linear scan. This may involve
+        // building (and caching) an index of the byte indicex of each manifest
+        // line.
+        for (manifest_path, node) in self.files_with_nodes() {
+            if manifest_path == path {
+                return Ok(Some(Node::from_hex_for_repo(node)?));
+            }
+        }
+        Ok(None)
+    }
 }

rust/rhg/src/commands/status.rs

@@ -1,305 +1,304 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::dispatch::DirstateMapMethods;
-use hg::errors::IoResultExt;
+use hg::errors::{HgError, IoResultExt};
+use hg::manifest::Manifest;
 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 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) = 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 = repo.dirstate_parents()?.p1.into();
-        let p1_hex = format!("{:x}", p1);
+        let p1 = repo.dirstate_parents()?.p1;
+        let manifest = repo.manifest_for_node(p1).map_err(|e| {
+            CommandError::from((e, &*format!("{:x}", p1.short())))
+        })?;
         for to_check in ds_status.unsure {
-            if cat_file_is_modified(repo, &to_check, &p1_hex)? {
+            if cat_file_is_modified(repo, &manifest, &to_check)? {
                 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,
+    manifest: &Manifest,
     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))?;
+) -> Result<bool, HgError> {
+    let file_node = manifest
+        .find_file(hg_path)?
+        .expect("ambgious file not in p1");
+    let filelog = repo.filelog(hg_path)?;
+    let filelog_entry = filelog.get_node(file_node).map_err(|_| {
+        HgError::corrupted("filelog missing node from manifest")
+    })?;
+    let contents_in_p1 = filelog_entry.data()?;
 
     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() {
+    let hg_data_len: u64 = match contents_in_p1.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
-    {
+    if fobj.metadata().when_reading_file(&fs_path)?.len() != hg_data_len {
         return Ok(true);
     }
-    for (fs_byte, hg_byte) in
-        BufReader::new(fobj).bytes().zip(output.concatenated)
+    for (fs_byte, &hg_byte) in BufReader::new(fobj).bytes().zip(contents_in_p1)
     {
-        if fs_byte.map_err(|e| StatusError::from(e))? != hg_byte {
+        if fs_byte.when_reading_file(&fs_path)? != hg_byte {
             return Ok(true);
         }
     }
     Ok(false)
 }