( )⚙ 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
Lint Skipped
Unit
Unit Tests Skipped

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
85

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
85

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
85

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 15842

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 dirstate::EntryState;
use std::collections::hash_map::{Entry, Iter}; use std::collections::hash_map::{Entry, Iter};
use std::collections::HashMap; use std::collections::HashMap;
use {DirsIterable, DirstateEntry, DirstateMapError}; use {DirsIterable, DirstateEntry, DirstateMapError};
#[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
pub fn len(&self) -> usize { pub fn len(&self) -> usize {
self.inner.len() self.inner.len()
} }
} }
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
use dirstate::EntryState;
use std::collections::HashMap; use std::collections::HashMap;
#[test] #[test]
fn test_delete_path_path_not_found() { fn test_delete_path_path_not_found() {
let mut map = DirsMultiset::new(DirsIterable::Manifest(&vec![]), None); let mut map = DirsMultiset::new(DirsIterable::Manifest(&vec![]), None);
let path = b"doesnotexist/"; let path = b"doesnotexist/";
assert_eq!( assert_eq!(
Err(DirstateMapError::PathNotFound(path.to_vec())), Err(DirstateMapError::PathNotFound(path.to_vec())),
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/mod.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 std::collections::HashMap; use std::collections::HashMap;
use std::convert::TryFrom;
use DirstateParseError;
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: Vec<u8>, pub p1: Vec<u8>,
pub p2: Vec<u8>, pub p2: Vec<u8>,
} }
/// 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/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 byteorder::{BigEndian, ReadBytesExt, WriteBytesExt}; use byteorder::{BigEndian, ReadBytesExt, WriteBytesExt};
use dirstate::{CopyMap, StateMap}; use dirstate::EntryState;
use std::convert::TryFrom;
use std::io::Cursor; use std::io::Cursor;
use std::time::Duration; use std::time::Duration;
use {DirstateEntry, DirstatePackError, DirstateParents, DirstateParseError}; use {
CopyMap, DirstateEntry, DirstatePackError, DirstateParents,
DirstateParseError, StateMap,
};
/// Parents are stored in the dirstate as byte hashes. /// Parents are stored in the dirstate as byte hashes.
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;
// TODO parse/pack: is mutate-on-loop better for performance? // TODO parse/pack: is mutate-on-loop better for performance?
pub fn parse_dirstate( pub fn parse_dirstate(
state_map: &mut StateMap, state_map: &mut StateMap,
copy_map: &mut CopyMap, copy_map: &mut CopyMap,
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.
extern crate byteorder; extern crate byteorder;
extern crate memchr; extern crate memchr;
#[macro_use] #[macro_use]
extern crate lazy_static; extern crate lazy_static;
extern crate core;
extern crate regex; extern crate regex;
mod ancestors; mod ancestors;
pub mod dagops; pub mod dagops;
pub mod utils;
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}, parsers::{pack_dirstate, parse_dirstate},
CopyMap, DirsIterable, DirstateEntry, DirstateParents, StateMap, CopyMap, DirsIterable, DirstateEntry, DirstateParents, EntryState,
StateMap,
}; };
mod filepatterns; mod filepatterns;
mod utils;
pub use filepatterns::{ pub use filepatterns::{
build_single_regex, read_pattern_file, PatternSyntax, PatternTuple, build_single_regex, read_pattern_file, PatternSyntax, PatternTuple,
}; };
/// Mercurial revision numbers /// Mercurial revision numbers
/// ///
/// As noted in revlog.c, revision numbers are actually encoded in /// As noted in revlog.c, revision numbers are actually encoded in
#[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 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/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 dirstate::extract_dirstate; use 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/dirstate/mod.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`
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 std::ffi::CStr; use std::ffi::CStr;
#[cfg(feature = "python27")] #[cfg(feature = "python27")]
extern crate python27_sys as python_sys; extern crate python27_sys as python_sys;
#[cfg(feature = "python3")] #[cfg(feature = "python3")]
extern crate python3_sys as python_sys; extern crate python3_sys as python_sys;
use self::python_sys::PyCapsule_Import; use self::python_sys::PyCapsule_Import;
use libc::{c_char, c_int}; use libc::{c_char, c_int};
use std::mem::transmute; use std::mem::transmute;
mod dirs_multiset; mod dirs_multiset;
use dirstate::dirs_multiset::Dirs; use dirstate::dirs_multiset::Dirs;
use std::convert::TryFrom;
/// 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,
/// rendering this type and the capsule below useless. /// rendering this type and the capsule below useless.
type MakeDirstateTupleFn = extern "C" fn( type MakeDirstateTupleFn = extern "C" fn(
} }
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/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`.
let state: u8 = entry.state.into();
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…
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`.
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,