( )⚙ D6629 rust-dirstate: use EntryState enum instead of literals

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6629

rust-dirstate: use EntryState enum instead of literals
ClosedPublic

Authored by Alphare on Jul 10 2019, 7:46 AM.

Details

Reviewers
kevincox
Group Reviewers
hg-reviewers
Commits
rHG7ceded4419a3: rust-dirstate: use EntryState enum instead of literals
rHG7102ef7dfa31: rust-dirstate: use EntryState enum instead of literals
Summary

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

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Alphare created this revision.Jul 10 2019, 7:46 AM
Alphare updated this revision to Diff 15898.Jul 12 2019, 5:22 AM
kevincox accepted this revision.Jul 15 2019, 12:08 PM
kevincox added inline comments.
rust/hg-core/src/lib.rs
78

I like using {!r} so that the substituted entry is quoted and unambiguous.

rust/hg-cpython/src/parsers.rs
42

If you are going to a comment I would say why. However in this case it probably isn't worth having a comment.

You can also do this on one line entry.state.into::<u8>() as c_char which I think is simple enough.

Alphare added inline comments.Jul 17 2019, 6:29 AM
rust/hg-core/src/lib.rs
78

Did you mean {:?}. I can't find anything related to your syntax.

rust/hg-cpython/src/parsers.rs
42

The one-liner does not compile, so I improved the comment to explain why (which it really should have in the first place).

kevincox added inline comments.Jul 17 2019, 7:35 AM
rust/hg-core/src/lib.rs
78

Sorry yes. I've been writing too much python at work.

Alphare added a commit: rHG7102ef7dfa31: rust-dirstate: use EntryState enum instead of literals.Aug 14 2019, 4:38 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG7102ef7dfa31: rust-dirstate: use EntryState enum instead of literals (authored by Alphare). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 16198

rust/hg-core/src/dirstate.rs

// dirstate module // dirstate module
// //
// Copyright 2019 Raphaël Gomès <rgomes@octobus.net> // Copyright 2019 Raphaël Gomès <rgomes@octobus.net>
// //
// This software may be used and distributed according to the terms of the // This software may be used and distributed according to the terms of the
// GNU General Public License version 2 or any later version. // GNU General Public License version 2 or any later version.
use crate::DirstateParseError;
use std::collections::HashMap; use std::collections::HashMap;
use std::convert::TryFrom;
pub mod dirs_multiset; pub mod dirs_multiset;
pub mod parsers; pub mod parsers;
#[derive(Debug, PartialEq, Clone)] #[derive(Debug, PartialEq, Clone)]
pub struct DirstateParents { pub struct DirstateParents {
pub p1: [u8; 20], pub p1: [u8; 20],
pub p2: [u8; 20], pub p2: [u8; 20],
} }
/// The C implementation uses all signed types. This will be an issue /// The C implementation uses all signed types. This will be an issue
/// either when 4GB+ source files are commonplace or in 2038, whichever /// either when 4GB+ source files are commonplace or in 2038, whichever
/// comes first. /// comes first.
#[derive(Debug, PartialEq, Copy, Clone)] #[derive(Debug, PartialEq, Copy, Clone)]
pub struct DirstateEntry { pub struct DirstateEntry {
pub state: i8, pub state: EntryState,
pub mode: i32, pub mode: i32,
pub mtime: i32, pub mtime: i32,
pub size: i32, pub size: i32,
} }
pub type StateMap = HashMap<Vec<u8>, DirstateEntry>; pub type StateMap = HashMap<Vec<u8>, DirstateEntry>;
pub type CopyMap = HashMap<Vec<u8>, Vec<u8>>; pub type CopyMap = HashMap<Vec<u8>, Vec<u8>>;
/// The Python implementation passes either a mapping (dirstate) or a flat /// The Python implementation passes either a mapping (dirstate) or a flat
/// iterable (manifest) /// iterable (manifest)
pub enum DirsIterable<'a> { pub enum DirsIterable<'a> {
Dirstate(&'a HashMap<Vec<u8>, DirstateEntry>), Dirstate(&'a HashMap<Vec<u8>, DirstateEntry>),
Manifest(&'a Vec<Vec<u8>>), 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

// dirs_multiset.rs // dirs_multiset.rs
// //
// Copyright 2019 Raphaël Gomès <rgomes@octobus.net> // Copyright 2019 Raphaël Gomès <rgomes@octobus.net>
// //
// This software may be used and distributed according to the terms of the // This software may be used and distributed according to the terms of the
// GNU General Public License version 2 or any later version. // GNU General Public License version 2 or any later version.
//! A multiset of directory names. //! A multiset of directory names.
//! //!
//! Used to counts the references to directories in a manifest or dirstate. //! 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::hash_map::{Entry, Iter};
use std::collections::HashMap; use std::collections::HashMap;
#[derive(PartialEq, Debug)] #[derive(PartialEq, Debug)]
pub struct DirsMultiset { pub struct DirsMultiset {
inner: HashMap<Vec<u8>, u32>, inner: HashMap<Vec<u8>, u32>,
} }
impl DirsMultiset { impl DirsMultiset {
/// Initializes the multiset from a dirstate or a manifest. /// Initializes the multiset from a dirstate or a manifest.
/// ///
/// If `skip_state` is provided, skips dirstate entries with equal state. /// 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 { let mut multiset = DirsMultiset {
inner: HashMap::new(), inner: HashMap::new(),
}; };
match iterable { match iterable {
DirsIterable::Dirstate(vec) => { DirsIterable::Dirstate(vec) => {
for (filename, DirstateEntry { state, .. }) in vec { for (filename, DirstateEntry { state, .. }) in vec {
// This `if` is optimized out of the loop // This `if` is optimized out of the loop
assert_eq!(expected, new); assert_eq!(expected, new);
let input_map = ["a/", "b/", "a/c", "a/d/"] let input_map = ["a/", "b/", "a/c", "a/d/"]
.iter() .iter()
.map(|f| { .map(|f| {
( (
f.as_bytes().to_vec(), f.as_bytes().to_vec(),
DirstateEntry { DirstateEntry {
state: 0, state: EntryState::Normal,
mode: 0, mode: 0,
mtime: 0, mtime: 0,
size: 0, size: 0,
}, },
) )
}) })
.collect(); .collect();
let expected_inner = [("", 2), ("a", 3), ("b", 1), ("a/d", 1)] let expected_inner = [("", 2), ("a", 3), ("b", 1), ("a/d", 1)]
.iter() .iter()
.map(|e| e.as_bytes().to_vec()) .map(|e| e.as_bytes().to_vec())
.collect(); .collect();
let expected_inner = [("", 2), ("a", 3), ("b", 1), ("a/d", 1)] let expected_inner = [("", 2), ("a", 3), ("b", 1), ("a/d", 1)]
.iter() .iter()
.map(|(k, v)| (k.as_bytes().to_vec(), *v)) .map(|(k, v)| (k.as_bytes().to_vec(), *v))
.collect(); .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 { let expected = DirsMultiset {
inner: expected_inner, inner: expected_inner,
}; };
// Skip does not affect a manifest // Skip does not affect a manifest
assert_eq!(expected, new); assert_eq!(expected, new);
let input_map = let input_map = [
[("a/", 'n'), ("a/b/", 'n'), ("a/c", 'r'), ("a/d/", 'm')] ("a/", EntryState::Normal),
("a/b/", EntryState::Normal),
("a/c", EntryState::Removed),
("a/d/", EntryState::Merged),
]
.iter() .iter()
.map(|(f, state)| { .map(|(f, state)| {
( (
f.as_bytes().to_vec(), f.as_bytes().to_vec(),
DirstateEntry { DirstateEntry {
state: *state as i8, state: *state,
mode: 0, mode: 0,
mtime: 0, mtime: 0,
size: 0, size: 0,
}, },
) )
}) })
.collect(); .collect();
// "a" incremented with "a/c" and "a/d/" // "a" incremented with "a/c" and "a/d/"
let expected_inner = [("", 1), ("a", 2), ("a/d", 1)] let expected_inner = [("", 1), ("a", 2), ("a/d", 1)]
.iter() .iter()
.map(|(k, v)| (k.as_bytes().to_vec(), *v)) .map(|(k, v)| (k.as_bytes().to_vec(), *v))
.collect(); .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 { let expected = DirsMultiset {
inner: expected_inner, inner: expected_inner,
}; };
assert_eq!(expected, new); assert_eq!(expected, new);
} }
} }

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

// Copyright 2019 Raphaël Gomès <rgomes@octobus.net> // Copyright 2019 Raphaël Gomès <rgomes@octobus.net>
// //
// This software may be used and distributed according to the terms of the // This software may be used and distributed according to the terms of the
// GNU General Public License version 2 or any later version. // GNU General Public License version 2 or any later version.
use crate::{ use crate::{
dirstate::{CopyMap, StateMap}, dirstate::{CopyMap, EntryState, StateMap},
utils::copy_into_array, utils::copy_into_array,
DirstateEntry, DirstatePackError, DirstateParents, DirstateParseError, DirstateEntry, DirstatePackError, DirstateParents, DirstateParseError,
}; };
use byteorder::{BigEndian, ReadBytesExt, WriteBytesExt}; use byteorder::{BigEndian, ReadBytesExt, WriteBytesExt};
use std::convert::TryInto; use std::convert::{TryFrom, TryInto};
use std::io::Cursor; use std::io::Cursor;
use std::time::Duration; use std::time::Duration;
/// Parents are stored in the dirstate as byte hashes. /// Parents are stored in the dirstate as byte hashes.
pub const PARENT_SIZE: usize = 20; pub const PARENT_SIZE: usize = 20;
/// Dirstate entries have a static part of 8 + 32 + 32 + 32 + 32 bits. /// Dirstate entries have a static part of 8 + 32 + 32 + 32 + 32 bits.
const MIN_ENTRY_SIZE: usize = 17; const MIN_ENTRY_SIZE: usize = 17;
while curr_pos < contents.len() { while curr_pos < contents.len() {
if curr_pos + MIN_ENTRY_SIZE > contents.len() { if curr_pos + MIN_ENTRY_SIZE > contents.len() {
return Err(DirstateParseError::Overflow); return Err(DirstateParseError::Overflow);
} }
let entry_bytes = &contents[curr_pos..]; let entry_bytes = &contents[curr_pos..];
let mut cursor = Cursor::new(entry_bytes); 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 mode = cursor.read_i32::<BigEndian>()?;
let size = cursor.read_i32::<BigEndian>()?; let size = cursor.read_i32::<BigEndian>()?;
let mtime = cursor.read_i32::<BigEndian>()?; let mtime = cursor.read_i32::<BigEndian>()?;
let path_len = cursor.read_i32::<BigEndian>()? as usize; let path_len = cursor.read_i32::<BigEndian>()? as usize;
if path_len > contents.len() - curr_pos { if path_len > contents.len() - curr_pos {
return Err(DirstateParseError::Overflow); return Err(DirstateParseError::Overflow);
} }
let mut new_state_map = vec![]; let mut new_state_map = vec![];
packed.extend(&parents.p1); packed.extend(&parents.p1);
packed.extend(&parents.p2); packed.extend(&parents.p2);
for (ref filename, entry) in state_map.iter() { for (ref filename, entry) in state_map.iter() {
let mut new_filename: Vec<u8> = filename.to_vec(); let mut new_filename: Vec<u8> = filename.to_vec();
let mut new_mtime: i32 = entry.mtime; 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 // The file was last modified "simultaneously" with the current
// write to dirstate (i.e. within the same second for file- // write to dirstate (i.e. within the same second for file-
// systems with a granularity of 1 sec). This commonly happens // systems with a granularity of 1 sec). This commonly happens
// for at least a couple of files on 'update'. // for at least a couple of files on 'update'.
// The user could change the file without changing its size // The user could change the file without changing its size
// within the same second. Invalidate the file's mtime in // within the same second. Invalidate the file's mtime in
// dirstate, forcing future 'status' calls to compare the // dirstate, forcing future 'status' calls to compare the
// contents of the file if the size is the same. This prevents // contents of the file if the size is the same. This prevents
// mistakenly treating such files as clean. // mistakenly treating such files as clean.
new_mtime = -1; new_mtime = -1;
new_state_map.push(( new_state_map.push((
filename.to_owned().to_vec(), filename.to_owned().to_vec(),
DirstateEntry { DirstateEntry {
mtime: new_mtime, mtime: new_mtime,
..*entry ..*entry
}, },
)); ));
} }
if let Some(copy) = copy_map.get(*filename) { if let Some(copy) = copy_map.get(*filename) {
new_filename.push('\0' as u8); new_filename.push('\0' as u8);
new_filename.extend(copy); 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.mode)?;
packed.write_i32::<BigEndian>(entry.size)?; packed.write_i32::<BigEndian>(entry.size)?;
packed.write_i32::<BigEndian>(new_mtime)?; packed.write_i32::<BigEndian>(new_mtime)?;
packed.write_i32::<BigEndian>(new_filename.len() as i32)?; packed.write_i32::<BigEndian>(new_filename.len() as i32)?;
packed.extend(new_filename) packed.extend(new_filename)
} }
if packed.len() != expected_size { if packed.len() != expected_size {
return Err(DirstatePackError::BadSize(expected_size, packed.len())); return Err(DirstatePackError::BadSize(expected_size, packed.len()));
} }
state_map.extend(new_state_map); state_map.extend(new_state_map);
Ok(packed) Ok(packed)
} }
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
use std::collections::HashMap; use std::collections::HashMap;
#[test] #[test]
fn test_pack_dirstate_empty() { fn test_pack_dirstate_empty() {
let mut state_map: StateMap = HashMap::new(); let mut state_map: StateMap = HashMap::new();
assert!(state_map.is_empty()) assert!(state_map.is_empty())
} }
#[test] #[test]
fn test_pack_dirstate_one_entry() { fn test_pack_dirstate_one_entry() {
let expected_state_map: StateMap = [( let expected_state_map: StateMap = [(
b"f1".to_vec(), b"f1".to_vec(),
DirstateEntry { DirstateEntry {
state: 'n' as i8, state: EntryState::Normal,
mode: 0o644, mode: 0o644,
size: 0, size: 0,
mtime: 791231220, mtime: 791231220,
}, },
)] )]
.iter() .iter()
.cloned() .cloned()
.collect(); .collect();
assert_eq!(expected_state_map, state_map); assert_eq!(expected_state_map, state_map);
} }
#[test] #[test]
fn test_pack_dirstate_one_entry_with_copy() { fn test_pack_dirstate_one_entry_with_copy() {
let expected_state_map: StateMap = [( let expected_state_map: StateMap = [(
b"f1".to_vec(), b"f1".to_vec(),
DirstateEntry { DirstateEntry {
state: 'n' as i8, state: EntryState::Normal,
mode: 0o644, mode: 0o644,
size: 0, size: 0,
mtime: 791231220, mtime: 791231220,
}, },
)] )]
.iter() .iter()
.cloned() .cloned()
.collect(); .collect();
assert_eq!(expected_state_map, state_map); assert_eq!(expected_state_map, state_map);
} }
#[test] #[test]
fn test_parse_pack_one_entry_with_copy() { fn test_parse_pack_one_entry_with_copy() {
let mut state_map: StateMap = [( let mut state_map: StateMap = [(
b"f1".to_vec(), b"f1".to_vec(),
DirstateEntry { DirstateEntry {
state: 'n' as i8, state: EntryState::Normal,
mode: 0o644, mode: 0o644,
size: 0, size: 0,
mtime: 791231220, mtime: 791231220,
}, },
)] )]
.iter() .iter()
.cloned() .cloned()
.collect(); .collect();
} }
#[test] #[test]
fn test_parse_pack_multiple_entries_with_copy() { fn test_parse_pack_multiple_entries_with_copy() {
let mut state_map: StateMap = [ let mut state_map: StateMap = [
( (
b"f1".to_vec(), b"f1".to_vec(),
DirstateEntry { DirstateEntry {
state: 'n' as i8, state: EntryState::Normal,
mode: 0o644, mode: 0o644,
size: 0, size: 0,
mtime: 791231220, mtime: 791231220,
}, },
), ),
( (
b"f2".to_vec(), b"f2".to_vec(),
DirstateEntry { DirstateEntry {
state: 'm' as i8, state: EntryState::Merged,
mode: 0o777, mode: 0o777,
size: 1000, size: 1000,
mtime: 791231220, mtime: 791231220,
}, },
), ),
( (
b"f3".to_vec(), b"f3".to_vec(),
DirstateEntry { DirstateEntry {
state: 'r' as i8, state: EntryState::Removed,
mode: 0o644, mode: 0o644,
size: 234553, size: 234553,
mtime: 791231220, mtime: 791231220,
}, },
), ),
( (
b"f4\xF6".to_vec(), b"f4\xF6".to_vec(),
DirstateEntry { DirstateEntry {
state: 'a' as i8, state: EntryState::Added,
mode: 0o644, mode: 0o644,
size: -1, size: -1,
mtime: -1, mtime: -1,
}, },
), ),
] ]
.iter() .iter()
.cloned() .cloned()
} }
#[test] #[test]
/// https://www.mercurial-scm.org/repo/hg/rev/af3f26b6bba4 /// https://www.mercurial-scm.org/repo/hg/rev/af3f26b6bba4
fn test_parse_pack_one_entry_with_copy_and_time_conflict() { fn test_parse_pack_one_entry_with_copy_and_time_conflict() {
let mut state_map: StateMap = [( let mut state_map: StateMap = [(
b"f1".to_vec(), b"f1".to_vec(),
DirstateEntry { DirstateEntry {
state: 'n' as i8, state: EntryState::Normal,
mode: 0o644, mode: 0o644,
size: 0, size: 0,
mtime: 15000000, mtime: 15000000,
}, },
)] )]
.iter() .iter()
.cloned() .cloned()
.collect(); .collect();
.unwrap(); .unwrap();
assert_eq!( assert_eq!(
( (
parents, parents,
[( [(
b"f1".to_vec(), b"f1".to_vec(),
DirstateEntry { DirstateEntry {
state: 'n' as i8, state: EntryState::Normal,
mode: 0o644, mode: 0o644,
size: 0, size: 0,
mtime: -1 mtime: -1
} }
)] )]
.iter() .iter()
.cloned() .cloned()
.collect::<StateMap>(), .collect::<StateMap>(),
copymap, copymap,
), ),
(new_parents, new_state_map, new_copy_map) (new_parents, new_state_map, new_copy_map)
) )
} }
} }

rust/hg-core/src/lib.rs

// Copyright 2018 Georges Racinet <gracinet@anybox.fr> // Copyright 2018 Georges Racinet <gracinet@anybox.fr>
// //
// This software may be used and distributed according to the terms of the // This software may be used and distributed according to the terms of the
// GNU General Public License version 2 or any later version. // GNU General Public License version 2 or any later version.
mod ancestors; mod ancestors;
pub mod dagops; pub mod dagops;
pub use ancestors::{AncestorsIterator, LazyAncestors, MissingAncestors}; pub use ancestors::{AncestorsIterator, LazyAncestors, MissingAncestors};
mod dirstate; mod dirstate;
pub mod discovery; pub mod discovery;
pub mod testing; // unconditionally built, for use from integration tests pub mod testing; // unconditionally built, for use from integration tests
pub use dirstate::{ pub use dirstate::{
dirs_multiset::DirsMultiset, dirs_multiset::DirsMultiset,
parsers::{pack_dirstate, parse_dirstate, PARENT_SIZE}, parsers::{pack_dirstate, parse_dirstate, PARENT_SIZE},
CopyMap, DirsIterable, DirstateEntry, DirstateParents, StateMap, CopyMap, DirsIterable, DirstateEntry, DirstateParents, EntryState,
StateMap,
}; };
mod filepatterns; mod filepatterns;
pub mod utils; pub mod utils;
pub use filepatterns::{ pub use filepatterns::{
build_single_regex, read_pattern_file, PatternSyntax, PatternTuple, build_single_regex, read_pattern_file, PatternSyntax, PatternTuple,
}; };
#[derive(Clone, Debug, PartialEq)] #[derive(Clone, Debug, PartialEq)]
pub enum DirstateParseError { pub enum DirstateParseError {
TooLittleData, TooLittleData,
Overflow, Overflow,
CorruptedEntry(String), CorruptedEntry(String),
Damaged, 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),
kevincoxUnsubmitted
Not Done

I like using {!r} so that the substituted entry is quoted and unambiguous.

kevincox: I like using `{!r}` so that the substituted entry is quoted and unambiguous.
AlphareAuthorUnsubmitted
Done

Did you mean {:?}. I can't find anything related to your syntax.

Alphare: Did you mean `{:?}`. I can't find anything related to your syntax.
kevincoxUnsubmitted
Not Done

Sorry yes. I've been writing too much python at work.

kevincox: Sorry yes. I've been writing too much python at work.
Damaged => "Dirstate appears to be damaged.".to_string(),
}
}
}
#[derive(Debug, PartialEq)] #[derive(Debug, PartialEq)]
pub enum DirstatePackError { pub enum DirstatePackError {
CorruptedEntry(String), CorruptedEntry(String),
CorruptedParent, CorruptedParent,
BadSize(usize, usize), 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)] #[derive(Debug, PartialEq)]
pub enum DirstateMapError { pub enum DirstateMapError {
PathNotFound(Vec<u8>), PathNotFound(Vec<u8>),
EmptyPath, EmptyPath,
} }
impl From<std::io::Error> for DirstatePackError { pub enum DirstateError {
fn from(e: std::io::Error) -> Self { Parse(DirstateParseError),
DirstatePackError::CorruptedEntry(e.to_string()) 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 { impl From<DirstatePackError> for DirstateError {
fn from(e: std::io::Error) -> Self { fn from(e: DirstatePackError) -> Self {
DirstateParseError::CorruptedEntry(e.to_string()) DirstateError::Pack(e)
} }
} }
#[derive(Debug)] #[derive(Debug)]
pub enum PatternError { pub enum PatternError {
UnsupportedSyntax(String), UnsupportedSyntax(String),
} }
#[derive(Debug)] #[derive(Debug)]
pub enum PatternFileError { pub enum PatternFileError {
IO(std::io::Error), IO(std::io::Error),
Pattern(PatternError, LineNumber), Pattern(PatternError, LineNumber),
} }
impl From<std::io::Error> for PatternFileError { impl From<std::io::Error> for PatternFileError {
fn from(e: std::io::Error) -> Self { fn from(e: std::io::Error) -> Self {
PatternFileError::IO(e) 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

// dirstate.rs // dirstate.rs
// //
// Copyright 2019 Raphaël Gomès <rgomes@octobus.net> // Copyright 2019 Raphaël Gomès <rgomes@octobus.net>
// //
// This software may be used and distributed according to the terms of the // This software may be used and distributed according to the terms of the
// GNU General Public License version 2 or any later version. // GNU General Public License version 2 or any later version.
//! Bindings for the `hg::dirstate` module provided by the //! Bindings for the `hg::dirstate` module provided by the
//! `hg-core` package. //! `hg-core` package.
//! //!
//! From Python, this will be seen as `mercurial.rustext.dirstate` //! From Python, this will be seen as `mercurial.rustext.dirstate`
mod dirs_multiset; mod dirs_multiset;
use crate::dirstate::dirs_multiset::Dirs; use crate::dirstate::dirs_multiset::Dirs;
use cpython::{ 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}; use libc::{c_char, c_int};
#[cfg(feature = "python27")] #[cfg(feature = "python27")]
use python27_sys::PyCapsule_Import; use python27_sys::PyCapsule_Import;
#[cfg(feature = "python3")] #[cfg(feature = "python3")]
use python3_sys::PyCapsule_Import; use python3_sys::PyCapsule_Import;
use std::convert::TryFrom;
use std::ffi::CStr; use std::ffi::CStr;
use std::mem::transmute; use std::mem::transmute;
/// C code uses a custom `dirstate_tuple` type, checks in multiple instances /// 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. /// 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 /// 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 /// 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, /// for a pure Python tuple of the same effective structure to be used,
} }
pub fn extract_dirstate(py: Python, dmap: &PyDict) -> Result<StateMap, PyErr> { pub fn extract_dirstate(py: Python, dmap: &PyDict) -> Result<StateMap, PyErr> {
dmap.items(py) dmap.items(py)
.iter() .iter()
.map(|(filename, stats)| { .map(|(filename, stats)| {
let stats = stats.extract::<PySequence>(py)?; let stats = stats.extract::<PySequence>(py)?;
let state = stats.get_item(py, 0)?.extract::<PyBytes>(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 mode = stats.get_item(py, 1)?.extract(py)?;
let size = stats.get_item(py, 2)?.extract(py)?; let size = stats.get_item(py, 2)?.extract(py)?;
let mtime = stats.get_item(py, 3)?.extract(py)?; let mtime = stats.get_item(py, 3)?.extract(py)?;
let filename = filename.extract::<PyBytes>(py)?; let filename = filename.extract::<PyBytes>(py)?;
let filename = filename.data(py); let filename = filename.data(py);
Ok(( Ok((
filename.to_owned(), filename.to_owned(),
DirstateEntry { DirstateEntry {

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

use std::cell::RefCell; use std::cell::RefCell;
use cpython::{ use cpython::{
exc, ObjectProtocol, PyBytes, PyDict, PyErr, PyObject, PyResult, exc, ObjectProtocol, PyBytes, PyDict, PyErr, PyObject, PyResult,
ToPyObject, ToPyObject,
}; };
use crate::dirstate::extract_dirstate; 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| { py_class!(pub class Dirs |py| {
data dirs_map: RefCell<DirsMultiset>; data dirs_map: RefCell<DirsMultiset>;
// `map` is either a `dict` or a flat iterator (usually a `set`, sometimes // `map` is either a `dict` or a flat iterator (usually a `set`, sometimes
// a `list`) // a `list`)
def __new__( def __new__(
_cls, _cls,
map: PyObject, map: PyObject,
skip: Option<PyObject> = None skip: Option<PyObject> = None
) -> PyResult<Self> { ) -> PyResult<Self> {
let mut skip_state: Option<i8> = None; let mut skip_state: Option<EntryState> = None;
if let Some(skip) = skip { 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 inner = if let Ok(map) = map.cast_as::<PyDict>(py) {
let dirstate = extract_dirstate(py, &map)?; let dirstate = extract_dirstate(py, &map)?;
DirsMultiset::new( DirsMultiset::new(
DirsIterable::Dirstate(&dirstate), DirsIterable::Dirstate(&dirstate),
skip_state, skip_state,
) )
} else { } else {

rust/hg-cpython/src/parsers.rs

st: PyBytes, st: PyBytes,
) -> PyResult<PyTuple> { ) -> PyResult<PyTuple> {
let mut dirstate_map = HashMap::new(); let mut dirstate_map = HashMap::new();
let mut copies = HashMap::new(); let mut copies = HashMap::new();
match parse_dirstate(&mut dirstate_map, &mut copies, st.data(py)) { match parse_dirstate(&mut dirstate_map, &mut copies, st.data(py)) {
Ok(parents) => { Ok(parents) => {
for (filename, entry) in dirstate_map { 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
kevincoxUnsubmitted
Not Done

If you are going to a comment I would say why. However in this case it probably isn't worth having a comment.

You can also do this on one line entry.state.into::<u8>() as c_char which I think is simple enough.

kevincox: If you are going to a comment I would say why. However in this case it probably isn't worth…
AlphareAuthorUnsubmitted
Done

The one-liner does not compile, so I improved the comment to explain why (which it really should have in the first place).

Alphare: The one-liner does not compile, so I improved the comment to explain why (which it really…
// cast.
let state: u8 = entry.state.into();
dmap.set_item( dmap.set_item(
py, py,
PyBytes::new(py, &filename), PyBytes::new(py, &filename),
decapsule_make_dirstate_tuple(py)?( decapsule_make_dirstate_tuple(py)?(
entry.state as c_char, state as c_char,
entry.mode, entry.mode,
entry.size, entry.size,
entry.mtime, entry.mtime,
), ),
)?; )?;
} }
for (path, copy_path) in copies { for (path, copy_path) in copies {
copymap.set_item( copymap.set_item(
DirstateEntry { DirstateEntry {
state, state,
mode, mode,
size, size,
mtime, mtime,
}, },
) in dirstate_map ) 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( dmap.set_item(
py, py,
PyBytes::new(py, &filename[..]), PyBytes::new(py, &filename[..]),
decapsule_make_dirstate_tuple(py)?( decapsule_make_dirstate_tuple(py)?(
state as c_char, state as c_char,
mode, mode,
size, size,
mtime, mtime,