Patchwork [2,of,3,RFC] obsolete: allow passing a revision to successorssets()

login
register
mail settings
Submitter Dan Villiom Podlaski Christiansen
Date Aug. 8, 2013, 7:48 a.m.
Message ID <b1f7ae28c3e371a70ee3.1375948125@dookie.local>
Download mbox | patch
Permalink /patch/2093/
State Accepted
Commit 77d434760857ac26d268297489d1d93cb7334efe
Headers show

Comments

Dan Villiom Podlaski Christiansen - Aug. 8, 2013, 7:48 a.m.
# HG changeset patch
# User Dan Villiom Podlaski Christiansen  <danchr@gmail.com>
# Date 1375616619 -7200
#      Sun Aug 04 13:43:39 2013 +0200
# Node ID b1f7ae28c3e371a70ee363a4fbe5d50063a224fc
# Parent  02c24ad6f4d2d954da11d128280978e25aaa48fd
obsolete: allow passing a revision to successorssets()
Augie Fackler - Aug. 9, 2013, 3:13 p.m.
On Thu, Aug 08, 2013 at 09:48:45AM +0200, Dan Villiom Podlaski Christiansen wrote:
> # HG changeset patch
> # User Dan Villiom Podlaski Christiansen  <danchr@gmail.com>
> # Date 1375616619 -7200
> #      Sun Aug 04 13:43:39 2013 +0200
> # Node ID b1f7ae28c3e371a70ee363a4fbe5d50063a224fc
> # Parent  02c24ad6f4d2d954da11d128280978e25aaa48fd
> obsolete: allow passing a revision to successorssets()

I like this. I've been surprised more than once this didn't already
work. Any objections to me crewing this?

>
> diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
> --- a/mercurial/obsolete.py
> +++ b/mercurial/obsolete.py
> @@ -496,6 +496,9 @@ def successorssets(repo, initialnode, ca
>      for its live spawn. Code that makes multiple calls to `successorssets`
>      *must* use this cache mechanism or suffer terrible performances."""
>
> +    if isinstance(initialnode, int):
> +        initialnode = repo.unfiltered().changelog.node(initialnode)
> +
>      succmarkers = repo.obsstore.successors
>
>      # Stack of nodes we search successors sets for
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - Aug. 15, 2013, 10:55 p.m.
On 9 août 2013, at 17:13, Augie Fackler wrote:

> On Thu, Aug 08, 2013 at 09:48:45AM +0200, Dan Villiom Podlaski Christiansen wrote:
>> # HG changeset patch
>> # User Dan Villiom Podlaski Christiansen  <danchr@gmail.com>
>> # Date 1375616619 -7200
>> #      Sun Aug 04 13:43:39 2013 +0200
>> # Node ID b1f7ae28c3e371a70ee363a4fbe5d50063a224fc
>> # Parent  02c24ad6f4d2d954da11d128280978e25aaa48fd
>> obsolete: allow passing a revision to successorssets()
> 
> I like this. I've been surprised more than once this didn't already
> work. Any objections to me crewing this?

Yes ! there is no good reason for this layer to be aware about integer.
Pierre-Yves David - Aug. 15, 2013, 11:50 p.m.
On 8 août 2013, at 09:48, Dan Villiom Podlaski Christiansen wrote:

> # HG changeset patch
> # User Dan Villiom Podlaski Christiansen  <danchr@gmail.com>
> # Date 1375616619 -7200
> #      Sun Aug 04 13:43:39 2013 +0200
> # Node ID b1f7ae28c3e371a70ee363a4fbe5d50063a224fc
> # Parent  02c24ad6f4d2d954da11d128280978e25aaa48fd
> obsolete: allow passing a revision to successorssets()

Why do you need this.

> 
> diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
> --- a/mercurial/obsolete.py
> +++ b/mercurial/obsolete.py
> @@ -496,6 +496,9 @@ def successorssets(repo, initialnode, ca
>     for its live spawn. Code that makes multiple calls to `successorssets`
>     *must* use this cache mechanism or suffer terrible performances."""
> 
> +    if isinstance(initialnode, int):
> +        initialnode = repo.unfiltered().changelog.node(initialnode)
> +

No! this function is not aware that revision exist and have no reason to be aware of them.

The current signature is:

	successorssets(repo, initialnode, cache=None)

it could be:

	successorssets(obsstore, knownnodes, initialnode, cache=None)

We pass the repo object because it happen to have the two informations handy. But We all hope to diet the localrepo class.
Augie Fackler - Aug. 16, 2013, 12:39 a.m.
On Thu, Aug 15, 2013 at 7:50 PM, Pierre-Yves David
<pierre-yves.david@ens-lyon.org> wrote:
>
> On 8 août 2013, at 09:48, Dan Villiom Podlaski Christiansen wrote:
>
>> # HG changeset patch
>> # User Dan Villiom Podlaski Christiansen  <danchr@gmail.com>
>> # Date 1375616619 -7200
>> #      Sun Aug 04 13:43:39 2013 +0200
>> # Node ID b1f7ae28c3e371a70ee363a4fbe5d50063a224fc
>> # Parent  02c24ad6f4d2d954da11d128280978e25aaa48fd
>> obsolete: allow passing a revision to successorssets()
>
> Why do you need this.


Because with troubled divergent changes, there's _no other way_ to
figure out what the divergence is. I lost 30 minutes screwing around
in a REPL to figure this out once, and this revset would have been a
godsend.
Pierre-Yves David - Aug. 16, 2013, 12:48 a.m.
On 16 août 2013, at 02:39, Augie Fackler wrote:

> On Thu, Aug 15, 2013 at 7:50 PM, Pierre-Yves David
> <pierre-yves.david@ens-lyon.org> wrote:
>> 
>> On 8 août 2013, at 09:48, Dan Villiom Podlaski Christiansen wrote:
>> 
>>> # HG changeset patch
>>> # User Dan Villiom Podlaski Christiansen  <danchr@gmail.com>
>>> # Date 1375616619 -7200
>>> #      Sun Aug 04 13:43:39 2013 +0200
>>> # Node ID b1f7ae28c3e371a70ee363a4fbe5d50063a224fc
>>> # Parent  02c24ad6f4d2d954da11d128280978e25aaa48fd
>>> obsolete: allow passing a revision to successorssets()
>> 
>> Why do you need this.
> 
> 
> Because with troubled divergent changes, there's _no other way_ to
> figure out what the divergence is.

Knowing the successors set if a useful feature. but why do you need this very internal function to takes revision?

> I lost 30 minutes screwing around in a REPL to figure this out once, and this revset would have been a godsend.

(note: Until then you can use the debugsuccessorsset command)

revset ? successorsset return a list of tuple. how do you plan to use that usefully in a revset ?

if evolve doesn't already provide a revset for all successors, that a shame a can be quickly fixed.
Augie Fackler - Aug. 16, 2013, 1:16 a.m.
On Aug 15, 2013, at 8:48 PM, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:

> 
> On 16 août 2013, at 02:39, Augie Fackler wrote:
> 
>> On Thu, Aug 15, 2013 at 7:50 PM, Pierre-Yves David
>> <pierre-yves.david@ens-lyon.org> wrote:
>>> 
>>> On 8 août 2013, at 09:48, Dan Villiom Podlaski Christiansen wrote:
>>> 
>>>> # HG changeset patch
>>>> # User Dan Villiom Podlaski Christiansen  <danchr@gmail.com>
>>>> # Date 1375616619 -7200
>>>> #      Sun Aug 04 13:43:39 2013 +0200
>>>> # Node ID b1f7ae28c3e371a70ee363a4fbe5d50063a224fc
>>>> # Parent  02c24ad6f4d2d954da11d128280978e25aaa48fd
>>>> obsolete: allow passing a revision to successorssets()
>>> 
>>> Why do you need this.
>> 
>> 
>> Because with troubled divergent changes, there's _no other way_ to
>> figure out what the divergence is.
> 
> Knowing the successors set if a useful feature. but why do you need this very internal function to takes revision?

Huh, okay. Marmoute convinced me I'm just insane. Dropping this for now.

> 
>> I lost 30 minutes screwing around in a REPL to figure this out once, and this revset would have been a godsend.
> 
> (note: Until then you can use the debugsuccessorsset command)
> 
> revset ? successorsset return a list of tuple. how do you plan to use that usefully in a revset ?
> 
> if evolve doesn't already provide a revset for all successors, that a shame a can be quickly fixed.
> 
> -- 
> Pierre-Yves

Patch

diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
--- a/mercurial/obsolete.py
+++ b/mercurial/obsolete.py
@@ -496,6 +496,9 @@  def successorssets(repo, initialnode, ca
     for its live spawn. Code that makes multiple calls to `successorssets`
     *must* use this cache mechanism or suffer terrible performances."""
 
+    if isinstance(initialnode, int):
+        initialnode = repo.unfiltered().changelog.node(initialnode)
+
     succmarkers = repo.obsstore.successors
 
     # Stack of nodes we search successors sets for