Patchwork D11230: rhg: Propagate permission errors when finding a repository

login
register
mail settings
Submitter phabricator
Date July 29, 2021, 10:23 a.m.
Message ID <differential-rev-PHID-DREV-tydws43zjhnfgc6a6q5x-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/49544/
State Superseded
Headers show

Comments

phabricator - July 29, 2021, 10:23 a.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  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.

REPOSITORY
  rHG Mercurial

BRANCH
  stable

REVISION DETAIL
  https://phab.mercurial-scm.org/D11230

AFFECTED FILES
  rust/hg-core/src/errors.rs
  rust/hg-core/src/repo.rs

CHANGE DETAILS




To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/rust/hg-core/src/repo.rs b/rust/hg-core/src/repo.rs
--- a/rust/hg-core/src/repo.rs
+++ b/rust/hg-core/src/repo.rs
@@ -1,5 +1,5 @@ 
 use crate::config::{Config, ConfigError, ConfigParseError};
-use crate::errors::{HgError, IoErrorContext, IoResultExt};
+use crate::errors::{HgError, HgResultExt, IoErrorContext, IoResultExt};
 use crate::exit_codes;
 use crate::requirements;
 use crate::utils::files::get_path_from_bytes;
@@ -51,7 +51,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 +73,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 +130,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 +286,20 @@ 
             .with_context(|| IoErrorContext::RenamingFile { from, to })
     }
 }
+
+fn fs_metadata(
+    path: impl AsRef<Path>,
+) -> Result<Option<std::fs::Metadata>, HgError> {
+    let path = path.as_ref();
+    std::fs::metadata(path)
+        .with_context(|| IoErrorContext::ReadingMetadata(path.to_owned()))
+        .io_not_found_as_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()))
+}
diff --git a/rust/hg-core/src/errors.rs b/rust/hg-core/src/errors.rs
--- a/rust/hg-core/src/errors.rs
+++ b/rust/hg-core/src/errors.rs
@@ -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())
             }