rust-dirstate: use EntryState enum instead of literals

This improves code readability quite a bit, while also adding a layer of
safety because we're checking the state byte against the enum.

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

rust/hg-core/src/dirstate.rs

@@ -1,38 +1,79 b''
 // dirstate module
 //
 // Copyright 2019 Raphaël Gomès <[e-mail]>
 //
 // 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::DirstateParseError;
 use std::collections::HashMap;
+use std::convert::TryFrom;
 
 pub mod dirs_multiset;
 pub mod parsers;
 
 #[derive(Debug, PartialEq, Clone)]
 pub struct DirstateParents {
     pub p1: [u8; 20],
     pub p2: [u8; 20],
 }
 
 /// The C implementation uses all signed types. This will be an issue
 /// either when 4GB+ source files are commonplace or in 2038, whichever
 /// comes first.
 #[derive(Debug, PartialEq, Copy, Clone)]
 pub struct DirstateEntry {
-    pub state: i8,
+    pub state: EntryState,
     pub mode: i32,
     pub mtime: i32,
     pub size: i32,
 }
 
 pub type StateMap = HashMap<Vec<u8>, DirstateEntry>;
 pub type CopyMap = HashMap<Vec<u8>, Vec<u8>>;
 
 /// The Python implementation passes either a mapping (dirstate) or a flat
 /// iterable (manifest)
 pub enum DirsIterable<'a> {
     Dirstate(&'a HashMap<Vec<u8>, DirstateEntry>),
     Manifest(&'a Vec<Vec<u8>>),
 }
+
+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+pub enum EntryState {
+    Normal,
+    Added,
+    Removed,
+    Merged,
+    Unknown,
+}
+
+impl TryFrom<u8> for EntryState {
+    type Error = DirstateParseError;
+
+    fn try_from(value: u8) -> Result<Self, Self::Error> {
+        match value {
+            b'n' => Ok(EntryState::Normal),
+            b'a' => Ok(EntryState::Added),
+            b'r' => Ok(EntryState::Removed),
+            b'm' => Ok(EntryState::Merged),
+            b'?' => Ok(EntryState::Unknown),
+            _ => Err(DirstateParseError::CorruptedEntry(format!(
+                "Incorrect entry state {}",
+                value
+            ))),
+        }
+    }
+}
+
+impl Into<u8> for EntryState {
+    fn into(self) -> u8 {
+        match self {
+            EntryState::Normal => b'n',
+            EntryState::Added => b'a',
+            EntryState::Removed => b'r',
+            EntryState::Merged => b'm',
+            EntryState::Unknown => b'?',
+        }
+    }
+}

rust/hg-core/src/dirstate/dirs_multiset.rs

@@ -1,328 +1,341 b''
 // dirs_multiset.rs
 //
 // Copyright 2019 Raphaël Gomès <[e-mail]>
 //
 // This software may be used and distributed according to the terms of the
 // GNU General Public License version 2 or any later version.
 
 //! A multiset of directory names.
 //!
 //! Used to counts the references to directories in a manifest or dirstate.
-use crate::{utils::files, DirsIterable, DirstateEntry, DirstateMapError};
+use crate::{
+    dirstate::EntryState, utils::files, DirsIterable, DirstateEntry,
+    DirstateMapError,
+};
 use std::collections::hash_map::{Entry, Iter};
 use std::collections::HashMap;
 
 #[derive(PartialEq, Debug)]
 pub struct DirsMultiset {
     inner: HashMap<Vec<u8>, u32>,
 }
 
 impl DirsMultiset {
     /// Initializes the multiset from a dirstate or a manifest.
     ///
     /// If `skip_state` is provided, skips dirstate entries with equal state.
-    pub fn new(iterable: DirsIterable, skip_state: Option<i8>) -> Self {
+    pub fn new(
+        iterable: DirsIterable,
+        skip_state: Option<EntryState>,
+    ) -> Self {
         let mut multiset = DirsMultiset {
             inner: HashMap::new(),
         };
 
         match iterable {
             DirsIterable::Dirstate(vec) => {
                 for (filename, DirstateEntry { state, .. }) in vec {
                     // This `if` is optimized out of the loop
                     if let Some(skip) = skip_state {
                         if skip != *state {
                             multiset.add_path(filename);
                         }
                     } else {
                         multiset.add_path(filename);
                     }
                 }
             }
             DirsIterable::Manifest(vec) => {
                 for filename in vec {
                     multiset.add_path(filename);
                 }
             }
         }
 
         multiset
     }
 
     /// Increases the count of deepest directory contained in the path.
     ///
     /// If the directory is not yet in the map, adds its parents.
     pub fn add_path(&mut self, path: &[u8]) {
         for subpath in files::find_dirs(path) {
             if let Some(val) = self.inner.get_mut(subpath) {
                 *val += 1;
                 break;
             }
             self.inner.insert(subpath.to_owned(), 1);
         }
     }
 
     /// Decreases the count of deepest directory contained in the path.
     ///
     /// If it is the only reference, decreases all parents until one is
     /// removed.
     /// If the directory is not in the map, something horrible has happened.
     pub fn delete_path(
         &mut self,
         path: &[u8],
     ) -> Result<(), DirstateMapError> {
         for subpath in files::find_dirs(path) {
             match self.inner.entry(subpath.to_owned()) {
                 Entry::Occupied(mut entry) => {
                     let val = entry.get().clone();
                     if val > 1 {
                         entry.insert(val - 1);
                         break;
                     }
                     entry.remove();
                 }
                 Entry::Vacant(_) => {
                     return Err(DirstateMapError::PathNotFound(
                         path.to_owned(),
                     ))
                 }
             };
         }
 
         Ok(())
     }
 
     pub fn contains_key(&self, key: &[u8]) -> bool {
         self.inner.contains_key(key)
     }
 
     pub fn iter(&self) -> Iter<Vec<u8>, u32> {
         self.inner.iter()
     }
 
     pub fn len(&self) -> usize {
         self.inner.len()
     }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
     use std::collections::HashMap;
 
     #[test]
     fn test_delete_path_path_not_found() {
         let mut map = DirsMultiset::new(DirsIterable::Manifest(&vec![]), None);
         let path = b"doesnotexist/";
         assert_eq!(
             Err(DirstateMapError::PathNotFound(path.to_vec())),
             map.delete_path(path)
         );
     }
 
     #[test]
     fn test_delete_path_empty_path() {
         let mut map =
             DirsMultiset::new(DirsIterable::Manifest(&vec![vec![]]), None);
         let path = b"";
         assert_eq!(Ok(()), map.delete_path(path));
         assert_eq!(
             Err(DirstateMapError::PathNotFound(path.to_vec())),
             map.delete_path(path)
         );
     }
 
     #[test]
     fn test_delete_path_successful() {
         let mut map = DirsMultiset {
             inner: [("", 5), ("a", 3), ("a/b", 2), ("a/c", 1)]
                 .iter()
                 .map(|(k, v)| (k.as_bytes().to_vec(), *v))
                 .collect(),
         };
 
         assert_eq!(Ok(()), map.delete_path(b"a/b/"));
         assert_eq!(Ok(()), map.delete_path(b"a/b/"));
         assert_eq!(
             Err(DirstateMapError::PathNotFound(b"a/b/".to_vec())),
             map.delete_path(b"a/b/")
         );
 
         assert_eq!(2, *map.inner.get(&b"a".to_vec()).unwrap());
         assert_eq!(1, *map.inner.get(&b"a/c".to_vec()).unwrap());
         eprintln!("{:?}", map);
         assert_eq!(Ok(()), map.delete_path(b"a/"));
         eprintln!("{:?}", map);
 
         assert_eq!(Ok(()), map.delete_path(b"a/c/"));
         assert_eq!(
             Err(DirstateMapError::PathNotFound(b"a/c/".to_vec())),
             map.delete_path(b"a/c/")
         );
     }
 
     #[test]
     fn test_add_path_empty_path() {
         let mut map = DirsMultiset::new(DirsIterable::Manifest(&vec![]), None);
         let path = b"";
         map.add_path(path);
 
         assert_eq!(1, map.len());
     }
 
     #[test]
     fn test_add_path_successful() {
         let mut map = DirsMultiset::new(DirsIterable::Manifest(&vec![]), None);
 
         map.add_path(b"a/");
         assert_eq!(1, *map.inner.get(&b"a".to_vec()).unwrap());
         assert_eq!(1, *map.inner.get(&Vec::new()).unwrap());
         assert_eq!(2, map.len());
 
         // Non directory should be ignored
         map.add_path(b"a");
         assert_eq!(1, *map.inner.get(&b"a".to_vec()).unwrap());
         assert_eq!(2, map.len());
 
         // Non directory will still add its base
         map.add_path(b"a/b");
         assert_eq!(2, *map.inner.get(&b"a".to_vec()).unwrap());
         assert_eq!(2, map.len());
 
         // Duplicate path works
         map.add_path(b"a/");
         assert_eq!(3, *map.inner.get(&b"a".to_vec()).unwrap());
 
         // Nested dir adds to its base
         map.add_path(b"a/b/");
         assert_eq!(4, *map.inner.get(&b"a".to_vec()).unwrap());
         assert_eq!(1, *map.inner.get(&b"a/b".to_vec()).unwrap());
 
         // but not its base's base, because it already existed
         map.add_path(b"a/b/c/");
         assert_eq!(4, *map.inner.get(&b"a".to_vec()).unwrap());
         assert_eq!(2, *map.inner.get(&b"a/b".to_vec()).unwrap());
 
         map.add_path(b"a/c/");
         assert_eq!(1, *map.inner.get(&b"a/c".to_vec()).unwrap());
 
         let expected = DirsMultiset {
             inner: [("", 2), ("a", 5), ("a/b", 2), ("a/b/c", 1), ("a/c", 1)]
                 .iter()
                 .map(|(k, v)| (k.as_bytes().to_vec(), *v))
                 .collect(),
         };
         assert_eq!(map, expected);
     }
 
     #[test]
     fn test_dirsmultiset_new_empty() {
         use DirsIterable::{Dirstate, Manifest};
 
         let new = DirsMultiset::new(Manifest(&vec![]), None);
         let expected = DirsMultiset {
             inner: HashMap::new(),
         };
         assert_eq!(expected, new);
 
         let new = DirsMultiset::new(Dirstate(&HashMap::new()), None);
         let expected = DirsMultiset {
             inner: HashMap::new(),
         };
         assert_eq!(expected, new);
     }
 
     #[test]
     fn test_dirsmultiset_new_no_skip() {
         use DirsIterable::{Dirstate, Manifest};
 
         let input_vec = ["a/", "b/", "a/c", "a/d/"]
             .iter()
             .map(|e| e.as_bytes().to_vec())
             .collect();
         let expected_inner = [("", 2), ("a", 3), ("b", 1), ("a/d", 1)]
             .iter()
             .map(|(k, v)| (k.as_bytes().to_vec(), *v))
             .collect();
 
         let new = DirsMultiset::new(Manifest(&input_vec), None);
         let expected = DirsMultiset {
             inner: expected_inner,
         };
         assert_eq!(expected, new);
 
         let input_map = ["a/", "b/", "a/c", "a/d/"]
             .iter()
             .map(|f| {
                 (
                     f.as_bytes().to_vec(),
                     DirstateEntry {
-                        state: 0,
+                        state: EntryState::Normal,
                         mode: 0,
                         mtime: 0,
                         size: 0,
                     },
                 )
             })
             .collect();
         let expected_inner = [("", 2), ("a", 3), ("b", 1), ("a/d", 1)]
             .iter()
             .map(|(k, v)| (k.as_bytes().to_vec(), *v))
             .collect();
 
         let new = DirsMultiset::new(Dirstate(&input_map), None);
         let expected = DirsMultiset {
             inner: expected_inner,
         };
         assert_eq!(expected, new);
     }
 
     #[test]
     fn test_dirsmultiset_new_skip() {
         use DirsIterable::{Dirstate, Manifest};
 
         let input_vec = ["a/", "b/", "a/c", "a/d/"]
             .iter()
             .map(|e| e.as_bytes().to_vec())
             .collect();
         let expected_inner = [("", 2), ("a", 3), ("b", 1), ("a/d", 1)]
             .iter()
             .map(|(k, v)| (k.as_bytes().to_vec(), *v))
             .collect();
 
-        let new = DirsMultiset::new(Manifest(&input_vec), Some('n' as i8));
+        let new =
+            DirsMultiset::new(Manifest(&input_vec), Some(EntryState::Normal));
         let expected = DirsMultiset {
             inner: expected_inner,
         };
         // Skip does not affect a manifest
         assert_eq!(expected, new);
 
-        let input_map =
-            [("a/", 'n'), ("a/b/", 'n'), ("a/c", 'r'), ("a/d/", 'm')]
-                .iter()
-                .map(|(f, state)| {
-                    (
-                        f.as_bytes().to_vec(),
-                        DirstateEntry {
-                            state: *state as i8,
-                            mode: 0,
-                            mtime: 0,
-                            size: 0,
-                        },
-                    )
-                })
-                .collect();
+        let input_map = [
+            ("a/", EntryState::Normal),
+            ("a/b/", EntryState::Normal),
+            ("a/c", EntryState::Removed),
+            ("a/d/", EntryState::Merged),
+        ]
+        .iter()
+        .map(|(f, state)| {
+            (
+                f.as_bytes().to_vec(),
+                DirstateEntry {
+                    state: *state,
+                    mode: 0,
+                    mtime: 0,
+                    size: 0,
+                },
+            )
+        })
+        .collect();
 
         // "a" incremented with "a/c" and "a/d/"
         let expected_inner = [("", 1), ("a", 2), ("a/d", 1)]
             .iter()
             .map(|(k, v)| (k.as_bytes().to_vec(), *v))
             .collect();
 
-        let new = DirsMultiset::new(Dirstate(&input_map), Some('n' as i8));
+        let new =
+            DirsMultiset::new(Dirstate(&input_map), Some(EntryState::Normal));
         let expected = DirsMultiset {
             inner: expected_inner,
         };
         assert_eq!(expected, new);
     }
+
 }

rust/hg-core/src/dirstate/parsers.rs

@@ -1,414 +1,415 b''
 // Copyright 2019 Raphaël Gomès <[e-mail]>
 //
 // 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::{
-    dirstate::{CopyMap, StateMap},
+    dirstate::{CopyMap, EntryState, StateMap},
     utils::copy_into_array,
     DirstateEntry, DirstatePackError, DirstateParents, DirstateParseError,
 };
 use byteorder::{BigEndian, ReadBytesExt, WriteBytesExt};
-use std::convert::TryInto;
+use std::convert::{TryFrom, TryInto};
 use std::io::Cursor;
 use std::time::Duration;
 
 /// Parents are stored in the dirstate as byte hashes.
 pub const PARENT_SIZE: usize = 20;
 /// Dirstate entries have a static part of 8 + 32 + 32 + 32 + 32 bits.
 const MIN_ENTRY_SIZE: usize = 17;
 
 // TODO parse/pack: is mutate-on-loop better for performance?
 
 pub fn parse_dirstate(
     state_map: &mut StateMap,
     copy_map: &mut CopyMap,
     contents: &[u8],
 ) -> Result<DirstateParents, DirstateParseError> {
     if contents.len() < PARENT_SIZE * 2 {
         return Err(DirstateParseError::TooLittleData);
     }
 
     let mut curr_pos = PARENT_SIZE * 2;
     let parents = DirstateParents {
         p1: copy_into_array(&contents[..PARENT_SIZE]),
         p2: copy_into_array(&contents[PARENT_SIZE..curr_pos]),
     };
 
     while curr_pos < contents.len() {
         if curr_pos + MIN_ENTRY_SIZE > contents.len() {
             return Err(DirstateParseError::Overflow);
         }
         let entry_bytes = &contents[curr_pos..];
 
         let mut cursor = Cursor::new(entry_bytes);
-        let state = cursor.read_i8()?;
+        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;
 
         if path_len > contents.len() - curr_pos {
             return Err(DirstateParseError::Overflow);
         }
 
         // Slice instead of allocating a Vec needed for `read_exact`
         let path = &entry_bytes[MIN_ENTRY_SIZE..MIN_ENTRY_SIZE + (path_len)];
 
         let (path, copy) = match memchr::memchr(0, path) {
             None => (path, None),
             Some(i) => (&path[..i], Some(&path[(i + 1)..])),
         };
 
         if let Some(copy_path) = copy {
             copy_map.insert(path.to_owned(), copy_path.to_owned());
         };
         state_map.insert(
             path.to_owned(),
             DirstateEntry {
                 state,
                 mode,
                 size,
                 mtime,
             },
         );
         curr_pos = curr_pos + MIN_ENTRY_SIZE + (path_len);
     }
 
     Ok(parents)
 }
 
 /// `now` is the duration in seconds since the Unix epoch
 pub fn pack_dirstate(
     state_map: &mut StateMap,
     copy_map: &CopyMap,
     parents: DirstateParents,
     now: Duration,
 ) -> Result<Vec<u8>, DirstatePackError> {
     // TODO move away from i32 before 2038.
     let now: i32 = now.as_secs().try_into().expect("time overflow");
 
     let expected_size: usize = state_map
         .iter()
         .map(|(filename, _)| {
             let mut length = MIN_ENTRY_SIZE + filename.len();
             if let Some(ref copy) = copy_map.get(filename) {
                 length += copy.len() + 1;
             }
             length
         })
         .sum();
     let expected_size = expected_size + PARENT_SIZE * 2;
 
     let mut packed = Vec::with_capacity(expected_size);
     let mut new_state_map = vec![];
 
     packed.extend(&parents.p1);
     packed.extend(&parents.p2);
 
     for (ref filename, entry) in state_map.iter() {
         let mut new_filename: Vec<u8> = filename.to_vec();
         let mut new_mtime: i32 = entry.mtime;
-        if entry.state == 'n' as i8 && entry.mtime == now.into() {
+        if entry.state == EntryState::Normal && entry.mtime == now {
             // The file was last modified "simultaneously" with the current
             // write to dirstate (i.e. within the same second for file-
             // systems with a granularity of 1 sec). This commonly happens
             // for at least a couple of files on 'update'.
             // The user could change the file without changing its size
             // within the same second. Invalidate the file's mtime in
             // dirstate, forcing future 'status' calls to compare the
             // contents of the file if the size is the same. This prevents
             // mistakenly treating such files as clean.
             new_mtime = -1;
             new_state_map.push((
                 filename.to_owned().to_vec(),
                 DirstateEntry {
                     mtime: new_mtime,
                     ..*entry
                 },
             ));
         }
 
         if let Some(copy) = copy_map.get(*filename) {
             new_filename.push('\0' as u8);
             new_filename.extend(copy);
         }
 
-        packed.write_i8(entry.state)?;
+        packed.write_u8(entry.state.into())?;
         packed.write_i32::<BigEndian>(entry.mode)?;
         packed.write_i32::<BigEndian>(entry.size)?;
         packed.write_i32::<BigEndian>(new_mtime)?;
         packed.write_i32::<BigEndian>(new_filename.len() as i32)?;
         packed.extend(new_filename)
     }
 
     if packed.len() != expected_size {
         return Err(DirstatePackError::BadSize(expected_size, packed.len()));
     }
 
     state_map.extend(new_state_map);
 
     Ok(packed)
 }
+
 #[cfg(test)]
 mod tests {
     use super::*;
     use std::collections::HashMap;
 
     #[test]
     fn test_pack_dirstate_empty() {
         let mut state_map: StateMap = HashMap::new();
         let copymap = HashMap::new();
         let parents = DirstateParents {
             p1: *b"12345678910111213141",
             p2: *b"00000000000000000000",
         };
         let now = Duration::new(15000000, 0);
         let expected = b"1234567891011121314100000000000000000000".to_vec();
 
         assert_eq!(
             expected,
             pack_dirstate(&mut state_map, &copymap, parents, now).unwrap()
         );
 
         assert!(state_map.is_empty())
     }
     #[test]
     fn test_pack_dirstate_one_entry() {
         let expected_state_map: StateMap = [(
             b"f1".to_vec(),
             DirstateEntry {
-                state: 'n' as i8,
+                state: EntryState::Normal,
                 mode: 0o644,
                 size: 0,
                 mtime: 791231220,
             },
         )]
         .iter()
         .cloned()
         .collect();
         let mut state_map = expected_state_map.clone();
 
         let copymap = HashMap::new();
         let parents = DirstateParents {
             p1: *b"12345678910111213141",
             p2: *b"00000000000000000000",
         };
         let now = Duration::new(15000000, 0);
         let expected = [
             49, 50, 51, 52, 53, 54, 55, 56, 57, 49, 48, 49, 49, 49, 50, 49,
             51, 49, 52, 49, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48,
             48, 48, 48, 48, 48, 48, 48, 48, 110, 0, 0, 1, 164, 0, 0, 0, 0, 47,
             41, 58, 244, 0, 0, 0, 2, 102, 49,
         ]
         .to_vec();
 
         assert_eq!(
             expected,
             pack_dirstate(&mut state_map, &copymap, parents, now).unwrap()
         );
 
         assert_eq!(expected_state_map, state_map);
     }
     #[test]
     fn test_pack_dirstate_one_entry_with_copy() {
         let expected_state_map: StateMap = [(
             b"f1".to_vec(),
             DirstateEntry {
-                state: 'n' as i8,
+                state: EntryState::Normal,
                 mode: 0o644,
                 size: 0,
                 mtime: 791231220,
             },
         )]
         .iter()
         .cloned()
         .collect();
         let mut state_map = expected_state_map.clone();
         let mut copymap = HashMap::new();
         copymap.insert(b"f1".to_vec(), b"copyname".to_vec());
         let parents = DirstateParents {
             p1: *b"12345678910111213141",
             p2: *b"00000000000000000000",
         };
         let now = Duration::new(15000000, 0);
         let expected = [
             49, 50, 51, 52, 53, 54, 55, 56, 57, 49, 48, 49, 49, 49, 50, 49,
             51, 49, 52, 49, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48,
             48, 48, 48, 48, 48, 48, 48, 48, 110, 0, 0, 1, 164, 0, 0, 0, 0, 47,
             41, 58, 244, 0, 0, 0, 11, 102, 49, 0, 99, 111, 112, 121, 110, 97,
             109, 101,
         ]
         .to_vec();
 
         assert_eq!(
             expected,
             pack_dirstate(&mut state_map, &copymap, parents, now).unwrap()
         );
         assert_eq!(expected_state_map, state_map);
     }
 
     #[test]
     fn test_parse_pack_one_entry_with_copy() {
         let mut state_map: StateMap = [(
             b"f1".to_vec(),
             DirstateEntry {
-                state: 'n' as i8,
+                state: EntryState::Normal,
                 mode: 0o644,
                 size: 0,
                 mtime: 791231220,
             },
         )]
         .iter()
         .cloned()
         .collect();
         let mut copymap = HashMap::new();
         copymap.insert(b"f1".to_vec(), b"copyname".to_vec());
         let parents = DirstateParents {
             p1: *b"12345678910111213141",
             p2: *b"00000000000000000000",
         };
         let now = Duration::new(15000000, 0);
         let result =
             pack_dirstate(&mut state_map, &copymap, parents.clone(), now)
                 .unwrap();
 
         let mut new_state_map: StateMap = HashMap::new();
         let mut new_copy_map: CopyMap = HashMap::new();
         let new_parents = parse_dirstate(
             &mut new_state_map,
             &mut new_copy_map,
             result.as_slice(),
         )
         .unwrap();
         assert_eq!(
             (parents, state_map, copymap),
             (new_parents, new_state_map, new_copy_map)
         )
     }
 
     #[test]
     fn test_parse_pack_multiple_entries_with_copy() {
         let mut state_map: StateMap = [
             (
                 b"f1".to_vec(),
                 DirstateEntry {
-                    state: 'n' as i8,
+                    state: EntryState::Normal,
                     mode: 0o644,
                     size: 0,
                     mtime: 791231220,
                 },
             ),
             (
                 b"f2".to_vec(),
                 DirstateEntry {
-                    state: 'm' as i8,
+                    state: EntryState::Merged,
                     mode: 0o777,
                     size: 1000,
                     mtime: 791231220,
                 },
             ),
             (
                 b"f3".to_vec(),
                 DirstateEntry {
-                    state: 'r' as i8,
+                    state: EntryState::Removed,
                     mode: 0o644,
                     size: 234553,
                     mtime: 791231220,
                 },
             ),
             (
                 b"f4\xF6".to_vec(),
                 DirstateEntry {
-                    state: 'a' as i8,
+                    state: EntryState::Added,
                     mode: 0o644,
                     size: -1,
                     mtime: -1,
                 },
             ),
         ]
         .iter()
         .cloned()
         .collect();
         let mut copymap = HashMap::new();
         copymap.insert(b"f1".to_vec(), b"copyname".to_vec());
         copymap.insert(b"f4\xF6".to_vec(), b"copyname2".to_vec());
         let parents = DirstateParents {
             p1: *b"12345678910111213141",
             p2: *b"00000000000000000000",
         };
         let now = Duration::new(15000000, 0);
         let result =
             pack_dirstate(&mut state_map, &copymap, parents.clone(), now)
                 .unwrap();
 
         let mut new_state_map: StateMap = HashMap::new();
         let mut new_copy_map: CopyMap = HashMap::new();
         let new_parents = parse_dirstate(
             &mut new_state_map,
             &mut new_copy_map,
             result.as_slice(),
         )
         .unwrap();
         assert_eq!(
             (parents, state_map, copymap),
             (new_parents, new_state_map, new_copy_map)
         )
     }
 
     #[test]
     /// https://www.mercurial-scm.org/repo/hg/rev/af3f26b6bba4
     fn test_parse_pack_one_entry_with_copy_and_time_conflict() {
         let mut state_map: StateMap = [(
             b"f1".to_vec(),
             DirstateEntry {
-                state: 'n' as i8,
+                state: EntryState::Normal,
                 mode: 0o644,
                 size: 0,
                 mtime: 15000000,
             },
         )]
         .iter()
         .cloned()
         .collect();
         let mut copymap = HashMap::new();
         copymap.insert(b"f1".to_vec(), b"copyname".to_vec());
         let parents = DirstateParents {
             p1: *b"12345678910111213141",
             p2: *b"00000000000000000000",
         };
         let now = Duration::new(15000000, 0);
         let result =
             pack_dirstate(&mut state_map, &copymap, parents.clone(), now)
                 .unwrap();
 
         let mut new_state_map: StateMap = HashMap::new();
         let mut new_copy_map: CopyMap = HashMap::new();
         let new_parents = parse_dirstate(
             &mut new_state_map,
             &mut new_copy_map,
             result.as_slice(),
         )
         .unwrap();
 
         assert_eq!(
             (
                 parents,
                 [(
                     b"f1".to_vec(),
                     DirstateEntry {
-                        state: 'n' as i8,
+                        state: EntryState::Normal,
                         mode: 0o644,
                         size: 0,
                         mtime: -1
                     }
                 )]
                 .iter()
                 .cloned()
                 .collect::<StateMap>(),
                 copymap,
             ),
             (new_parents, new_state_map, new_copy_map)
         )
     }
 }

rust/hg-core/src/lib.rs

@@ -1,105 +1,148 b''
 // Copyright 2018 Georges Racinet <[e-mail]>
 //
 // This software may be used and distributed according to the terms of the
 // GNU General Public License version 2 or any later version.
 mod ancestors;
 pub mod dagops;
 pub use ancestors::{AncestorsIterator, LazyAncestors, MissingAncestors};
 mod dirstate;
 pub mod discovery;
 pub mod testing; // unconditionally built, for use from integration tests
 pub use dirstate::{
     dirs_multiset::DirsMultiset,
     parsers::{pack_dirstate, parse_dirstate, PARENT_SIZE},
-    CopyMap, DirsIterable, DirstateEntry, DirstateParents, StateMap,
+    CopyMap, DirsIterable, DirstateEntry, DirstateParents, EntryState,
+    StateMap,
 };
 mod filepatterns;
 pub mod utils;
 
 pub use filepatterns::{
     build_single_regex, read_pattern_file, PatternSyntax, PatternTuple,
 };
 
 /// Mercurial revision numbers
 ///
 /// As noted in revlog.c, revision numbers are actually encoded in
 /// 4 bytes, and are liberally converted to ints, whence the i32
 pub type Revision = i32;
 
 /// Marker expressing the absence of a parent
 ///
 /// Independently of the actual representation, `NULL_REVISION` is guaranteed
 /// to be smaller that all existing revisions.
 pub const NULL_REVISION: Revision = -1;
 
 /// Same as `mercurial.node.wdirrev`
 ///
 /// This is also equal to `i32::max_value()`, but it's better to spell
 /// it out explicitely, same as in `mercurial.node`
 pub const WORKING_DIRECTORY_REVISION: Revision = 0x7fffffff;
 
 /// The simplest expression of what we need of Mercurial DAGs.
 pub trait Graph {
     /// Return the two parents of the given `Revision`.
     ///
     /// Each of the parents can be independently `NULL_REVISION`
     fn parents(&self, rev: Revision) -> Result<[Revision; 2], GraphError>;
 }
 
 pub type LineNumber = usize;
 
 #[derive(Clone, Debug, PartialEq)]
 pub enum GraphError {
     ParentOutOfRange(Revision),
     WorkingDirectoryUnsupported,
 }
 
 #[derive(Clone, Debug, PartialEq)]
 pub enum DirstateParseError {
     TooLittleData,
     Overflow,
     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 DirstatePackError {
     CorruptedEntry(String),
     CorruptedParent,
     BadSize(usize, usize),
 }
 
+impl From<std::io::Error> for DirstatePackError {
+    fn from(e: std::io::Error) -> Self {
+        DirstatePackError::CorruptedEntry(e.to_string())
+    }
+}
 #[derive(Debug, PartialEq)]
 pub enum DirstateMapError {
     PathNotFound(Vec<u8>),
     EmptyPath,
 }
 
-impl From<std::io::Error> for DirstatePackError {
-    fn from(e: std::io::Error) -> Self {
-        DirstatePackError::CorruptedEntry(e.to_string())
+pub enum DirstateError {
+    Parse(DirstateParseError),
+    Pack(DirstatePackError),
+    Map(DirstateMapError),
+    IO(std::io::Error),
+}
+
+impl From<DirstateParseError> for DirstateError {
+    fn from(e: DirstateParseError) -> Self {
+        DirstateError::Parse(e)
     }
 }
 
-impl From<std::io::Error> for DirstateParseError {
-    fn from(e: std::io::Error) -> Self {
-        DirstateParseError::CorruptedEntry(e.to_string())
+impl From<DirstatePackError> for DirstateError {
+    fn from(e: DirstatePackError) -> Self {
+        DirstateError::Pack(e)
     }
 }
 
 #[derive(Debug)]
 pub enum PatternError {
     UnsupportedSyntax(String),
 }
 
 #[derive(Debug)]
 pub enum PatternFileError {
     IO(std::io::Error),
     Pattern(PatternError, LineNumber),
 }
 
 impl From<std::io::Error> for PatternFileError {
     fn from(e: std::io::Error) -> Self {
         PatternFileError::IO(e)
     }
 }
+
+impl From<DirstateMapError> for DirstateError {
+    fn from(e: DirstateMapError) -> Self {
+        DirstateError::Map(e)
+    }
+}
+
+impl From<std::io::Error> for DirstateError {
+    fn from(e: std::io::Error) -> Self {
+        DirstateError::IO(e)
+    }
+}

rust/hg-cpython/src/dirstate.rs

@@ -1,97 +1,103 b''
 // dirstate.rs
 //
 // Copyright 2019 Raphaël Gomès <[e-mail]>
 //
 // This software may be used and distributed according to the terms of the
 // GNU General Public License version 2 or any later version.
 
 //! Bindings for the `hg::dirstate` module provided by the
 //! `hg-core` package.
 //!
 //! From Python, this will be seen as `mercurial.rustext.dirstate`
 mod dirs_multiset;
 use crate::dirstate::dirs_multiset::Dirs;
 use cpython::{
-    PyBytes, PyDict, PyErr, PyModule, PyObject, PyResult, PySequence, Python,
+    exc, PyBytes, PyDict, PyErr, PyModule, PyObject, PyResult, PySequence,
+    Python,
 };
-use hg::{DirstateEntry, StateMap};
+use hg::{DirstateEntry, DirstateParseError, EntryState, StateMap};
 use libc::{c_char, c_int};
 #[cfg(feature = "python27")]
 use python27_sys::PyCapsule_Import;
 #[cfg(feature = "python3")]
 use python3_sys::PyCapsule_Import;
+use std::convert::TryFrom;
 use std::ffi::CStr;
 use std::mem::transmute;
 
 /// C code uses a custom `dirstate_tuple` type, checks in multiple instances
 /// for this type, and raises a Python `Exception` if the check does not pass.
 /// Because this type differs only in name from the regular Python tuple, it
 /// would be a good idea in the near future to remove it entirely to allow
 /// for a pure Python tuple of the same effective structure to be used,
 /// rendering this type and the capsule below useless.
 type MakeDirstateTupleFn = extern "C" fn(
     state: c_char,
     mode: c_int,
     size: c_int,
     mtime: c_int,
 ) -> PyObject;
 
 /// This is largely a copy/paste from cindex.rs, pending the merge of a
 /// `py_capsule_fn!` macro in the rust-cpython project:
 /// https://github.com/dgrunwald/rust-cpython/pull/169
 pub fn decapsule_make_dirstate_tuple(
     py: Python,
 ) -> PyResult<MakeDirstateTupleFn> {
     unsafe {
         let caps_name = CStr::from_bytes_with_nul_unchecked(
             b"mercurial.cext.parsers.make_dirstate_tuple_CAPI\0",
         );
         let from_caps = PyCapsule_Import(caps_name.as_ptr(), 0);
         if from_caps.is_null() {
             return Err(PyErr::fetch(py));
         }
         Ok(transmute(from_caps))
     }
 }
 
 pub fn extract_dirstate(py: Python, dmap: &PyDict) -> Result<StateMap, PyErr> {
     dmap.items(py)
         .iter()
         .map(|(filename, stats)| {
             let stats = stats.extract::<PySequence>(py)?;
             let state = stats.get_item(py, 0)?.extract::<PyBytes>(py)?;
-            let state = state.data(py)[0] as i8;
+            let state = EntryState::try_from(state.data(py)[0]).map_err(
+                |e: DirstateParseError| {
+                    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)?;
             let filename = filename.extract::<PyBytes>(py)?;
             let filename = filename.data(py);
             Ok((
                 filename.to_owned(),
                 DirstateEntry {
                     state,
                     mode,
                     size,
                     mtime,
                 },
             ))
         })
         .collect()
 }
 
 /// Create the module, with `__package__` given from parent
 pub fn init_module(py: Python, package: &str) -> PyResult<PyModule> {
     let dotted_name = &format!("{}.dirstate", package);
     let m = PyModule::new(py, dotted_name)?;
 
     m.add(py, "__package__", package)?;
     m.add(py, "__doc__", "Dirstate - Rust implementation")?;
 
     m.add_class::<Dirs>(py)?;
 
     let sys = PyModule::import(py, "sys")?;
     let sys_modules: PyDict = sys.get(py, "modules")?.extract(py)?;
     sys_modules.set_item(py, dotted_name, &m)?;
 
     Ok(m)
 }

rust/hg-cpython/src/dirstate/dirs_multiset.rs

@@ -1,108 +1,118 b''
 // dirs_multiset.rs
 //
 // Copyright 2019 Raphaël Gomès <[e-mail]>
 //
 // This software may be used and distributed according to the terms of the
 // GNU General Public License version 2 or any later version.
 
 //! Bindings for the `hg::dirstate::dirs_multiset` file provided by the
 //! `hg-core` package.
 
 use std::cell::RefCell;
 
 use cpython::{
     exc, ObjectProtocol, PyBytes, PyDict, PyErr, PyObject, PyResult,
     ToPyObject,
 };
 
 use crate::dirstate::extract_dirstate;
-use hg::{DirsIterable, DirsMultiset, DirstateMapError};
+use hg::{
+    DirsIterable, DirsMultiset, DirstateMapError, DirstateParseError,
+    EntryState,
+};
+use std::convert::TryInto;
 
 py_class!(pub class Dirs |py| {
     data dirs_map: RefCell<DirsMultiset>;
 
     // `map` is either a `dict` or a flat iterator (usually a `set`, sometimes
     // a `list`)
     def __new__(
         _cls,
         map: PyObject,
         skip: Option<PyObject> = None
     ) -> PyResult<Self> {
-        let mut skip_state: Option<i8> = None;
+        let mut skip_state: Option<EntryState> = None;
         if let Some(skip) = skip {
-            skip_state = Some(skip.extract::<PyBytes>(py)?.data(py)[0] as i8);
+            skip_state = Some(
+                skip.extract::<PyBytes>(py)?.data(py)[0]
+                    .try_into()
+                    .map_err(|e: DirstateParseError| {
+                        PyErr::new::<exc::ValueError, _>(py, e.to_string())
+                    })?,
+            );
         }
         let inner = if let Ok(map) = map.cast_as::<PyDict>(py) {
             let dirstate = extract_dirstate(py, &map)?;
             DirsMultiset::new(
                 DirsIterable::Dirstate(&dirstate),
                 skip_state,
             )
         } else {
             let map: Result<Vec<Vec<u8>>, PyErr> = map
                 .iter(py)?
                 .map(|o| Ok(o?.extract::<PyBytes>(py)?.data(py).to_owned()))
                 .collect();
             DirsMultiset::new(
                 DirsIterable::Manifest(&map?),
                 skip_state,
             )
         };
 
         Self::create_instance(py, RefCell::new(inner))
     }
 
     def addpath(&self, path: PyObject) -> PyResult<PyObject> {
         self.dirs_map(py).borrow_mut().add_path(
             path.extract::<PyBytes>(py)?.data(py),
         );
         Ok(py.None())
     }
 
     def delpath(&self, path: PyObject) -> PyResult<PyObject> {
         self.dirs_map(py).borrow_mut().delete_path(
             path.extract::<PyBytes>(py)?.data(py),
         )
             .and(Ok(py.None()))
             .or_else(|e| {
                 match e {
                     DirstateMapError::PathNotFound(_p) => {
                         Err(PyErr::new::<exc::ValueError, _>(
                             py,
                             "expected a value, found none".to_string(),
                         ))
                     }
                     DirstateMapError::EmptyPath => {
                         Ok(py.None())
                     }
                 }
             })
     }
 
     // This is really inefficient on top of being ugly, but it's an easy way
     // of having it work to continue working on the rest of the module
     // hopefully bypassing Python entirely pretty soon.
     def __iter__(&self) -> PyResult<PyObject> {
         let dict = PyDict::new(py);
 
         for (key, value) in self.dirs_map(py).borrow().iter() {
             dict.set_item(
                 py,
                 PyBytes::new(py, &key[..]),
                 value.to_py_object(py),
             )?;
         }
 
         let locals = PyDict::new(py);
         locals.set_item(py, "obj", dict)?;
 
         py.eval("iter(obj)", None, Some(&locals))
     }
 
     def __contains__(&self, item: PyObject) -> PyResult<bool> {
         Ok(self
             .dirs_map(py)
             .borrow()
             .contains_key(item.extract::<PyBytes>(py)?.data(py).as_ref()))
     }
 });

rust/hg-cpython/src/parsers.rs

@@ -1,196 +1,207 b''
 // parsers.rs
 //
 // Copyright 2019 Raphaël Gomès <[e-mail]>
 //
 // This software may be used and distributed according to the terms of the
 // GNU General Public License version 2 or any later version.
 
 //! Bindings for the `hg::dirstate::parsers` module provided by the
 //! `hg-core` package.
 //!
 //! From Python, this will be seen as `mercurial.rustext.parsers`
 //!
 use cpython::{
     exc, PyBytes, PyDict, PyErr, PyInt, PyModule, PyResult, PyTuple, Python,
     ToPyObject,
 };
 use hg::{
     pack_dirstate, parse_dirstate, utils::copy_into_array, DirstateEntry,
     DirstatePackError, DirstateParents, DirstateParseError, PARENT_SIZE,
 };
 use std::collections::HashMap;
 
 use libc::c_char;
 
 use crate::dirstate::{decapsule_make_dirstate_tuple, extract_dirstate};
 use std::time::Duration;
 
 fn parse_dirstate_wrapper(
     py: Python,
     dmap: PyDict,
     copymap: PyDict,
     st: PyBytes,
 ) -> PyResult<PyTuple> {
     let mut dirstate_map = HashMap::new();
     let mut copies = HashMap::new();
 
     match parse_dirstate(&mut dirstate_map, &mut copies, st.data(py)) {
         Ok(parents) => {
             for (filename, entry) in dirstate_map {
+                // Explicitly go through u8 first, then cast to
+                // platform-specific `c_char` because Into<u8> has a specific
+                // implementation while `as c_char` would just do a naive enum
+                // cast.
+                let state: u8 = entry.state.into();
+
                 dmap.set_item(
                     py,
                     PyBytes::new(py, &filename),
                     decapsule_make_dirstate_tuple(py)?(
-                        entry.state as c_char,
+                        state as c_char,
                         entry.mode,
                         entry.size,
                         entry.mtime,
                     ),
                 )?;
             }
             for (path, copy_path) in copies {
                 copymap.set_item(
                     py,
                     PyBytes::new(py, &path),
                     PyBytes::new(py, &copy_path),
                 )?;
             }
             Ok(
                 (PyBytes::new(py, &parents.p1), PyBytes::new(py, &parents.p2))
                     .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()
                 }
             },
         )),
     }
 }
 
 fn pack_dirstate_wrapper(
     py: Python,
     dmap: PyDict,
     copymap: PyDict,
     pl: PyTuple,
     now: PyInt,
 ) -> PyResult<PyBytes> {
     let p1 = pl.get_item(py, 0).extract::<PyBytes>(py)?;
     let p1: &[u8] = p1.data(py);
     let p2 = pl.get_item(py, 1).extract::<PyBytes>(py)?;
     let p2: &[u8] = p2.data(py);
 
     let mut dirstate_map = extract_dirstate(py, &dmap)?;
 
     let copies: Result<HashMap<Vec<u8>, Vec<u8>>, PyErr> = copymap
         .items(py)
         .iter()
         .map(|(key, value)| {
             Ok((
                 key.extract::<PyBytes>(py)?.data(py).to_owned(),
                 value.extract::<PyBytes>(py)?.data(py).to_owned(),
             ))
         })
         .collect();
 
     if p1.len() != PARENT_SIZE || p2.len() != PARENT_SIZE {
         return Err(PyErr::new::<exc::ValueError, _>(
             py,
             "expected a 20-byte hash".to_string(),
         ));
     }
 
     match pack_dirstate(
         &mut dirstate_map,
         &copies?,
         DirstateParents {
             p1: copy_into_array(&p1),
             p2: copy_into_array(&p2),
         },
         Duration::from_secs(now.value(py) as u64),
     ) {
         Ok(packed) => {
             for (
                 filename,
                 DirstateEntry {
                     state,
                     mode,
                     size,
                     mtime,
                 },
             ) in dirstate_map
             {
+                // Explicitly go through u8 first, then cast to
+                // platform-specific `c_char` because Into<u8> has a specific
+                // implementation while `as c_char` would just do a naive enum
+                // cast.
+                let state: u8 = state.into();
                 dmap.set_item(
                     py,
                     PyBytes::new(py, &filename[..]),
                     decapsule_make_dirstate_tuple(py)?(
                         state as c_char,
                         mode,
                         size,
                         mtime,
                     ),
                 )?;
             }
             Ok(PyBytes::new(py, &packed))
         }
         Err(error) => Err(PyErr::new::<exc::ValueError, _>(
             py,
             match error {
                 DirstatePackError::CorruptedParent => {
                     "expected a 20-byte hash".to_string()
                 }
                 DirstatePackError::CorruptedEntry(e) => e,
                 DirstatePackError::BadSize(expected, actual) => {
                     format!("bad dirstate size: {} != {}", actual, expected)
                 }
             },
         )),
     }
 }
 
 /// Create the module, with `__package__` given from parent
 pub fn init_parsers_module(py: Python, package: &str) -> PyResult<PyModule> {
     let dotted_name = &format!("{}.parsers", package);
     let m = PyModule::new(py, dotted_name)?;
 
     m.add(py, "__package__", package)?;
     m.add(py, "__doc__", "Parsers - Rust implementation")?;
 
     m.add(
         py,
         "parse_dirstate",
         py_fn!(
             py,
             parse_dirstate_wrapper(dmap: PyDict, copymap: PyDict, st: PyBytes)
         ),
     )?;
     m.add(
         py,
         "pack_dirstate",
         py_fn!(
             py,
             pack_dirstate_wrapper(
                 dmap: PyDict,
                 copymap: PyDict,
                 pl: PyTuple,
                 now: PyInt
             )
         ),
     )?;
 
     let sys = PyModule::import(py, "sys")?;
     let sys_modules: PyDict = sys.get(py, "modules")?.extract(py)?;
     sys_modules.set_item(py, dotted_name, &m)?;
 
     Ok(m)
 }