rhg: Propagate permission errors when finding a repository

The Rust standard library has a `Path::is_dir` method that returns false
for any I/O error (such as a permission error),
not just "No such file or directory".

Instead add an `is_dir` function that returns false for non-directories
and for "No such file or directory" errors, but propagates other I/O errors.

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

rust/hg-core/src/errors.rs

@@ -1,194 +1,199 b''
 use crate::config::ConfigValueParseError;
 use crate::exit_codes;
 use std::fmt;
 
 /// Common error cases that can happen in many different APIs
 #[derive(Debug, derive_more::From)]
 pub enum HgError {
     IoError {
         error: std::io::Error,
         context: IoErrorContext,
     },
 
     /// A file under `.hg/` normally only written by Mercurial is not in the
     /// expected format. This indicates a bug in Mercurial, filesystem
     /// corruption, or hardware failure.
     ///
     /// The given string is a short explanation for users, not intended to be
     /// machine-readable.
     CorruptedRepository(String),
 
     /// The respository or requested operation involves a feature not
     /// supported by the Rust implementation. Falling back to the Python
     /// implementation may or may not work.
     ///
     /// The given string is a short explanation for users, not intended to be
     /// machine-readable.
     UnsupportedFeature(String),
 
     /// Operation cannot proceed for some other reason.
     ///
     /// The message is a short explanation for users, not intended to be
     /// machine-readable.
     Abort {
         message: String,
         detailed_exit_code: exit_codes::ExitCode,
     },
 
     /// A configuration value is not in the expected syntax.
     ///
     /// These errors can happen in many places in the code because values are
     /// parsed lazily as the file-level parser does not know the expected type
     /// and syntax of each value.
     #[from]
     ConfigValueParseError(ConfigValueParseError),
 }
 
 /// Details about where an I/O error happened
 #[derive(Debug)]
 pub enum IoErrorContext {
+    /// `std::fs::metadata`
+    ReadingMetadata(std::path::PathBuf),
     ReadingFile(std::path::PathBuf),
     WritingFile(std::path::PathBuf),
     RemovingFile(std::path::PathBuf),
     RenamingFile {
         from: std::path::PathBuf,
         to: std::path::PathBuf,
     },
     /// `std::fs::canonicalize`
     CanonicalizingPath(std::path::PathBuf),
     /// `std::env::current_dir`
     CurrentDir,
     /// `std::env::current_exe`
     CurrentExe,
 }
 
 impl HgError {
     pub fn corrupted(explanation: impl Into<String>) -> Self {
         // TODO: capture a backtrace here and keep it in the error value
         // to aid debugging?
         // https://doc.rust-lang.org/std/backtrace/struct.Backtrace.html
         HgError::CorruptedRepository(explanation.into())
     }
 
     pub fn unsupported(explanation: impl Into<String>) -> Self {
         HgError::UnsupportedFeature(explanation.into())
     }
 
     pub fn abort(
         explanation: impl Into<String>,
         exit_code: exit_codes::ExitCode,
     ) -> Self {
         HgError::Abort {
             message: explanation.into(),
             detailed_exit_code: exit_code,
         }
     }
 }
 
 // TODO: use `DisplayBytes` instead to show non-Unicode filenames losslessly?
 impl fmt::Display for HgError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
             HgError::Abort { message, .. } => write!(f, "{}", message),
             HgError::IoError { error, context } => {
                 write!(f, "abort: {}: {}", context, error)
             }
             HgError::CorruptedRepository(explanation) => {
                 write!(f, "abort: {}", explanation)
             }
             HgError::UnsupportedFeature(explanation) => {
                 write!(f, "unsupported feature: {}", explanation)
             }
             HgError::ConfigValueParseError(error) => error.fmt(f),
         }
     }
 }
 
 // TODO: use `DisplayBytes` instead to show non-Unicode filenames losslessly?
 impl fmt::Display for IoErrorContext {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
+            IoErrorContext::ReadingMetadata(path) => {
+                write!(f, "when reading metadata of {}", path.display())
+            }
             IoErrorContext::ReadingFile(path) => {
                 write!(f, "when reading {}", path.display())
             }
             IoErrorContext::WritingFile(path) => {
                 write!(f, "when writing {}", path.display())
             }
             IoErrorContext::RemovingFile(path) => {
                 write!(f, "when removing {}", path.display())
             }
             IoErrorContext::RenamingFile { from, to } => write!(
                 f,
                 "when renaming {} to {}",
                 from.display(),
                 to.display()
             ),
             IoErrorContext::CanonicalizingPath(path) => {
                 write!(f, "when canonicalizing {}", path.display())
             }
             IoErrorContext::CurrentDir => {
                 write!(f, "error getting current working directory")
             }
             IoErrorContext::CurrentExe => {
                 write!(f, "error getting current executable")
             }
         }
     }
 }
 
 pub trait IoResultExt<T> {
     /// Annotate a possible I/O error as related to a reading a file at the
     /// given path.
     ///
     /// This allows printing something like “File not found when reading
     /// example.txt” instead of just “File not found”.
     ///
     /// Converts a `Result` with `std::io::Error` into one with `HgError`.
     fn when_reading_file(self, path: &std::path::Path) -> Result<T, HgError>;
 
     fn with_context(
         self,
         context: impl FnOnce() -> IoErrorContext,
     ) -> Result<T, HgError>;
 }
 
 impl<T> IoResultExt<T> for std::io::Result<T> {
     fn when_reading_file(self, path: &std::path::Path) -> Result<T, HgError> {
         self.with_context(|| IoErrorContext::ReadingFile(path.to_owned()))
     }
 
     fn with_context(
         self,
         context: impl FnOnce() -> IoErrorContext,
     ) -> Result<T, HgError> {
         self.map_err(|error| HgError::IoError {
             error,
             context: context(),
         })
     }
 }
 
 pub trait HgResultExt<T> {
     /// Handle missing files separately from other I/O error cases.
     ///
     /// Wraps the `Ok` type in an `Option`:
     ///
     /// * `Ok(x)` becomes `Ok(Some(x))`
     /// * An I/O "not found" error becomes `Ok(None)`
     /// * Other errors are unchanged
     fn io_not_found_as_none(self) -> Result<Option<T>, HgError>;
 }
 
 impl<T> HgResultExt<T> for Result<T, HgError> {
     fn io_not_found_as_none(self) -> Result<Option<T>, HgError> {
         match self {
             Ok(x) => Ok(Some(x)),
             Err(HgError::IoError { error, .. })
                 if error.kind() == std::io::ErrorKind::NotFound =>
             {
                 Ok(None)
             }
             Err(other_error) => Err(other_error),
         }
     }
 }

rust/hg-core/src/repo.rs

@@ -1,288 +1,315 b''
 use crate::config::{Config, ConfigError, ConfigParseError};
 use crate::errors::{HgError, IoErrorContext, IoResultExt};
 use crate::exit_codes;
 use crate::requirements;
 use crate::utils::files::get_path_from_bytes;
 use crate::utils::SliceExt;
 use memmap::{Mmap, MmapOptions};
 use std::collections::HashSet;
+use std::io::ErrorKind;
 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,
 }
 
 #[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(),
         }
     }
 }
 
 /// Filesystem access abstraction for the contents of a given "base" diretory
 #[derive(Clone, Copy)]
 pub struct Vfs<'a> {
     pub(crate) base: &'a Path,
 }
 
 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 ancestor.join(".hg").is_dir() {
+            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 root.join(".hg").is_dir() {
+            if is_dir(root.join(".hg"))? {
                 Self::new_at_path(root.to_owned(), config)
-            } else if root.is_file() {
+            } 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_newlines()).to_owned();
             if relative {
                 shared_path = dot_hg.join(shared_path)
             }
-            if !shared_path.is_dir() {
+            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,
         };
 
         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);
         }
         let parents = if self.has_dirstate_v2() {
             crate::dirstate_tree::on_disk::read_docket(&dirstate)?.parents()
         } else {
             crate::dirstate::parsers::parse_dirstate_parents(&dirstate)?
                 .clone()
         };
         Ok(parents)
     }
 }
 
 impl Vfs<'_> {
     pub fn join(&self, relative_path: impl AsRef<Path>) -> PathBuf {
         self.base.join(relative_path)
     }
 
     pub fn read(
         &self,
         relative_path: impl AsRef<Path>,
     ) -> Result<Vec<u8>, HgError> {
         let path = self.join(relative_path);
         std::fs::read(&path).when_reading_file(&path)
     }
 
     pub fn mmap_open(
         &self,
         relative_path: impl AsRef<Path>,
     ) -> Result<Mmap, HgError> {
         let path = self.base.join(relative_path);
         let file = std::fs::File::open(&path).when_reading_file(&path)?;
         // TODO: what are the safety requirements here?
         let mmap = unsafe { MmapOptions::new().map(&file) }
             .when_reading_file(&path)?;
         Ok(mmap)
     }
 
     pub fn rename(
         &self,
         relative_from: impl AsRef<Path>,
         relative_to: impl AsRef<Path>,
     ) -> Result<(), HgError> {
         let from = self.join(relative_from);
         let to = self.join(relative_to);
         std::fs::rename(&from, &to)
             .with_context(|| IoErrorContext::RenamingFile { from, to })
     }
 }
+
+fn fs_metadata(
+    path: impl AsRef<Path>,
+) -> Result<Option<std::fs::Metadata>, HgError> {
+    let path = path.as_ref();
+    match std::fs::metadata(path) {
+        Ok(meta) => Ok(Some(meta)),
+        Err(error) => match error.kind() {
+            // TODO: when we require a Rust version where `NotADirectory` is
+            // stable, invert this logic and return None for it and `NotFound`
+            // and propagate any other error.
+            ErrorKind::PermissionDenied => Err(error).with_context(|| {
+                IoErrorContext::ReadingMetadata(path.to_owned())
+            }),
+            _ => Ok(None),
+        },
+    }
+}
+
+fn is_dir(path: impl AsRef<Path>) -> Result<bool, HgError> {
+    Ok(fs_metadata(path)?.map_or(false, |meta| meta.is_dir()))
+}
+
+fn is_file(path: impl AsRef<Path>) -> Result<bool, HgError> {
+    Ok(fs_metadata(path)?.map_or(false, |meta| meta.is_file()))
+}