Patchwork D9894: rust: Remove DirstateParseError and ListDirstateTrackedFilesError

login
register
mail settings
Submitter phabricator
Date Jan. 27, 2021, 9:14 p.m.
Message ID <differential-rev-PHID-DREV-oqpz6edkcz37dbwdknu2-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48207/
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.

REVISION SUMMARY
  Use HgError instead.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  rust/hg-core/src/dirstate.rs
  rust/hg-core/src/dirstate/dirstate_map.rs
  rust/hg-core/src/dirstate/parsers.rs
  rust/hg-core/src/lib.rs
  rust/hg-core/src/operations/list_tracked_files.rs
  rust/hg-core/src/operations/mod.rs
  rust/hg-cpython/src/dirstate.rs
  rust/hg-cpython/src/dirstate/dirs_multiset.rs
  rust/hg-cpython/src/dirstate/dirstate_map.rs
  rust/hg-cpython/src/parsers.rs
  rust/rhg/src/error.rs

CHANGE DETAILS




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

Patch

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
@@ -2,7 +2,8 @@ 
 use crate::ui::utf8_to_local;
 use crate::ui::UiError;
 use format_bytes::format_bytes;
-use hg::operations::{FindRootError, ListDirstateTrackedFilesError};
+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;
@@ -27,6 +28,9 @@ 
     Abort(Option<Vec<u8>>),
     /// A mercurial capability as not been implemented.
     Unimplemented,
+    /// Common cases
+    #[from]
+    Other(HgError),
 }
 
 impl CommandError {
@@ -42,6 +46,10 @@ 
             CommandError::StderrError => exitcode::ABORT,
             CommandError::Abort(_) => exitcode::ABORT,
             CommandError::Unimplemented => exitcode::UNIMPLEMENTED_COMMAND,
+            CommandError::Other(HgError::UnsupportedFeature(_)) => {
+                exitcode::UNIMPLEMENTED_COMMAND
+            }
+            CommandError::Other(_) => exitcode::ABORT,
         }
     }
 
@@ -141,21 +149,3 @@ 
         }
     }
 }
-
-impl From<ListDirstateTrackedFilesError> for CommandError {
-    fn from(err: ListDirstateTrackedFilesError) -> Self {
-        match err {
-            ListDirstateTrackedFilesError::IoError(err) => {
-                CommandError::Abort(Some(
-                    utf8_to_local(&format!("abort: {}\n", err)).into(),
-                ))
-            }
-            ListDirstateTrackedFilesError::ParseError(_) => {
-                CommandError::Abort(Some(
-                    // TODO find a better error message
-                    b"abort: parse error\n".to_vec(),
-                ))
-            }
-        }
-    }
-}
diff --git a/rust/hg-cpython/src/parsers.rs b/rust/hg-cpython/src/parsers.rs
--- a/rust/hg-cpython/src/parsers.rs
+++ b/rust/hg-cpython/src/parsers.rs
@@ -15,7 +15,7 @@ 
 };
 use hg::{
     pack_dirstate, parse_dirstate, utils::hg_path::HgPathBuf, DirstateEntry,
-    DirstateParents, DirstateParseError, FastHashMap, PARENT_SIZE,
+    DirstateParents, FastHashMap, PARENT_SIZE,
 };
 use std::convert::TryInto;
 
@@ -58,21 +58,7 @@ 
                     .to_py_object(py),
             )
         }
-        Err(e) => Err(PyErr::new::<exc::ValueError, _>(
-            py,
-            match e {
-                DirstateParseError::TooLittleData => {
-                    "too little data for parents".to_string()
-                }
-                DirstateParseError::Overflow => {
-                    "overflow in dirstate".to_string()
-                }
-                DirstateParseError::CorruptedEntry(e) => e,
-                DirstateParseError::Damaged => {
-                    "dirstate appears to be damaged".to_string()
-                }
-            },
-        )),
+        Err(e) => Err(PyErr::new::<exc::ValueError, _>(py, e.to_string())),
     }
 }
 
diff --git a/rust/hg-cpython/src/dirstate/dirstate_map.rs b/rust/hg-cpython/src/dirstate/dirstate_map.rs
--- a/rust/hg-cpython/src/dirstate/dirstate_map.rs
+++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs
@@ -26,10 +26,10 @@ 
     dirstate::{dirs_multiset::Dirs, make_dirstate_tuple},
 };
 use hg::{
+    errors::HgError,
     utils::hg_path::{HgPath, HgPathBuf},
     DirsMultiset, DirstateEntry, DirstateMap as RustDirstateMap,
-    DirstateMapError, DirstateParents, DirstateParseError, EntryState,
-    StateMapIter, PARENT_SIZE,
+    DirstateMapError, DirstateParents, EntryState, StateMapIter, PARENT_SIZE,
 };
 
 // TODO
@@ -84,13 +84,13 @@ 
             HgPath::new(f.extract::<PyBytes>(py)?.data(py)),
             oldstate.extract::<PyBytes>(py)?.data(py)[0]
                 .try_into()
-                .map_err(|e: DirstateParseError| {
+                .map_err(|e: HgError| {
                     PyErr::new::<exc::ValueError, _>(py, e.to_string())
                 })?,
             DirstateEntry {
                 state: state.extract::<PyBytes>(py)?.data(py)[0]
                     .try_into()
-                    .map_err(|e: DirstateParseError| {
+                    .map_err(|e: HgError| {
                         PyErr::new::<exc::ValueError, _>(py, e.to_string())
                     })?,
                 mode: mode.extract(py)?,
@@ -113,7 +113,7 @@ 
                 HgPath::new(f.extract::<PyBytes>(py)?.data(py)),
                 oldstate.extract::<PyBytes>(py)?.data(py)[0]
                     .try_into()
-                    .map_err(|e: DirstateParseError| {
+                    .map_err(|e: HgError| {
                         PyErr::new::<exc::ValueError, _>(py, e.to_string())
                     })?,
                 size.extract(py)?,
@@ -137,7 +137,7 @@ 
                 HgPath::new(f.extract::<PyBytes>(py)?.data(py)),
                 oldstate.extract::<PyBytes>(py)?.data(py)[0]
                     .try_into()
-                    .map_err(|e: DirstateParseError| {
+                    .map_err(|e: HgError| {
                         PyErr::new::<exc::ValueError, _>(py, e.to_string())
                     })?,
             )
diff --git a/rust/hg-cpython/src/dirstate/dirs_multiset.rs b/rust/hg-cpython/src/dirstate/dirs_multiset.rs
--- a/rust/hg-cpython/src/dirstate/dirs_multiset.rs
+++ b/rust/hg-cpython/src/dirstate/dirs_multiset.rs
@@ -18,9 +18,9 @@ 
 
 use crate::dirstate::extract_dirstate;
 use hg::{
+    errors::HgError,
     utils::hg_path::{HgPath, HgPathBuf},
-    DirsMultiset, DirsMultisetIter, DirstateMapError, DirstateParseError,
-    EntryState,
+    DirsMultiset, DirsMultisetIter, DirstateMapError, EntryState,
 };
 
 py_class!(pub class Dirs |py| {
@@ -38,7 +38,7 @@ 
             skip_state = Some(
                 skip.extract::<PyBytes>(py)?.data(py)[0]
                     .try_into()
-                    .map_err(|e: DirstateParseError| {
+                    .map_err(|e: HgError| {
                         PyErr::new::<exc::ValueError, _>(py, e.to_string())
                     })?,
             );
@@ -46,7 +46,7 @@ 
         let inner = if let Ok(map) = map.cast_as::<PyDict>(py) {
             let dirstate = extract_dirstate(py, &map)?;
             DirsMultiset::from_dirstate(&dirstate, skip_state)
-                .map_err(|e| {
+                .map_err(|e: DirstateMapError| {
                     PyErr::new::<exc::ValueError, _>(py, e.to_string())
                 })?
         } else {
diff --git a/rust/hg-cpython/src/dirstate.rs b/rust/hg-cpython/src/dirstate.rs
--- a/rust/hg-cpython/src/dirstate.rs
+++ b/rust/hg-cpython/src/dirstate.rs
@@ -24,10 +24,7 @@ 
     exc, PyBytes, PyDict, PyErr, PyList, PyModule, PyObject, PyResult,
     PySequence, Python,
 };
-use hg::{
-    utils::hg_path::HgPathBuf, DirstateEntry, DirstateParseError, EntryState,
-    StateMap,
-};
+use hg::{utils::hg_path::HgPathBuf, DirstateEntry, EntryState, StateMap};
 use libc::{c_char, c_int};
 use std::convert::TryFrom;
 
@@ -79,11 +76,10 @@ 
         .map(|(filename, stats)| {
             let stats = stats.extract::<PySequence>(py)?;
             let state = stats.get_item(py, 0)?.extract::<PyBytes>(py)?;
-            let state = EntryState::try_from(state.data(py)[0]).map_err(
-                |e: DirstateParseError| {
+            let state =
+                EntryState::try_from(state.data(py)[0]).map_err(|e| {
                     PyErr::new::<exc::ValueError, _>(py, e.to_string())
-                },
-            )?;
+                })?;
             let mode = stats.get_item(py, 1)?.extract(py)?;
             let size = stats.get_item(py, 2)?.extract(py)?;
             let mtime = stats.get_item(py, 3)?.extract(py)?;
diff --git a/rust/hg-core/src/operations/mod.rs b/rust/hg-core/src/operations/mod.rs
--- a/rust/hg-core/src/operations/mod.rs
+++ b/rust/hg-core/src/operations/mod.rs
@@ -10,5 +10,5 @@ 
 pub use cat::cat;
 pub use debugdata::{debug_data, DebugDataKind};
 pub use find_root::{find_root, find_root_from_path, FindRootError};
+pub use list_tracked_files::Dirstate;
 pub use list_tracked_files::{list_rev_tracked_files, FilesForRev};
-pub use list_tracked_files::{Dirstate, ListDirstateTrackedFilesError};
diff --git a/rust/hg-core/src/operations/list_tracked_files.rs b/rust/hg-core/src/operations/list_tracked_files.rs
--- a/rust/hg-core/src/operations/list_tracked_files.rs
+++ b/rust/hg-core/src/operations/list_tracked_files.rs
@@ -6,24 +6,16 @@ 
 // GNU General Public License version 2 or any later version.
 
 use crate::dirstate::parsers::parse_dirstate;
+use crate::errors::{HgError, IoResultExt};
 use crate::repo::Repo;
 use crate::revlog::changelog::Changelog;
 use crate::revlog::manifest::{Manifest, ManifestEntry};
 use crate::revlog::node::Node;
 use crate::revlog::revlog::RevlogError;
 use crate::utils::hg_path::HgPath;
-use crate::{DirstateParseError, EntryState};
+use crate::EntryState;
 use rayon::prelude::*;
 
-/// Error type for `Dirstate` methods
-#[derive(Debug, derive_more::From)]
-pub enum ListDirstateTrackedFilesError {
-    /// Error when reading the `dirstate` file
-    IoError(std::io::Error),
-    /// Error when parsing the `dirstate` file
-    ParseError(DirstateParseError),
-}
-
 /// List files under Mercurial control in the working directory
 /// by reading the dirstate
 pub struct Dirstate {
@@ -32,16 +24,18 @@ 
 }
 
 impl Dirstate {
-    pub fn new(repo: &Repo) -> Result<Self, ListDirstateTrackedFilesError> {
-        let content = repo.hg_vfs().read("dirstate")?;
+    pub fn new(repo: &Repo) -> Result<Self, HgError> {
+        let content = repo
+            .hg_vfs()
+            .read("dirstate")
+            // TODO: this will be more accurate when we use `HgError` in
+            // `Vfs::read`.
+            .for_file("dirstate".as_ref())?;
         Ok(Self { content })
     }
 
-    pub fn tracked_files(
-        &self,
-    ) -> Result<Vec<&HgPath>, ListDirstateTrackedFilesError> {
-        let (_, entries, _) = parse_dirstate(&self.content)
-            .map_err(ListDirstateTrackedFilesError::ParseError)?;
+    pub fn tracked_files(&self) -> Result<Vec<&HgPath>, HgError> {
+        let (_, entries, _) = parse_dirstate(&self.content)?;
         let mut files: Vec<&HgPath> = entries
             .into_iter()
             .filter_map(|(path, entry)| match entry.state {
diff --git a/rust/hg-core/src/lib.rs b/rust/hg-core/src/lib.rs
--- a/rust/hg-core/src/lib.rs
+++ b/rust/hg-core/src/lib.rs
@@ -51,33 +51,6 @@ 
 /// write access to your repository, you have other issues.
 pub type FastHashMap<K, V> = HashMap<K, V, RandomXxHashBuilder64>;
 
-#[derive(Clone, Debug, PartialEq)]
-pub enum DirstateParseError {
-    TooLittleData,
-    Overflow,
-    // TODO refactor to use bytes instead of String
-    CorruptedEntry(String),
-    Damaged,
-}
-
-impl From<std::io::Error> for DirstateParseError {
-    fn from(e: std::io::Error) -> Self {
-        DirstateParseError::CorruptedEntry(e.to_string())
-    }
-}
-
-impl ToString for DirstateParseError {
-    fn to_string(&self) -> String {
-        use crate::DirstateParseError::*;
-        match self {
-            TooLittleData => "Too little data for dirstate.".to_string(),
-            Overflow => "Overflow in dirstate.".to_string(),
-            CorruptedEntry(e) => format!("Corrupted entry: {:?}.", e),
-            Damaged => "Dirstate appears to be damaged.".to_string(),
-        }
-    }
-}
-
 #[derive(Debug, PartialEq)]
 pub enum DirstateMapError {
     PathNotFound(HgPathBuf),
@@ -99,9 +72,7 @@ 
 
 #[derive(Debug, derive_more::From)]
 pub enum DirstateError {
-    Parse(DirstateParseError),
     Map(DirstateMapError),
-    IO(std::io::Error),
     Common(errors::HgError),
 }
 
diff --git a/rust/hg-core/src/dirstate/parsers.rs b/rust/hg-core/src/dirstate/parsers.rs
--- a/rust/hg-core/src/dirstate/parsers.rs
+++ b/rust/hg-core/src/dirstate/parsers.rs
@@ -7,7 +7,7 @@ 
 use crate::utils::hg_path::HgPath;
 use crate::{
     dirstate::{CopyMap, EntryState, StateMap},
-    DirstateEntry, DirstateParents, DirstateParseError,
+    DirstateEntry, DirstateParents,
 };
 use byteorder::{BigEndian, ReadBytesExt, WriteBytesExt};
 use micro_timer::timed;
@@ -27,11 +27,9 @@ 
 );
 
 #[timed]
-pub fn parse_dirstate(
-    contents: &[u8],
-) -> Result<ParseResult, DirstateParseError> {
+pub fn parse_dirstate(contents: &[u8]) -> Result<ParseResult, HgError> {
     if contents.len() < PARENT_SIZE * 2 {
-        return Err(DirstateParseError::TooLittleData);
+        return Err(HgError::corrupted("Too little data for dirstate."));
     }
     let mut copies = vec![];
     let mut entries = vec![];
@@ -44,19 +42,21 @@ 
 
     while curr_pos < contents.len() {
         if curr_pos + MIN_ENTRY_SIZE > contents.len() {
-            return Err(DirstateParseError::Overflow);
+            return Err(HgError::corrupted("Overflow in dirstate."));
         }
         let entry_bytes = &contents[curr_pos..];
 
         let mut cursor = Cursor::new(entry_bytes);
-        let state = EntryState::try_from(cursor.read_u8()?)?;
-        let mode = cursor.read_i32::<BigEndian>()?;
-        let size = cursor.read_i32::<BigEndian>()?;
-        let mtime = cursor.read_i32::<BigEndian>()?;
-        let path_len = cursor.read_i32::<BigEndian>()? as usize;
+        // Unwraping errors from `byteorder` as we’ve already checked
+        // `MIN_ENTRY_SIZE` so the input should never be too short.
+        let state = EntryState::try_from(cursor.read_u8().unwrap())?;
+        let mode = cursor.read_i32::<BigEndian>().unwrap();
+        let size = cursor.read_i32::<BigEndian>().unwrap();
+        let mtime = cursor.read_i32::<BigEndian>().unwrap();
+        let path_len = cursor.read_i32::<BigEndian>().unwrap() as usize;
 
         if path_len > contents.len() - curr_pos {
-            return Err(DirstateParseError::Overflow);
+            return Err(HgError::corrupted("Overflow in dirstate."));
         }
 
         // Slice instead of allocating a Vec needed for `read_exact`
diff --git a/rust/hg-core/src/dirstate/dirstate_map.rs b/rust/hg-core/src/dirstate/dirstate_map.rs
--- a/rust/hg-core/src/dirstate/dirstate_map.rs
+++ b/rust/hg-core/src/dirstate/dirstate_map.rs
@@ -5,6 +5,7 @@ 
 // 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::errors::HgError;
 use crate::revlog::node::NULL_NODE_ID;
 use crate::{
     dirstate::{parsers::PARENT_SIZE, EntryState, SIZE_FROM_OTHER_PARENT},
@@ -14,7 +15,7 @@ 
         hg_path::{HgPath, HgPathBuf},
     },
     CopyMap, DirsMultiset, DirstateEntry, DirstateError, DirstateMapError,
-    DirstateParents, DirstateParseError, FastHashMap, StateMap,
+    DirstateParents, FastHashMap, StateMap,
 };
 use micro_timer::timed;
 use std::collections::HashSet;
@@ -370,7 +371,9 @@ 
                 p2: NULL_NODE_ID,
             };
         } else {
-            return Err(DirstateError::Parse(DirstateParseError::Damaged));
+            return Err(
+                HgError::corrupted("Dirstate appears to be damaged").into()
+            );
         }
 
         self.parents = Some(parents);
diff --git a/rust/hg-core/src/dirstate.rs b/rust/hg-core/src/dirstate.rs
--- a/rust/hg-core/src/dirstate.rs
+++ b/rust/hg-core/src/dirstate.rs
@@ -5,7 +5,8 @@ 
 // 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::{utils::hg_path::HgPathBuf, DirstateParseError, FastHashMap};
+use crate::errors::HgError;
+use crate::{utils::hg_path::HgPathBuf, FastHashMap};
 use std::collections::hash_map;
 use std::convert::TryFrom;
 
@@ -60,7 +61,7 @@ 
 }
 
 impl TryFrom<u8> for EntryState {
-    type Error = DirstateParseError;
+    type Error = HgError;
 
     fn try_from(value: u8) -> Result<Self, Self::Error> {
         match value {
@@ -69,8 +70,8 @@ 
             b'r' => Ok(EntryState::Removed),
             b'm' => Ok(EntryState::Merged),
             b'?' => Ok(EntryState::Unknown),
-            _ => Err(DirstateParseError::CorruptedEntry(format!(
-                "Incorrect entry state {}",
+            _ => Err(HgError::CorruptedRepository(format!(
+                "Incorrect dirstate entry state {}",
                 value
             ))),
         }