D7522: rust-dirs: handle forgotten `Result`s
Alphare (Raphaël Gomès)
phabricator at mercurial-scm.org
Wed Nov 27 13:30:34 UTC 2019
Alphare created this revision.
Herald added subscribers: mercurial-devel, kevincox, durin42.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
In 20a3bf5e71d6 <https://phab.mercurial-scm.org/rHG20a3bf5e71d669799f42122d7afb5d09c37b6835> I introduced a temporary bugfix to align Rust code with a new
behavior from C/Python and forgot about a few `Result`s (cargo's compiler cache
does not re-emit warnings on cached modules). This fixes it.
For the record, I am still unsure that this behavior change is a good idea.
Note: I was already quite unhappy with the setters and getters for the
`DirstateMap` and, indirectly, `Dirs`, and this only further reinforces my
feelings. I hope we can one day fix that situation at the type level; Georges
Racinet and I were just talking about devising a POC for using the builder
pattern in the context of FFI with Python, we'll see what comes out of it.
REPOSITORY
rHG Mercurial
BRANCH
default
REVISION DETAIL
https://phab.mercurial-scm.org/D7522
AFFECTED FILES
rust/hg-core/src/dirstate/dirs_multiset.rs
rust/hg-core/src/dirstate/dirstate_map.rs
rust/hg-cpython/src/dirstate/dirs_multiset.rs
rust/hg-cpython/src/dirstate/dirstate_map.rs
CHANGE DETAILS
diff --git a/rust/hg-cpython/src/dirstate/dirstate_map.rs b/rust/hg-cpython/src/dirstate/dirstate_map.rs
--- a/rust/hg-cpython/src/dirstate/dirstate_map.rs
+++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs
@@ -200,6 +200,9 @@
let d = d.extract::<PyBytes>(py)?;
Ok(self.inner_shared(py).borrow_mut()?
.has_tracked_dir(HgPath::new(d.data(py)))
+ .map_err(|e| {
+ PyErr::new::<exc::ValueError, _>(py, e.to_string())
+ })?
.to_py_object(py))
}
@@ -207,6 +210,9 @@
let d = d.extract::<PyBytes>(py)?;
Ok(self.inner_shared(py).borrow_mut()?
.has_dir(HgPath::new(d.data(py)))
+ .map_err(|e| {
+ PyErr::new::<exc::ValueError, _>(py, e.to_string())
+ })?
.to_py_object(py))
}
@@ -330,24 +336,35 @@
def getdirs(&self) -> PyResult<Dirs> {
// TODO don't copy, share the reference
- self.inner_shared(py).borrow_mut()?.set_dirs();
+ self.inner_shared(py).borrow_mut()?.set_dirs()
+ .map_err(|e| {
+ PyErr::new::<exc::ValueError, _>(py, e.to_string())
+ })?;
Dirs::from_inner(
py,
DirsMultiset::from_dirstate(
&self.inner_shared(py).borrow(),
Some(EntryState::Removed),
- ),
+ )
+ .map_err(|e| {
+ PyErr::new::<exc::ValueError, _>(py, e.to_string())
+ })?,
)
}
def getalldirs(&self) -> PyResult<Dirs> {
// TODO don't copy, share the reference
- self.inner_shared(py).borrow_mut()?.set_all_dirs();
+ self.inner_shared(py).borrow_mut()?.set_all_dirs()
+ .map_err(|e| {
+ PyErr::new::<exc::ValueError, _>(py, e.to_string())
+ })?;
Dirs::from_inner(
py,
DirsMultiset::from_dirstate(
&self.inner_shared(py).borrow(),
None,
- ),
+ ).map_err(|e| {
+ PyErr::new::<exc::ValueError, _>(py, e.to_string())
+ })?,
)
}
diff --git a/rust/hg-cpython/src/dirstate/dirs_multiset.rs b/rust/hg-cpython/src/dirstate/dirs_multiset.rs
--- a/rust/hg-cpython/src/dirstate/dirs_multiset.rs
+++ b/rust/hg-cpython/src/dirstate/dirs_multiset.rs
@@ -47,6 +47,9 @@
let inner = if let Ok(map) = map.cast_as::<PyDict>(py) {
let dirstate = extract_dirstate(py, &map)?;
DirsMultiset::from_dirstate(&dirstate, skip_state)
+ .map_err(|e| {
+ PyErr::new::<exc::ValueError, _>(py, e.to_string())
+ })?
} else {
let map: Result<Vec<HgPathBuf>, PyErr> = map
.iter(py)?
@@ -57,6 +60,9 @@
})
.collect();
DirsMultiset::from_manifest(&map?)
+ .map_err(|e| {
+ PyErr::new::<exc::ValueError, _>(py, e.to_string())
+ })?
};
Self::create_instance(
diff --git a/rust/hg-core/src/dirstate/dirstate_map.rs b/rust/hg-core/src/dirstate/dirstate_map.rs
--- a/rust/hg-core/src/dirstate/dirstate_map.rs
+++ b/rust/hg-core/src/dirstate/dirstate_map.rs
@@ -126,7 +126,7 @@
}
if old_state == EntryState::Unknown {
if let Some(ref mut all_dirs) = self.all_dirs {
- all_dirs.add_path(filename);
+ all_dirs.add_path(filename)?;
}
}
@@ -227,30 +227,32 @@
/// emulate a Python lazy property, but it is ugly and unidiomatic.
/// TODO One day, rewriting this struct using the typestate might be a
/// good idea.
- pub fn set_all_dirs(&mut self) {
+ pub fn set_all_dirs(&mut self) -> Result<(), DirstateMapError> {
if self.all_dirs.is_none() {
self.all_dirs =
- Some(DirsMultiset::from_dirstate(&self.state_map, None));
+ Some(DirsMultiset::from_dirstate(&self.state_map, None)?);
}
+ Ok(())
}
- pub fn set_dirs(&mut self) {
+ pub fn set_dirs(&mut self) -> Result<(), DirstateMapError> {
if self.dirs.is_none() {
self.dirs = Some(DirsMultiset::from_dirstate(
&self.state_map,
Some(EntryState::Removed),
- ));
+ )?);
}
+ Ok(())
}
- pub fn has_tracked_dir(&mut self, directory: &HgPath) -> bool {
- self.set_dirs();
- self.dirs.as_ref().unwrap().contains(directory)
+ pub fn has_tracked_dir(&mut self, directory: &HgPath) -> Result<bool, DirstateMapError> {
+ self.set_dirs()?;
+ Ok(self.dirs.as_ref().unwrap().contains(directory))
}
- pub fn has_dir(&mut self, directory: &HgPath) -> bool {
- self.set_all_dirs();
- self.all_dirs.as_ref().unwrap().contains(directory)
+ pub fn has_dir(&mut self, directory: &HgPath) -> Result<bool, DirstateMapError> {
+ self.set_all_dirs()?;
+ Ok(self.all_dirs.as_ref().unwrap().contains(directory))
}
pub fn parents(
diff --git a/rust/hg-core/src/dirstate/dirs_multiset.rs b/rust/hg-core/src/dirstate/dirs_multiset.rs
--- a/rust/hg-core/src/dirstate/dirs_multiset.rs
+++ b/rust/hg-core/src/dirstate/dirs_multiset.rs
@@ -30,7 +30,7 @@
pub fn from_dirstate(
vec: &HashMap<HgPathBuf, DirstateEntry>,
skip_state: Option<EntryState>,
- ) -> Self {
+ ) -> Result<Self, DirstateMapError> {
let mut multiset = DirsMultiset {
inner: HashMap::new(),
};
@@ -39,27 +39,27 @@
// This `if` is optimized out of the loop
if let Some(skip) = skip_state {
if skip != *state {
- multiset.add_path(filename);
+ multiset.add_path(filename)?;
}
} else {
- multiset.add_path(filename);
+ multiset.add_path(filename)?;
}
}
- multiset
+ Ok(multiset)
}
/// Initializes the multiset from a manifest.
- pub fn from_manifest(vec: &Vec<HgPathBuf>) -> Self {
+ pub fn from_manifest(vec: &Vec<HgPathBuf>) -> Result<Self, DirstateMapError> {
let mut multiset = DirsMultiset {
inner: HashMap::new(),
};
for filename in vec {
- multiset.add_path(filename);
+ multiset.add_path(filename)?;
}
- multiset
+ Ok(multiset)
}
/// Increases the count of deepest directory contained in the path.
To: Alphare, #hg-reviewers
Cc: durin42, kevincox, mercurial-devel
More information about the Mercurial-devel
mailing list