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

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

@@ -1,115 +1,119 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::HgPath;
 
+use crate::errors::HgError;
 use itertools::put_back;
 use itertools::PutBack;
 use std::cmp::Ordering;
 
 pub struct CatOutput<'a> {
     /// 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 results: Vec<(&'a HgPath, Vec<u8>)>,
     /// Which of the CLI arguments did not match any manifest file
     pub missing: Vec<&'a HgPath>,
     /// The node ID that the given revset was resolved to
     pub node: Node,
 }
 
 // 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()) {
+            None => return Ok(None),
+            Some(result) => {
+                let (path, value) = result?;
+                match needle.as_bytes().cmp(path.as_bytes()) {
                 Ordering::Less => {
-                    i.put_back(val);
-                    return None;
+                        i.put_back(Ok((path, value)));
+                        return Ok(None);
                 }
                 Ordering::Greater => continue,
-                Ordering::Equal => return Some(val.1),
-            },
+                    Ordering::Equal => return Ok(Some(value)),
+                }
+            }
         }
     }
 }
 
 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
 ///
 /// * `root`: Repository root
 /// * `rev`: The revision to cat the files from.
 /// * `files`: The files to output.
 pub fn cat<'a>(
     repo: &Repo,
     revset: &str,
     mut files: Vec<&'a HgPath>,
 ) -> Result<CatOutput<'a>, RevlogError> {
     let rev = crate::revset::resolve_single(revset, repo)?;
     let manifest = repo.manifest_for_rev(rev)?;
     let node = *repo
         .changelog()?
         .node_from_rev(rev)
         .expect("should succeed when repo.manifest did");
     let mut results: Vec<(&'a HgPath, Vec<u8>)> = vec![];
     let mut found_any = false;
 
     files.sort_unstable();
 
     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;
         let file_log = repo.filelog(file_path)?;
         let file_node = Node::from_hex_for_repo(node_bytes)?;
         results.push((
             file_path,
             file_log.data_for_node(file_node)?.into_data()?,
         ));
     }
 
     Ok(CatOutput {
         found_any,
         results,
         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_for_rev(rev)?))
 }
 
 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()
     }
 }

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

@@ -1,101 +1,104 b''
 use crate::errors::HgError;
 use crate::repo::Repo;
 use crate::revlog::revlog::{Revlog, RevlogError};
 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 `Manifest` for the given node ID.
     ///
     /// Note: this is a node ID in the manifestlog, typically found through
     /// `ChangelogEntry::manifest_node`. It is *not* the node ID of any
     /// changeset.
     ///
     /// See also `Repo::manifest_for_node`
     pub fn data_for_node(
         &self,
         node: NodePrefix,
     ) -> Result<Manifest, RevlogError> {
         let rev = self.revlog.rev_from_node(node)?;
         self.data_for_rev(rev)
     }
 
     /// Return the `Manifest` of a given revision number.
     ///
     /// Note: this is a revision number in the manifestlog, *not* of any
     /// changeset.
     ///
     /// See also `Repo::manifest_for_rev`
     pub fn data_for_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> {
+    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]))
         })
     }
 
     /// 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() {
+        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)?));
             }
         }
         Ok(None)
     }
 }

rust/rhg/src/commands/files.rs

@@ -1,71 +1,72 b''
 use crate::error::CommandError;
 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;
 use hg::utils::hg_path::HgPath;
 use std::borrow::Cow;
 
 pub const HELP_TEXT: &str = "
 List tracked files.
 
 Returns 0 on success.
 ";
 
 pub fn args() -> clap::App<'static, 'static> {
     clap::SubCommand::with_name("files")
         .arg(
             Arg::with_name("rev")
                 .help("search the repository as it is in REV")
                 .short("-r")
                 .long("--revision")
                 .value_name("REV")
                 .takes_value(true),
         )
         .about(HELP_TEXT)
 }
 
 pub fn run(invocation: &crate::CliInvocation) -> Result<(), CommandError> {
     let relative = invocation.config.get(b"ui", b"relative-paths");
     if relative.is_some() {
         return Err(CommandError::unsupported(
             "non-default ui.relative-paths",
         ));
     }
 
     let rev = invocation.subcommand_args.value_of("rev");
 
     let repo = invocation.repo?;
     if let Some(rev) = rev {
         let files = list_rev_tracked_files(repo, rev).map_err(|e| (e, rev))?;
         display_files(invocation.ui, repo, files.iter())
     } 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;
 
     relativize_paths(repo, files, |path: Cow<[u8]>| -> Result<(), UiError> {
         any = true;
         stdout.write_all(path.as_ref())?;
         stdout.write_all(b"\n")
     })?;
     stdout.flush()?;
     if any {
         Ok(())
     } else {
         Err(CommandError::Unsuccessful)
     }
 }

rust/rhg/src/commands/status.rs

@@ -1,324 +1,324 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, UiError};
 use crate::utils::path_utils::relativize_paths;
 use clap::{Arg, SubCommand};
 use hg;
 use hg::config::Config;
 use hg::dirstate::TruncatedTimestamp;
 use hg::errors::HgError;
 use hg::manifest::Manifest;
 use hg::matchers::AlwaysMatcher;
 use hg::repo::Repo;
 use hg::utils::hg_path::{hg_path_to_os_string, HgPath};
 use hg::{HgPathCow, StatusOptions};
 use log::{info, warn};
 use std::borrow::Cow;
 
 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')"
         ));
     }
 
     // TODO: lift these limitations
     if invocation.config.get_bool(b"ui", b"tweakdefaults")? {
         return Err(CommandError::unsupported(
             "ui.tweakdefaults is not yet supported with rhg status",
         ));
     }
     if invocation.config.get_bool(b"ui", b"statuscopies")? {
         return Err(CommandError::unsupported(
             "ui.statuscopies is not yet supported with rhg status",
         ));
     }
     if invocation
         .config
         .get(b"commands", b"status.terse")
         .is_some()
     {
         return Err(CommandError::unsupported(
             "status.terse is not yet supported with rhg status",
         ));
     }
 
     let ui = invocation.ui;
     let config = invocation.config;
     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: TruncatedTimestamp::new_truncate(0, 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 = 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, &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, repo, config, &mut ds_status.modified, b"M")?;
     }
     if display_states.added {
         display_status_paths(ui, repo, config, &mut ds_status.added, b"A")?;
     }
     if display_states.removed {
         display_status_paths(ui, repo, config, &mut ds_status.removed, b"R")?;
     }
     if display_states.deleted {
         display_status_paths(ui, repo, config, &mut ds_status.deleted, b"!")?;
     }
     if display_states.unknown {
         display_status_paths(ui, repo, config, &mut ds_status.unknown, b"?")?;
     }
     if display_states.ignored {
         display_status_paths(ui, repo, config, &mut ds_status.ignored, b"I")?;
     }
     if display_states.clean {
         display_status_paths(ui, repo, config, &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,
     repo: &Repo,
     config: &Config,
     paths: &mut [HgPathCow],
     status_prefix: &[u8],
 ) -> Result<(), CommandError> {
     paths.sort_unstable();
     let mut relative: bool = config.get_bool(b"ui", b"relative-paths")?;
     relative = config
         .get_option(b"commands", b"status.relative")?
         .unwrap_or(relative);
     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(),
                 )
             },
         )?;
     } else {
         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,
 ) -> 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.data_for_node(file_node).map_err(|_| {
         HgError::corrupted("filelog missing node from manifest")
     })?;
     let contents_in_p1 = filelog_entry.data()?;
 
     let fs_path = hg_path_to_os_string(hg_path).expect("HgPath conversion");
     let fs_contents = repo.working_directory_vfs().read(fs_path)?;
     return Ok(contents_in_p1 != &*fs_contents);
 }

rust/rhg/src/utils/path_utils.rs

@@ -1,48 +1,49 b''
 // path utils module
 //
 // 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::UiError;
+use hg::errors::HgError;
 use hg::repo::Repo;
 use hg::utils::current_dir;
 use hg::utils::files::{get_bytes_from_path, relativize_path};
 use hg::utils::hg_path::HgPath;
 use hg::utils::hg_path::HgPathBuf;
 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()?;
     let repo_root = repo.working_directory_path();
     let repo_root = cwd.join(repo_root); // Make it absolute
     let repo_root_hgpath =
         HgPathBuf::from(get_bytes_from_path(repo_root.to_owned()));
     let outside_repo: bool;
     let cwd_hgpath: HgPathBuf;
 
     if let Ok(cwd_relative_to_repo) = cwd.strip_prefix(&repo_root) {
         // The current directory is inside the repo, so we can work with
         // relative paths
         outside_repo = false;
         cwd_hgpath =
             HgPathBuf::from(get_bytes_from_path(cwd_relative_to_repo));
     } else {
         outside_repo = true;
         cwd_hgpath = HgPathBuf::from(get_bytes_from_path(cwd));
     }
 
     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(())
 }