Patchwork D6233: rust-discovery: implementing and exposing stats()

login
register
mail settings
Submitter phabricator
Date April 12, 2019, 6:34 p.m.
Message ID <differential-rev-PHID-DREV-6xt5qgccjhk7wl253zz6-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/39578/
State Superseded
Headers show

Comments

phabricator - April 12, 2019, 6:34 p.m.
gracinet created this revision.
Herald added subscribers: mercurial-devel, kevincox, durin42.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This time, it's simple enough that we can do it in all layers in
  one shot.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  rust/hg-core/src/discovery.rs
  rust/hg-cpython/src/discovery.rs
  tests/test-rust-discovery.py

CHANGE DETAILS




To: gracinet, #hg-reviewers
Cc: durin42, kevincox, mercurial-devel
Yuya Nishihara - April 17, 2019, 11:01 p.m.
Can't apply by `:D6233`. Can you rebase?

```
% hg pimp :D6233
applying patch from stdin
patching file mercurial/setdiscovery.py
Hunk #1 succeeded at 167 with fuzz 2 (offset 5 lines).
Hunk #2 FAILED at 343
1 out of 2 hunks FAILED -- saving rejects to file mercurial/setdiscovery.py.rej
abort: patch failed to apply
```
phabricator - April 17, 2019, 11:29 p.m.
yuja added a comment.


  Can't apply by `:D6233`. Can you rebase?
  
    % hg pimp :D6233
    applying patch from stdin
    patching file mercurial/setdiscovery.py
    Hunk #1 succeeded at 167 with fuzz 2 (offset 5 lines).
    Hunk #2 FAILED at 343
    1 out of 2 hunks FAILED -- saving rejects to file mercurial/setdiscovery.py.rej
    abort: patch failed to apply

REPOSITORY
  rHG Mercurial

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

To: gracinet, #hg-reviewers, kevincox
Cc: yuja, durin42, kevincox, mercurial-devel
Georges Racinet - April 18, 2019, 9:15 a.m.
On 4/18/19 1:29 AM, yuja (Yuya Nishihara) wrote:
> yuja added a comment.
>
>
>   Can't apply by `:D6233`. Can you rebase?

Just rebased everything onto 644eaffab9db.

>   
>     % hg pimp :D6233
>     applying patch from stdin
>     patching file mercurial/setdiscovery.py
>     Hunk #1 succeeded at 167 with fuzz 2 (offset 5 lines).
>     Hunk #2 FAILED at 343
>     1 out of 2 hunks FAILED -- saving rejects to file mercurial/setdiscovery.py.rej
>     abort: patch failed to apply

I don't know 'hg pimp', but I'm a bit surprised you got a failed
application on a file that's not modified by this revision.

>
> REPOSITORY
>   rHG Mercurial
>
> REVISION DETAIL
>   https://phab.mercurial-scm.org/D6233
>
> To: gracinet, #hg-reviewers, kevincox
> Cc: yuja, durin42, kevincox, mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - April 18, 2019, 10:32 p.m.
> On 4/18/19 1:29 AM, yuja (Yuya Nishihara) wrote:
> > yuja added a comment.
> >
> >
> >   Can't apply by `:D6233`. Can you rebase?
> 
> Just rebased everything onto 644eaffab9db.
> 
> >   
> >     % hg pimp :D6233
> >     applying patch from stdin
> >     patching file mercurial/setdiscovery.py
> >     Hunk #1 succeeded at 167 with fuzz 2 (offset 5 lines).
> >     Hunk #2 FAILED at 343
> >     1 out of 2 hunks FAILED -- saving rejects to file mercurial/setdiscovery.py.rej
> >     abort: patch failed to apply
> 
> I don't know 'hg pimp', but I'm a bit surprised you got a failed
> application on a file that's not modified by this revision.

It's basically `hg phabread | hg import`, and the conflict was D6228.
Excluding D6228 fixed the issue.

Phabricator sucks.
Georges Racinet - April 19, 2019, 8:15 a.m.
On 4/19/19 12:32 AM, Yuya Nishihara wrote:
>> On 4/18/19 1:29 AM, yuja (Yuya Nishihara) wrote:
>>> yuja added a comment.
>>>
>>>
>>>   Can't apply by `:D6233`. Can you rebase?
>> Just rebased everything onto 644eaffab9db.
>>
>>>   
>>>     % hg pimp :D6233
>>>     applying patch from stdin
>>>     patching file mercurial/setdiscovery.py
>>>     Hunk #1 succeeded at 167 with fuzz 2 (offset 5 lines).
>>>     Hunk #2 FAILED at 343
>>>     1 out of 2 hunks FAILED -- saving rejects to file mercurial/setdiscovery.py.rej
>>>     abort: patch failed to apply
>> I don't know 'hg pimp', but I'm a bit surprised you got a failed
>> application on a file that's not modified by this revision.
> It's basically `hg phabread | hg import`, and the conflict was D6228.
> Excluding D6228 fixed the issue.

Ah ok, so then, I should have untied D6229 from D6228 after I abandoned
the latter.

>
> Phabricator sucks.

For the record, I don't like it too much either, and I noticed you
prefer email interaction. That's why I'm sending patches that I don't
feel need  special attention by Kevin by email… and utimately ended up
submitting D6228 both ways…

Anyway, thanks for the queueing
Yuya Nishihara - April 19, 2019, 12:01 p.m.
> For the record, I don't like it too much either, and I noticed you
> prefer email interaction. That's why I'm sending patches that I don't
> feel need  special attention by Kevin by email… and utimately ended up
> submitting D6228 both ways…

That's okay, and seems better overall for the community as I'm getting less
involved in Mercurial. Still fun to read Rust patches. :)

Thanks.

Patch

diff --git a/tests/test-rust-discovery.py b/tests/test-rust-discovery.py
--- a/tests/test-rust-discovery.py
+++ b/tests/test-rust-discovery.py
@@ -82,6 +82,14 @@ 
 
         self.assertEqual(disco.commonheads(), {1})
 
+    def testaddmissingsstats(self):
+        idx = self.parseindex()
+        disco = PartialDiscovery(idx, [3])
+        self.assertIsNone(disco.stats()['undecided'], None)
+
+        disco.addmissings([2])
+        self.assertEqual(disco.stats()['undecided'], 2)
+
     def testaddinfocommonfirst(self):
         idx = self.parseindex()
         disco = PartialDiscovery(idx, [3])
diff --git a/rust/hg-cpython/src/discovery.rs b/rust/hg-cpython/src/discovery.rs
--- a/rust/hg-cpython/src/discovery.rs
+++ b/rust/hg-cpython/src/discovery.rs
@@ -14,7 +14,9 @@ 
 
 use crate::conversion::{py_set, rev_pyiter_collect};
 use cindex::Index;
-use cpython::{ObjectProtocol, PyDict, PyModule, PyObject, PyResult, Python};
+use cpython::{
+    ObjectProtocol, PyDict, PyModule, PyObject, PyResult, Python, ToPyObject,
+};
 use exceptions::GraphError;
 use hg::discovery::PartialDiscovery as CorePartialDiscovery;
 use hg::Revision;
@@ -82,6 +84,15 @@ 
         Ok(self.inner(py).borrow().is_complete())
     }
 
+    def stats(&self) -> PyResult<PyDict> {
+        let stats = self.inner(py).borrow().stats();
+        let as_dict: PyDict = PyDict::new(py);
+        as_dict.set_item(py, "undecided",
+                         stats.undecided.map(|l| l.to_py_object(py))
+                              .unwrap_or_else(|| py.None()))?;
+        Ok(as_dict)
+    }
+
     def commonheads(&self) -> PyResult<PyObject> {
         py_set(
             py,
diff --git a/rust/hg-core/src/discovery.rs b/rust/hg-core/src/discovery.rs
--- a/rust/hg-core/src/discovery.rs
+++ b/rust/hg-core/src/discovery.rs
@@ -23,6 +23,10 @@ 
     missing: HashSet<Revision>,
 }
 
+pub struct DiscoveryStats {
+    pub undecided: Option<usize>,
+}
+
 impl<G: Graph + Clone> PartialDiscovery<G> {
     /// Create a PartialDiscovery object, with the intent
     /// of comparing our `::<target_heads>` revset to the contents of another
@@ -114,6 +118,13 @@ 
             Some(self.common.missing_ancestors(tgt)?.into_iter().collect());
         Ok(())
     }
+
+    /// Provide statistics about the current state of the discovery process
+    pub fn stats(&self) -> DiscoveryStats {
+        DiscoveryStats {
+            undecided: self.undecided.as_ref().map(|s| s.len()),
+        }
+    }
 }
 
 #[cfg(test)]
@@ -156,6 +167,7 @@ 
         let mut disco = full_disco();
         assert_eq!(disco.undecided, None);
         assert!(!disco.has_info());
+        assert_eq!(disco.stats().undecided, None);
 
         disco.add_common_revisions(vec![11, 12])?;
         assert!(disco.has_info());
@@ -167,6 +179,7 @@ 
         assert_eq!(disco.undecided, None);
         disco.ensure_undecided()?;
         assert_eq!(sorted_undecided(&disco), vec![5, 8, 10, 13]);
+        assert_eq!(disco.stats().undecided, Some(4));
         Ok(())
     }