Patchwork D9896: rust: replace RequirementsError with HgError

login
register
mail settings
Submitter phabricator
Date Jan. 27, 2021, 9:14 p.m.
Message ID <differential-rev-PHID-DREV-kl4sa2vou3l425owiwhx-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48206/
State Superseded
Headers show

Comments

phabricator - Jan. 27, 2021, 9:14 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  rust/hg-core/src/repo.rs
  rust/hg-core/src/requirements.rs
  rust/rhg/src/error.rs
  tests/test-rhg.t

CHANGE DETAILS




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

Patch

diff --git a/tests/test-rhg.t b/tests/test-rhg.t
--- a/tests/test-rhg.t
+++ b/tests/test-rhg.t
@@ -163,7 +163,7 @@ 
 
   $ echo -e '\xFF' >> .hg/requires
   $ rhg debugrequirements
-  abort: .hg/requires is corrupted
+  corrupted repository: parse error in 'requires' file
   [255]
 
 Persistent nodemap
diff --git a/rust/rhg/src/error.rs b/rust/rhg/src/error.rs
--- a/rust/rhg/src/error.rs
+++ b/rust/rhg/src/error.rs
@@ -4,7 +4,6 @@ 
 use format_bytes::format_bytes;
 use hg::errors::HgError;
 use hg::operations::FindRootError;
-use hg::requirements::RequirementsError;
 use hg::revlog::revlog::RevlogError;
 use hg::utils::files::get_bytes_from_path;
 use std::convert::From;
@@ -17,9 +16,6 @@ 
     RootNotFound(PathBuf),
     /// The current directory cannot be found
     CurrentDirNotFound(std::io::Error),
-    /// `.hg/requires`
-    #[from]
-    RequirementsError(RequirementsError),
     /// The standard output stream cannot be written to
     StdoutError,
     /// The standard error stream cannot be written to
@@ -38,10 +34,6 @@ 
         match self {
             CommandError::RootNotFound(_) => exitcode::ABORT,
             CommandError::CurrentDirNotFound(_) => exitcode::ABORT,
-            CommandError::RequirementsError(
-                RequirementsError::Unsupported { .. },
-            ) => exitcode::UNIMPLEMENTED_COMMAND,
-            CommandError::RequirementsError(_) => exitcode::ABORT,
             CommandError::StdoutError => exitcode::ABORT,
             CommandError::StderrError => exitcode::ABORT,
             CommandError::Abort(_) => exitcode::ABORT,
@@ -67,15 +59,9 @@ 
                 b"abort: error getting current working directory: {}\n",
                 e.to_string().as_bytes(),
             )),
-            CommandError::RequirementsError(RequirementsError::Corrupted) => {
-                Some(
-                    "abort: .hg/requires is corrupted\n".as_bytes().to_owned(),
-                )
-            }
             CommandError::Abort(message) => message.to_owned(),
 
-            CommandError::RequirementsError(_)
-            | CommandError::StdoutError
+            CommandError::StdoutError
             | CommandError::StderrError
             | CommandError::Unimplemented
             | CommandError::Other(HgError::UnsupportedFeature(_)) => None,
diff --git a/rust/hg-core/src/requirements.rs b/rust/hg-core/src/requirements.rs
--- a/rust/hg-core/src/requirements.rs
+++ b/rust/hg-core/src/requirements.rs
@@ -1,19 +1,7 @@ 
+use crate::errors::{HgError, HgResultExt, IoResultExt};
 use crate::repo::Repo;
-use std::io;
 
-#[derive(Debug)]
-pub enum RequirementsError {
-    // TODO: include a path?
-    Io(io::Error),
-    /// The `requires` file is corrupted
-    Corrupted,
-    /// The repository requires a feature that we don't support
-    Unsupported {
-        feature: String,
-    },
-}
-
-fn parse(bytes: &[u8]) -> Result<Vec<String>, ()> {
+fn parse(bytes: &[u8]) -> Result<Vec<String>, HgError> {
     // The Python code reading this file uses `str.splitlines`
     // which looks for a number of line separators (even including a couple of
     // non-ASCII ones), but Python code writing it always uses `\n`.
@@ -27,16 +15,21 @@ 
             if line[0].is_ascii_alphanumeric() && line.is_ascii() {
                 Ok(String::from_utf8(line.into()).unwrap())
             } else {
-                Err(())
+                Err(HgError::corrupted("parse error in 'requires' file"))
             }
         })
         .collect()
 }
 
-pub fn load(repo: &Repo) -> Result<Vec<String>, RequirementsError> {
-    match repo.hg_vfs().read("requires") {
-        Ok(bytes) => parse(&bytes).map_err(|()| RequirementsError::Corrupted),
-
+pub fn load(repo: &Repo) -> Result<Vec<String>, HgError> {
+    if let Some(bytes) = repo
+        .hg_vfs()
+        .read("requires")
+        .for_file("requires".as_ref())
+        .io_not_found_as_none()?
+    {
+        parse(&bytes)
+    } else {
         // Treat a missing file the same as an empty file.
         // From `mercurial/localrepo.py`:
         // > requires file contains a newline-delimited list of
@@ -44,18 +37,19 @@ 
         // > the repository. This file was introduced in Mercurial 0.9.2,
         // > which means very old repositories may not have one. We assume
         // > a missing file translates to no requirements.
-        Err(error) if error.kind() == std::io::ErrorKind::NotFound => {
-            Ok(Vec::new())
-        }
-
-        Err(error) => Err(RequirementsError::Io(error))?,
+        Ok(Vec::new())
     }
 }
 
-pub fn check(repo: &Repo) -> Result<(), RequirementsError> {
+pub fn check(repo: &Repo) -> Result<(), HgError> {
     for feature in load(repo)? {
         if !SUPPORTED.contains(&&*feature) {
-            return Err(RequirementsError::Unsupported { feature });
+            // TODO: collect and all unknown features and include them in the
+            // error message?
+            return Err(HgError::UnsupportedFeature(format!(
+                "repository requires feature unknown to this Mercurial: {}",
+                feature
+            )));
         }
     }
     Ok(())
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,3 +1,4 @@ 
+use crate::errors::HgError;
 use crate::operations::{find_root, FindRootError};
 use crate::requirements;
 use memmap::{Mmap, MmapOptions};
@@ -33,9 +34,7 @@ 
         find_root().map(Self::for_path)
     }
 
-    pub fn check_requirements(
-        &self,
-    ) -> Result<(), requirements::RequirementsError> {
+    pub fn check_requirements(&self) -> Result<(), HgError> {
         requirements::check(self)
     }