Patchwork [evolve-ext] evolve: dedupe divergents when running evolve --all --any or evolve --rev

login
register
mail settings
Submitter Laurent Charignon
Date June 26, 2015, 12:18 a.m.
Message ID <6c3d487fa37c5684185e.1435277931@lcharignon-mbp.local>
Download mbox | patch
Permalink /patch/9782/
State Changes Requested
Headers show

Comments

Laurent Charignon - June 26, 2015, 12:18 a.m.
# HG changeset patch
# User Laurent Charignon <lcharignon@fb.com>
# Date 1435190063 25200
#      Wed Jun 24 16:54:23 2015 -0700
# Node ID 6c3d487fa37c5684185e650a2a15796f01187ebc
# Parent  d3328e6775b1f23c6ab41ccd8712e02abb6eea72
evolve: dedupe divergents when running evolve --all --any or evolve --rev

Before this patch, when running evolve --all --any or evolve --rev with the
--divergent flag, we were selecting all of the divergents. After solving the
first one, its counterparts would get pruned and potentially hidden which would
crash when trying to resolve them. This patch introduces logic to dedupe the
divergents to be resolved by keeping only one per group of divergent with the
lower revision number.
Pierre-Yves David - June 26, 2015, 8:26 a.m.
On 06/25/2015 05:18 PM, Laurent Charignon wrote:
> # HG changeset patch
> # User Laurent Charignon <lcharignon@fb.com>
> # Date 1435190063 25200
> #      Wed Jun 24 16:54:23 2015 -0700
> # Node ID 6c3d487fa37c5684185e650a2a15796f01187ebc
> # Parent  d3328e6775b1f23c6ab41ccd8712e02abb6eea72
> evolve: dedupe divergents when running evolve --all --any or evolve --rev
>
> Before this patch, when running evolve --all --any or evolve --rev with the
> --divergent flag, we were selecting all of the divergents. After solving the
> first one, its counterparts would get pruned and potentially hidden which would
> crash when trying to resolve them. This patch introduces logic to dedupe the
> divergents to be resolved by keeping only one per group of divergent with the
> lower revision number.

1) Computing the divergence data multiple time is probably significantly 
expensive, (eg: we could rules out the one we discard for improvement)

2) This going to misbehave When there is more than 2 divergent branches,

However, this is better than crashing so I'm ready to took it. I've some 
comment on the test however


>
> diff --git a/hgext/evolve.py b/hgext/evolve.py
> --- a/hgext/evolve.py
> +++ b/hgext/evolve.py
> @@ -1412,6 +1412,17 @@ def builddependencies(repo, revs):
>                   rdependencies[succ].add(r)
>       return dependencies, rdependencies
>
> +def _dedupedivergents(repo, revs):
> +    """Dedupe the divergents revs in revs to get one from each group with the
> +    lowest revision numbers"""
> +    repo = repo.unfiltered()
> +    res = set()
> +    for rev in revs:
> +        divergent = repo[rev]
> +        base, others = divergentdata(divergent)
> +        res.add(min([divergent] + list(others)))
> +    return res
> +
>   def _selectrevs(repo, allopt, revopt, anyopt, targetcat):
>       """select troubles in repo matching according to given options"""
>       revs = set()
> @@ -1421,6 +1432,9 @@ def _selectrevs(repo, allopt, revopt, an
>               revs = scmutil.revrange(repo, revopt) & revs
>           elif not anyopt and targetcat == 'unstable':
>               revs = set(_aspiringdescendant(repo, repo.revs('(.::) - obsolete()::')))
> +        if targetcat == 'divergent':
> +            # Pick one divergent per group of divergents
> +            revs = _dedupedivergents(repo, revs)
>       elif anyopt:
>           revs = repo.revs('first(%s())' % (targetcat))
>       elif targetcat == 'unstable':
> @@ -1432,6 +1446,7 @@ def _selectrevs(repo, allopt, revopt, an
>               raise error.Abort(msg, hint=hint)
>       elif targetcat in repo['.'].troubles():
>           revs = set([repo['.'].rev()])
> +
>       return revs
>
>
> diff --git a/tests/test-divergent.t b/tests/test-divergent.t
> new file mode 100644
> --- /dev/null
> +++ b/tests/test-divergent.t
> @@ -0,0 +1,80 @@


This test file want a main title and a comment regarding its purpose

> +  $ cat >> $HGRCPATH <<EOF
> +  > [defaults]
> +  > amend=-d "0 0"
> +  > fold=-d "0 0"
> +  > [web]
> +  > push_ssl = false
> +  > allow_push = *
> +  > [phases]
> +  > publish = False
> +  > [diff]
> +  > git = 1
> +  > unified = 0
> +  > [ui]
> +  > logtemplate = {rev}:{node|short}@{branch}({phase}) {desc|firstline} [{troubles}]\n
> +  > [extensions]
> +  > hgext.graphlog=
> +  > EOF
> +  $ echo "evolve=$(echo $(dirname $TESTDIR))/hgext/evolve.py" >> $HGRCPATH
> +  $ mkcommit() {
> +  >    echo "$1" > "$1"
> +  >    hg add "$1"
> +  >    hg ci -m "add $1"
> +  > }
> +
> +  $ mkstack() {
> +  >    # Creates a stack of commit based on $1 with messages from $2, $3 ..
> +  >    hg update "$1" -C
> +  >    shift
> +  >    mkcommits $*
> +  > }

This one is not used anywhere.

> +
> +  $ mkcommits() {
> +  >   for i in $@; do mkcommit $i ; done
> +  > }

This one have 1 user with two entries. it does not strike me a super useful.

> +
> +Basic test


You want this to be a title and more description about the purpose of 
the test.

> +  $ hg init test1
> +  $ cd test1
> +  $ mkcommits _a _b
> +  $ hg up "desc(_a)"
> +  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +  $ mkcommit bdivergent1
> +  created new head
> +  $ hg up "desc(_a)"
> +  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +  $ mkcommit bdivergent2
> +  created new head
> +  $ hg prune -s "desc(bdivergent1)" "desc(_b)"
> +  1 changesets pruned
> +  $ hg prune -s "desc(bdivergent2)" "desc(_b)" --hidden
> +  1 changesets pruned
> +  2 new divergent changesets
> +  $ hg log -G
> +  @  3:e708fd28d5cf@default(draft) add bdivergent2 [divergent]
> +  |
> +  | o  2:c2f698071cba@default(draft) add bdivergent1 [divergent]
> +  |/
> +  o  0:135f39f4bd78@default(draft) add _a []
> +
> +  $ hg evolve --all --any --divergent
> +  merge:[2] add bdivergent1
> +  with: [3] add bdivergent2
> +  base: [1] add _b
> +  updating to "local" conflict
> +  1 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  merge:[3] add bdivergent2
> +  with: [5] add bdivergent1
> +  base: [1] add _b
> +  updating to "local" conflict
> +  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  2 new divergent changesets
> +  working directory is now at aa26817f6fbe
> +  $ hg log -G
> +  @  7:aa26817f6fbe@default(draft) add bdivergent2 []
> +  |
> +  o  0:135f39f4bd78@default(draft) add _a []
> +
> +  $ cd ..
Laurent Charignon - June 26, 2015, 5:59 p.m.
> On Jun 26, 2015, at 1:26 AM, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:
> 
> 
> 
> On 06/25/2015 05:18 PM, Laurent Charignon wrote:
>> # HG changeset patch
>> # User Laurent Charignon <lcharignon@fb.com>
>> # Date 1435190063 25200
>> #      Wed Jun 24 16:54:23 2015 -0700
>> # Node ID 6c3d487fa37c5684185e650a2a15796f01187ebc
>> # Parent  d3328e6775b1f23c6ab41ccd8712e02abb6eea72
>> evolve: dedupe divergents when running evolve --all --any or evolve --rev
>> 
>> Before this patch, when running evolve --all --any or evolve --rev with the
>> --divergent flag, we were selecting all of the divergents. After solving the
>> first one, its counterparts would get pruned and potentially hidden which would
>> crash when trying to resolve them. This patch introduces logic to dedupe the
>> divergents to be resolved by keeping only one per group of divergent with the
>> lower revision number.
> 
> 1) Computing the divergence data multiple time is probably significantly expensive, (eg: we could rules out the one we discard for improvement)
> 
> 2) This going to misbehave When there is more than 2 divergent branches,
> 
> However, this is better than crashing so I'm ready to took it. I've some comment on the test however

ok

> 
> 
>> 
>> diff --git a/hgext/evolve.py b/hgext/evolve.py
>> --- a/hgext/evolve.py
>> +++ b/hgext/evolve.py
>> @@ -1412,6 +1412,17 @@ def builddependencies(repo, revs):
>>                  rdependencies[succ].add(r)
>>      return dependencies, rdependencies
>> 
>> +def _dedupedivergents(repo, revs):
>> +    """Dedupe the divergents revs in revs to get one from each group with the
>> +    lowest revision numbers"""
>> +    repo = repo.unfiltered()
>> +    res = set()
>> +    for rev in revs:
>> +        divergent = repo[rev]
>> +        base, others = divergentdata(divergent)
>> +        res.add(min([divergent] + list(others)))
>> +    return res
>> +
>>  def _selectrevs(repo, allopt, revopt, anyopt, targetcat):
>>      """select troubles in repo matching according to given options"""
>>      revs = set()
>> @@ -1421,6 +1432,9 @@ def _selectrevs(repo, allopt, revopt, an
>>              revs = scmutil.revrange(repo, revopt) & revs
>>          elif not anyopt and targetcat == 'unstable':
>>              revs = set(_aspiringdescendant(repo, repo.revs('(.::) - obsolete()::')))
>> +        if targetcat == 'divergent':
>> +            # Pick one divergent per group of divergents
>> +            revs = _dedupedivergents(repo, revs)
>>      elif anyopt:
>>          revs = repo.revs('first(%s())' % (targetcat))
>>      elif targetcat == 'unstable':
>> @@ -1432,6 +1446,7 @@ def _selectrevs(repo, allopt, revopt, an
>>              raise error.Abort(msg, hint=hint)
>>      elif targetcat in repo['.'].troubles():
>>          revs = set([repo['.'].rev()])
>> +
>>      return revs
>> 
>> 
>> diff --git a/tests/test-divergent.t b/tests/test-divergent.t
>> new file mode 100644
>> --- /dev/null
>> +++ b/tests/test-divergent.t
>> @@ -0,0 +1,80 @@
> 
> 
> This test file want a main title and a comment regarding its purpose

Will do
> 
>> +  $ cat >> $HGRCPATH <<EOF
>> +  > [defaults]
>> +  > amend=-d "0 0"
>> +  > fold=-d "0 0"
>> +  > [web]
>> +  > push_ssl = false
>> +  > allow_push = *
>> +  > [phases]
>> +  > publish = False
>> +  > [diff]
>> +  > git = 1
>> +  > unified = 0
>> +  > [ui]
>> +  > logtemplate = {rev}:{node|short}@{branch}({phase}) {desc|firstline} [{troubles}]\n
>> +  > [extensions]
>> +  > hgext.graphlog=
>> +  > EOF
>> +  $ echo "evolve=$(echo $(dirname $TESTDIR))/hgext/evolve.py" >> $HGRCPATH
>> +  $ mkcommit() {
>> +  >    echo "$1" > "$1"
>> +  >    hg add "$1"
>> +  >    hg ci -m "add $1"
>> +  > }
>> +
>> +  $ mkstack() {
>> +  >    # Creates a stack of commit based on $1 with messages from $2, $3 ..
>> +  >    hg update "$1" -C
>> +  >    shift
>> +  >    mkcommits $*
>> +  > }
> 
> This one is not used anywhere.

I had to take this patch out of a series with tons of new tests and I forgot to remove this that is used later.
I will remove it if you want.
We really should have helpers functions for tests that are loaded for all the tests.

> 
>> +
>> +  $ mkcommits() {
>> +  >   for i in $@; do mkcommit $i ; done
>> +  > }
> 
> This one have 1 user with two entries. it does not strike me a super useful.

ditto

> 
>> +
>> +Basic test
> 
> 
> You want this to be a title and more description about the purpose of the test.

ok

> 
>> +  $ hg init test1
>> +  $ cd test1
>> +  $ mkcommits _a _b
>> +  $ hg up "desc(_a)"
>> +  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
>> +  $ mkcommit bdivergent1
>> +  created new head
>> +  $ hg up "desc(_a)"
>> +  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
>> +  $ mkcommit bdivergent2
>> +  created new head
>> +  $ hg prune -s "desc(bdivergent1)" "desc(_b)"
>> +  1 changesets pruned
>> +  $ hg prune -s "desc(bdivergent2)" "desc(_b)" --hidden
>> +  1 changesets pruned
>> +  2 new divergent changesets
>> +  $ hg log -G
>> +  @  3:e708fd28d5cf@default(draft) add bdivergent2 [divergent]
>> +  |
>> +  | o  2:c2f698071cba@default(draft) add bdivergent1 [divergent]
>> +  |/
>> +  o  0:135f39f4bd78@default(draft) add _a []
>> +
>> +  $ hg evolve --all --any --divergent
>> +  merge:[2] add bdivergent1
>> +  with: [3] add bdivergent2
>> +  base: [1] add _b
>> +  updating to "local" conflict
>> +  1 files updated, 0 files merged, 1 files removed, 0 files unresolved
>> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>> +  merge:[3] add bdivergent2
>> +  with: [5] add bdivergent1
>> +  base: [1] add _b
>> +  updating to "local" conflict
>> +  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
>> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>> +  2 new divergent changesets
>> +  working directory is now at aa26817f6fbe
>> +  $ hg log -G
>> +  @  7:aa26817f6fbe@default(draft) add bdivergent2 []
>> +  |
>> +  o  0:135f39f4bd78@default(draft) add _a []
>> +
>> +  $ cd ..
> 
> 
> -- 
> Pierre-Yves David

Patch

diff --git a/hgext/evolve.py b/hgext/evolve.py
--- a/hgext/evolve.py
+++ b/hgext/evolve.py
@@ -1412,6 +1412,17 @@  def builddependencies(repo, revs):
                 rdependencies[succ].add(r)
     return dependencies, rdependencies
 
+def _dedupedivergents(repo, revs):
+    """Dedupe the divergents revs in revs to get one from each group with the
+    lowest revision numbers"""
+    repo = repo.unfiltered()
+    res = set()
+    for rev in revs:
+        divergent = repo[rev]
+        base, others = divergentdata(divergent)
+        res.add(min([divergent] + list(others)))
+    return res
+
 def _selectrevs(repo, allopt, revopt, anyopt, targetcat):
     """select troubles in repo matching according to given options"""
     revs = set()
@@ -1421,6 +1432,9 @@  def _selectrevs(repo, allopt, revopt, an
             revs = scmutil.revrange(repo, revopt) & revs
         elif not anyopt and targetcat == 'unstable':
             revs = set(_aspiringdescendant(repo, repo.revs('(.::) - obsolete()::')))
+        if targetcat == 'divergent':
+            # Pick one divergent per group of divergents
+            revs = _dedupedivergents(repo, revs)
     elif anyopt:
         revs = repo.revs('first(%s())' % (targetcat))
     elif targetcat == 'unstable':
@@ -1432,6 +1446,7 @@  def _selectrevs(repo, allopt, revopt, an
             raise error.Abort(msg, hint=hint)
     elif targetcat in repo['.'].troubles():
         revs = set([repo['.'].rev()])
+
     return revs
 
 
diff --git a/tests/test-divergent.t b/tests/test-divergent.t
new file mode 100644
--- /dev/null
+++ b/tests/test-divergent.t
@@ -0,0 +1,80 @@ 
+  $ cat >> $HGRCPATH <<EOF
+  > [defaults]
+  > amend=-d "0 0"
+  > fold=-d "0 0"
+  > [web]
+  > push_ssl = false
+  > allow_push = *
+  > [phases]
+  > publish = False
+  > [diff]
+  > git = 1
+  > unified = 0
+  > [ui]
+  > logtemplate = {rev}:{node|short}@{branch}({phase}) {desc|firstline} [{troubles}]\n
+  > [extensions]
+  > hgext.graphlog=
+  > EOF
+  $ echo "evolve=$(echo $(dirname $TESTDIR))/hgext/evolve.py" >> $HGRCPATH
+  $ mkcommit() {
+  >    echo "$1" > "$1"
+  >    hg add "$1"
+  >    hg ci -m "add $1"
+  > }
+
+  $ mkstack() {
+  >    # Creates a stack of commit based on $1 with messages from $2, $3 ..
+  >    hg update "$1" -C
+  >    shift
+  >    mkcommits $*
+  > }
+
+  $ mkcommits() {
+  >   for i in $@; do mkcommit $i ; done
+  > }
+
+Basic test
+  $ hg init test1
+  $ cd test1
+  $ mkcommits _a _b
+  $ hg up "desc(_a)"
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  $ mkcommit bdivergent1
+  created new head
+  $ hg up "desc(_a)"
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  $ mkcommit bdivergent2
+  created new head
+  $ hg prune -s "desc(bdivergent1)" "desc(_b)"
+  1 changesets pruned
+  $ hg prune -s "desc(bdivergent2)" "desc(_b)" --hidden
+  1 changesets pruned
+  2 new divergent changesets
+  $ hg log -G
+  @  3:e708fd28d5cf@default(draft) add bdivergent2 [divergent]
+  |
+  | o  2:c2f698071cba@default(draft) add bdivergent1 [divergent]
+  |/
+  o  0:135f39f4bd78@default(draft) add _a []
+  
+  $ hg evolve --all --any --divergent
+  merge:[2] add bdivergent1
+  with: [3] add bdivergent2
+  base: [1] add _b
+  updating to "local" conflict
+  1 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  merge:[3] add bdivergent2
+  with: [5] add bdivergent1
+  base: [1] add _b
+  updating to "local" conflict
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  2 new divergent changesets
+  working directory is now at aa26817f6fbe
+  $ hg log -G
+  @  7:aa26817f6fbe@default(draft) add bdivergent2 []
+  |
+  o  0:135f39f4bd78@default(draft) add _a []
+  
+  $ cd ..