# HG changeset patch
# User Simon Sapin <simon.sapin@octobus.net>
# Date 2021-11-23 17:27:42
# Node ID 10c32e1b892a79cf4beb23e7e94edd439bd6cdfa
# Parent  51f26c8088b2e9bb5e45a9065825587f9293f5b4

rhg: Propogate manifest parse errors instead of panicking

The Rust parser for the manifest file format is an iterator. Now every item
from that iterator is a `Result`, which makes error handling required
in multiple new places.

This makes better recovery on errors possible, compare to a run time panic.

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

diff --git a/rust/hg-core/src/operations/cat.rs b/rust/hg-core/src/operations/cat.rs
--- a/rust/hg-core/src/operations/cat.rs
+++ b/rust/hg-core/src/operations/cat.rs
@@ -11,6 +11,7 @@ use crate::revlog::Node;
 
 use crate::utils::hg_path::HgPath;
 
+use crate::errors::HgError;
 use itertools::put_back;
 use itertools::PutBack;
 use std::cmp::Ordering;
@@ -28,21 +29,24 @@ pub struct CatOutput<'a> {
 }
 
 // Find an item in an iterator over a sorted collection.
-fn find_item<'a, 'b, 'c, D, I: Iterator<Item = (&'a HgPath, D)>>(
+fn find_item<'a, D, I: Iterator<Item = Result<(&'a HgPath, D), HgError>>>(
     i: &mut PutBack<I>,
-    needle: &'b HgPath,
-) -> Option<D> {
+    needle: &HgPath,
+) -> Result<Option<D>, HgError> {
     loop {
         match i.next() {
-            None => return None,
-            Some(val) => match needle.as_bytes().cmp(val.0.as_bytes()) {
-                Ordering::Less => {
-                    i.put_back(val);
-                    return None;
+            None => return Ok(None),
+            Some(result) => {
+                let (path, value) = result?;
+                match needle.as_bytes().cmp(path.as_bytes()) {
+                    Ordering::Less => {
+                        i.put_back(Ok((path, value)));
+                        return Ok(None);
+                    }
+                    Ordering::Greater => continue,
+                    Ordering::Equal => return Ok(Some(value)),
                 }
-                Ordering::Greater => continue,
-                Ordering::Equal => return Some(val.1),
-            },
+            }
         }
     }
 }
@@ -51,23 +55,23 @@ fn find_files_in_manifest<
     'manifest,
     'query,
     Data,
-    Manifest: Iterator<Item = (&'manifest HgPath, Data)>,
+    Manifest: Iterator<Item = Result<(&'manifest HgPath, Data), HgError>>,
     Query: Iterator<Item = &'query HgPath>,
 >(
     manifest: Manifest,
     query: Query,
-) -> (Vec<(&'query HgPath, Data)>, Vec<&'query HgPath>) {
+) -> Result<(Vec<(&'query HgPath, Data)>, Vec<&'query HgPath>), HgError> {
     let mut manifest = put_back(manifest);
     let mut res = vec![];
     let mut missing = vec![];
 
     for file in query {
-        match find_item(&mut manifest, file) {
+        match find_item(&mut manifest, file)? {
             None => missing.push(file),
             Some(item) => res.push((file, item)),
         }
     }
-    return (res, missing);
+    return Ok((res, missing));
 }
 
 /// Output the given revision of files
@@ -94,7 +98,7 @@ pub fn cat<'a>(
     let (found, missing) = find_files_in_manifest(
         manifest.files_with_nodes(),
         files.into_iter().map(|f| f.as_ref()),
-    );
+    )?;
 
     for (file_path, node_bytes) in found {
         found_any = true;
diff --git a/rust/hg-core/src/operations/list_tracked_files.rs b/rust/hg-core/src/operations/list_tracked_files.rs
--- a/rust/hg-core/src/operations/list_tracked_files.rs
+++ b/rust/hg-core/src/operations/list_tracked_files.rs
@@ -76,7 +76,7 @@ pub fn list_rev_tracked_files(
 pub struct FilesForRev(Manifest);
 
 impl FilesForRev {
-    pub fn iter(&self) -> impl Iterator<Item = &HgPath> {
+    pub fn iter(&self) -> impl Iterator<Item = Result<&HgPath, HgError>> {
         self.0.files()
     }
 }
diff --git a/rust/hg-core/src/revlog/manifest.rs b/rust/hg-core/src/revlog/manifest.rs
--- a/rust/hg-core/src/revlog/manifest.rs
+++ b/rust/hg-core/src/revlog/manifest.rs
@@ -63,26 +63,28 @@ impl Manifest {
     }
 
     /// Return an iterator over the files of the entry.
-    pub fn files(&self) -> impl Iterator<Item = &HgPath> {
+    pub fn files(&self) -> impl Iterator<Item = Result<&HgPath, HgError>> {
         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])
+            let pos =
+                line.iter().position(|x| x == &b'\0').ok_or_else(|| {
+                    HgError::corrupted("manifest line should contain \\0")
+                })?;
+            Ok(HgPath::new(&line[..pos]))
         })
     }
 
     /// Return an iterator over the files of the entry.
-    pub fn files_with_nodes(&self) -> impl Iterator<Item = (&HgPath, &[u8])> {
+    pub fn files_with_nodes(
+        &self,
+    ) -> impl Iterator<Item = Result<(&HgPath, &[u8]), HgError>> {
         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 pos =
+                line.iter().position(|x| x == &b'\0').ok_or_else(|| {
+                    HgError::corrupted("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])
+            Ok((HgPath::new(&line[..pos]), &line[hash_start..hash_end]))
         })
     }
 
@@ -91,7 +93,8 @@ impl Manifest {
         // 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() {
+        for entry in self.files_with_nodes() {
+            let (manifest_path, node) = entry?;
             if manifest_path == path {
                 return Ok(Some(Node::from_hex_for_repo(node)?));
             }
diff --git a/rust/rhg/src/commands/files.rs b/rust/rhg/src/commands/files.rs
--- a/rust/rhg/src/commands/files.rs
+++ b/rust/rhg/src/commands/files.rs
@@ -3,6 +3,7 @@ use crate::ui::Ui;
 use crate::ui::UiError;
 use crate::utils::path_utils::relativize_paths;
 use clap::Arg;
+use hg::errors::HgError;
 use hg::operations::list_rev_tracked_files;
 use hg::operations::Dirstate;
 use hg::repo::Repo;
@@ -45,14 +46,14 @@ pub fn run(invocation: &crate::CliInvoca
     } else {
         let distate = Dirstate::new(repo)?;
         let files = distate.tracked_files()?;
-        display_files(invocation.ui, repo, files)
+        display_files(invocation.ui, repo, files.into_iter().map(Ok))
     }
 }
 
 fn display_files<'a>(
     ui: &Ui,
     repo: &Repo,
-    files: impl IntoIterator<Item = &'a HgPath>,
+    files: impl IntoIterator<Item = Result<&'a HgPath, HgError>>,
 ) -> Result<(), CommandError> {
     let mut stdout = ui.stdout_buffer();
     let mut any = false;
diff --git a/rust/rhg/src/commands/status.rs b/rust/rhg/src/commands/status.rs
--- a/rust/rhg/src/commands/status.rs
+++ b/rust/rhg/src/commands/status.rs
@@ -279,7 +279,7 @@ fn display_status_paths(
     if relative && !ui.plain() {
         relativize_paths(
             repo,
-            paths,
+            paths.iter().map(Ok),
             |path: Cow<[u8]>| -> Result<(), UiError> {
                 ui.write_stdout(
                     &[status_prefix, b" ", path.as_ref(), b"\n"].concat(),
diff --git a/rust/rhg/src/utils/path_utils.rs b/rust/rhg/src/utils/path_utils.rs
--- a/rust/rhg/src/utils/path_utils.rs
+++ b/rust/rhg/src/utils/path_utils.rs
@@ -5,6 +5,7 @@
 
 use crate::error::CommandError;
 use crate::ui::UiError;
+use hg::errors::HgError;
 use hg::repo::Repo;
 use hg::utils::current_dir;
 use hg::utils::files::{get_bytes_from_path, relativize_path};
@@ -14,7 +15,7 @@ use std::borrow::Cow;
 
 pub fn relativize_paths(
     repo: &Repo,
-    paths: impl IntoIterator<Item = impl AsRef<HgPath>>,
+    paths: impl IntoIterator<Item = Result<impl AsRef<HgPath>, HgError>>,
     mut callback: impl FnMut(Cow<[u8]>) -> Result<(), UiError>,
 ) -> Result<(), CommandError> {
     let cwd = current_dir()?;
@@ -38,10 +39,10 @@ pub fn relativize_paths(
 
     for file in paths {
         if outside_repo {
-            let file = repo_root_hgpath.join(file.as_ref());
+            let file = repo_root_hgpath.join(file?.as_ref());
             callback(relativize_path(&file, &cwd_hgpath))?;
         } else {
-            callback(relativize_path(file.as_ref(), &cwd_hgpath))?;
+            callback(relativize_path(file?.as_ref(), &cwd_hgpath))?;
         }
     }
     Ok(())