changeset 47780:cf5f8da2244c stable

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
author Simon Sapin <simon.sapin@octobus.net>
date Thu, 29 Jul 2021 12:22:25 +0200
parents 6df528ed47a9
children 4870a8dc24d9
files rust/hg-core/src/errors.rs rust/hg-core/src/repo.rs
diffstat 2 files changed, 36 insertions(+), 4 deletions(-) [+]
line wrap: on
line diff
--- a/rust/hg-core/src/errors.rs	Thu Jul 29 11:53:03 2021 +0200
+++ b/rust/hg-core/src/errors.rs	Thu Jul 29 12:22:25 2021 +0200
@@ -47,6 +47,8 @@
 /// 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),
@@ -108,6 +110,9 @@
 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())
             }
--- a/rust/hg-core/src/repo.rs	Thu Jul 29 11:53:03 2021 +0200
+++ b/rust/hg-core/src/repo.rs	Thu Jul 29 12:22:25 2021 +0200
@@ -6,6 +6,7 @@
 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
@@ -51,7 +52,7 @@
         // 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());
             }
         }
@@ -73,9 +74,9 @@
         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 {
@@ -130,7 +131,7 @@
             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()
@@ -286,3 +287,29 @@
             .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()))
+}