Patchwork [3,of,4] convert: introduce hg.revs to replace hg.startrev and --rev with a revset

login
register
mail settings
Submitter Mads Kiilerich
Date Oct. 2, 2013, 5:56 p.m.
Message ID <103a80342ae8a92284ee.1380736605@mk-desktop>
Download mbox | patch
Permalink /patch/2710/
State Superseded
Commit e271970b98216098c1cb0c71ae1076d7ac0ad8ee
Headers show

Comments

Mads Kiilerich - Oct. 2, 2013, 5:56 p.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1374273788 -7200
#      Sat Jul 20 00:43:08 2013 +0200
# Node ID 103a80342ae8a92284ee184ae3232f10e93e8403
# Parent  6b53306b03188cb3803a8957d73ce551ec72b0b9
convert: introduce hg.revs to replace hg.startrev and --rev with a revset

The existing knobs for controlling which revisions to convert were often
insufficient. Revsets is a shiny hammer that provides a better solution.

The normal fancy deprecation markup doesn't work here so we just remove the
documentation of hg.startrev.

Changing --rev to take a revset for hg source repos would be a much more
elegant solution ... but not backwards compatible.
Augie Fackler - Oct. 3, 2013, 2:50 p.m.
On Wed, Oct 02, 2013 at 07:56:45PM +0200, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1374273788 -7200
> #      Sat Jul 20 00:43:08 2013 +0200
> # Node ID 103a80342ae8a92284ee184ae3232f10e93e8403
> # Parent  6b53306b03188cb3803a8957d73ce551ec72b0b9
> convert: introduce hg.revs to replace hg.startrev and --rev with a revset
>
> The existing knobs for controlling which revisions to convert were often
> insufficient. Revsets is a shiny hammer that provides a better solution.
>
> The normal fancy deprecation markup doesn't work here so we just remove the
> documentation of hg.startrev.
>
> Changing --rev to take a revset for hg source repos would be a much more
> elegant solution ... but not backwards compatible.

It's new functionality, how would it fail to be backwards compatible?
What am I missing?

>
> diff --git a/hgext/convert/__init__.py b/hgext/convert/__init__.py
> --- a/hgext/convert/__init__.py
> +++ b/hgext/convert/__init__.py
> @@ -155,8 +155,7 @@
>          (forces target IDs to change). It takes a boolean argument and
>          defaults to False.
>
> -    :convert.hg.startrev: convert start revision and its descendants.
> -        It takes a hg revision identifier and defaults to 0.
> +    :convert.hg.revs: revset specifying the source revisions to convert.
>
>      CVS Source
>      ##########
> diff --git a/hgext/convert/hg.py b/hgext/convert/hg.py
> --- a/hgext/convert/hg.py
> +++ b/hgext/convert/hg.py
> @@ -21,7 +21,7 @@
>  import os, time, cStringIO
>  from mercurial.i18n import _
>  from mercurial.node import bin, hex, nullid
> -from mercurial import hg, util, context, bookmarks, error
> +from mercurial import hg, util, context, bookmarks, error, scmutil
>
>  from common import NoRepo, commit, converter_source, converter_sink
>
> @@ -252,23 +252,37 @@
>          self.convertfp = None
>          # Restrict converted revisions to startrev descendants
>          startnode = ui.config('convert', 'hg.startrev')
> -        if startnode is not None:
> -            try:
> -                startnode = self.repo.lookup(startnode)
> -            except error.RepoError:
> -                raise util.Abort(_('%s is not a valid start revision')
> -                                 % startnode)
> -            startrev = self.repo.changelog.rev(startnode)
> -            children = {startnode: 1}
> -            for r in self.repo.changelog.descendants([startrev]):
> -                children[self.repo.changelog.node(r)] = 1
> -            self.keep = children.__contains__
> +        hgrevs = ui.config('convert', 'hg.revs')
> +        if hgrevs is None:
> +            if startnode is not None:
> +                try:
> +                    startnode = self.repo.lookup(startnode)
> +                except error.RepoError:
> +                    raise util.Abort(_('%s is not a valid start revision')
> +                                     % startnode)
> +                startrev = self.repo.changelog.rev(startnode)
> +                children = {startnode: 1}
> +                for r in self.repo.changelog.descendants([startrev]):
> +                    children[self.repo.changelog.node(r)] = 1
> +                self.keep = children.__contains__
> +            else:
> +                self.keep = util.always
> +            if rev:
> +                self._heads = [self.repo[rev].node()]
> +            else:
> +                self._heads = self.repo.heads()
>          else:
> -            self.keep = util.always
> -        if rev:
> -            self._heads = [self.repo[rev].node()]
> -        else:
> -            self._heads = self.repo.heads()
> +            if rev or startnode is not None:
> +                raise util.Abort(_('hg.revs cannot be combined with '
> +                                   'hg.startrev or --rev'))
> +            nodes = set()
> +            parents = set()
> +            for r in scmutil.revrange(self.repo, [hgrevs]):
> +                ctx = self.repo[r]
> +                nodes.add(ctx.node())
> +                parents.update(p.node() for p in ctx.parents())
> +            self.keep = nodes.__contains__
> +            self._heads = nodes - parents
>
>      def changectx(self, rev):
>          if self.lastrev != rev:
> diff --git a/tests/test-convert-hg-startrev.t b/tests/test-convert-hg-startrev.t
> --- a/tests/test-convert-hg-startrev.t
> +++ b/tests/test-convert-hg-startrev.t
> @@ -183,3 +183,23 @@
>    b
>    $ hg -q verify
>    $ cd ..
> +
> +Convert from revset in convert.hg.revs
> +
> +  $ hg convert --config convert.hg.revs='3:4+0' source revsetrepo
> +  initializing destination revsetrepo repository
> +  scanning source...
> +  sorting...
> +  converting...
> +  2 0: add a b f
> +  1 3: change a
> +  0 4: merge 2 and 3
> +
> +  $ glog revsetrepo
> +  o  2 "4: merge 2 and 3" files: b c d e f
> +  |
> +  o  1 "3: change a" files: a
> +  |
> +  o  0 "0: add a b f" files: a b f
> +
> +  $ cd ..
> diff --git a/tests/test-convert.t b/tests/test-convert.t
> --- a/tests/test-convert.t
> +++ b/tests/test-convert.t
> @@ -135,9 +135,8 @@
>                      store original revision ID in changeset (forces target IDs
>                      to change). It takes a boolean argument and defaults to
>                      False.
> -      convert.hg.startrev
> -                    convert start revision and its descendants. It takes a hg
> -                    revision identifier and defaults to 0.
> +      convert.hg.revs
> +                    revset specifying the source revisions to convert.
>
>        CVS Source
>        ##########
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Mads Kiilerich - Oct. 3, 2013, 3:24 p.m.
On 10/03/2013 04:50 PM, Augie Fackler wrote:
> On Wed, Oct 02, 2013 at 07:56:45PM +0200, Mads Kiilerich wrote:
>> # HG changeset patch
>> # User Mads Kiilerich <madski@unity3d.com>
>> # Date 1374273788 -7200
>> #      Sat Jul 20 00:43:08 2013 +0200
>> # Node ID 103a80342ae8a92284ee184ae3232f10e93e8403
>> # Parent  6b53306b03188cb3803a8957d73ce551ec72b0b9
>> convert: introduce hg.revs to replace hg.startrev and --rev with a revset
>>
>> The existing knobs for controlling which revisions to convert were often
>> insufficient. Revsets is a shiny hammer that provides a better solution.
>>
>> The normal fancy deprecation markup doesn't work here so we just remove the
>> documentation of hg.startrev.
>>
>> Changing --rev to take a revset for hg source repos would be a much more
>> elegant solution ... but not backwards compatible.
> It's new functionality, how would it fail to be backwards compatible?
> What am I missing?

Revsets has been introduced in --rev handling in a lot of places and 
thus introduced new functionality. There are corner cases where it 
didn't stay backward compatible ... but they are few and that was 
probably ok.

Just sayin' that we can't do the same here. "-r 7" used to mean revision 
0 to 7 - it would be a major change if it suddenly just meant revision 
7. It would obviously also only work for hg sources.

Magic behaviour like the following might be ok for diff, but I don't 
think it would be ok for convert:

   $ hg init
   $ echo 0 > f && hg ci -Am0
   adding f
   $ echo 1 > f && hg ci -Am1
   $ echo 2 > f && hg ci -Am2
   $ echo 3 > f
   $ hg diff --nodates -r '(2)'
   diff -r a6fee907f844 f
   --- a/f
   +++ b/f
   @@ -1,1 +1,1 @@
   -2
   +3
   $ hg diff --nodates -r '(1+2)'
   diff -r c4c1f08311dd -r a6fee907f844 f
   --- a/f
   +++ b/f
   @@ -1,1 +1,1 @@
   -1
   +2

/Mads
Augie Fackler - Oct. 5, 2013, 9:24 p.m.
On Oct 3, 2013, at 11:24 AM, Mads Kiilerich <mads@kiilerich.com> wrote:

> On 10/03/2013 04:50 PM, Augie Fackler wrote:
>> On Wed, Oct 02, 2013 at 07:56:45PM +0200, Mads Kiilerich wrote:
>>> # HG changeset patch
>>> # User Mads Kiilerich <madski@unity3d.com>
>>> # Date 1374273788 -7200
>>> #      Sat Jul 20 00:43:08 2013 +0200
>>> # Node ID 103a80342ae8a92284ee184ae3232f10e93e8403
>>> # Parent  6b53306b03188cb3803a8957d73ce551ec72b0b9
>>> convert: introduce hg.revs to replace hg.startrev and --rev with a revset
>>> 
>>> The existing knobs for controlling which revisions to convert were often
>>> insufficient. Revsets is a shiny hammer that provides a better solution.
>>> 
>>> The normal fancy deprecation markup doesn't work here so we just remove the
>>> documentation of hg.startrev.
>>> 
>>> Changing --rev to take a revset for hg source repos would be a much more
>>> elegant solution ... but not backwards compatible.
>> It's new functionality, how would it fail to be backwards compatible?
>> What am I missing?
> 
> Revsets has been introduced in --rev handling in a lot of places and thus introduced new functionality. There are corner cases where it didn't stay backward compatible ... but they are few and that was probably ok.
> 
> Just sayin' that we can't do the same here. "-r 7" used to mean revision 0 to 7 - it would be a major change if it suddenly just meant revision 7. It would obviously also only work for hg sources.

Got it, that makes sense. Can you add this description to the commit message and resend?

> 
> Magic behaviour like the following might be ok for diff, but I don't think it would be ok for convert:
> 
>  $ hg init
>  $ echo 0 > f && hg ci -Am0
>  adding f
>  $ echo 1 > f && hg ci -Am1
>  $ echo 2 > f && hg ci -Am2
>  $ echo 3 > f
>  $ hg diff --nodates -r '(2)'
>  diff -r a6fee907f844 f
>  --- a/f
>  +++ b/f
>  @@ -1,1 +1,1 @@
>  -2
>  +3
>  $ hg diff --nodates -r '(1+2)'
>  diff -r c4c1f08311dd -r a6fee907f844 f
>  --- a/f
>  +++ b/f
>  @@ -1,1 +1,1 @@
>  -1
>  +2
> 
> /Mads

Patch

diff --git a/hgext/convert/__init__.py b/hgext/convert/__init__.py
--- a/hgext/convert/__init__.py
+++ b/hgext/convert/__init__.py
@@ -155,8 +155,7 @@ 
         (forces target IDs to change). It takes a boolean argument and
         defaults to False.
 
-    :convert.hg.startrev: convert start revision and its descendants.
-        It takes a hg revision identifier and defaults to 0.
+    :convert.hg.revs: revset specifying the source revisions to convert.
 
     CVS Source
     ##########
diff --git a/hgext/convert/hg.py b/hgext/convert/hg.py
--- a/hgext/convert/hg.py
+++ b/hgext/convert/hg.py
@@ -21,7 +21,7 @@ 
 import os, time, cStringIO
 from mercurial.i18n import _
 from mercurial.node import bin, hex, nullid
-from mercurial import hg, util, context, bookmarks, error
+from mercurial import hg, util, context, bookmarks, error, scmutil
 
 from common import NoRepo, commit, converter_source, converter_sink
 
@@ -252,23 +252,37 @@ 
         self.convertfp = None
         # Restrict converted revisions to startrev descendants
         startnode = ui.config('convert', 'hg.startrev')
-        if startnode is not None:
-            try:
-                startnode = self.repo.lookup(startnode)
-            except error.RepoError:
-                raise util.Abort(_('%s is not a valid start revision')
-                                 % startnode)
-            startrev = self.repo.changelog.rev(startnode)
-            children = {startnode: 1}
-            for r in self.repo.changelog.descendants([startrev]):
-                children[self.repo.changelog.node(r)] = 1
-            self.keep = children.__contains__
+        hgrevs = ui.config('convert', 'hg.revs')
+        if hgrevs is None:
+            if startnode is not None:
+                try:
+                    startnode = self.repo.lookup(startnode)
+                except error.RepoError:
+                    raise util.Abort(_('%s is not a valid start revision')
+                                     % startnode)
+                startrev = self.repo.changelog.rev(startnode)
+                children = {startnode: 1}
+                for r in self.repo.changelog.descendants([startrev]):
+                    children[self.repo.changelog.node(r)] = 1
+                self.keep = children.__contains__
+            else:
+                self.keep = util.always
+            if rev:
+                self._heads = [self.repo[rev].node()]
+            else:
+                self._heads = self.repo.heads()
         else:
-            self.keep = util.always
-        if rev:
-            self._heads = [self.repo[rev].node()]
-        else:
-            self._heads = self.repo.heads()
+            if rev or startnode is not None:
+                raise util.Abort(_('hg.revs cannot be combined with '
+                                   'hg.startrev or --rev'))
+            nodes = set()
+            parents = set()
+            for r in scmutil.revrange(self.repo, [hgrevs]):
+                ctx = self.repo[r]
+                nodes.add(ctx.node())
+                parents.update(p.node() for p in ctx.parents())
+            self.keep = nodes.__contains__
+            self._heads = nodes - parents
 
     def changectx(self, rev):
         if self.lastrev != rev:
diff --git a/tests/test-convert-hg-startrev.t b/tests/test-convert-hg-startrev.t
--- a/tests/test-convert-hg-startrev.t
+++ b/tests/test-convert-hg-startrev.t
@@ -183,3 +183,23 @@ 
   b
   $ hg -q verify
   $ cd ..
+
+Convert from revset in convert.hg.revs
+
+  $ hg convert --config convert.hg.revs='3:4+0' source revsetrepo
+  initializing destination revsetrepo repository
+  scanning source...
+  sorting...
+  converting...
+  2 0: add a b f
+  1 3: change a
+  0 4: merge 2 and 3
+
+  $ glog revsetrepo
+  o  2 "4: merge 2 and 3" files: b c d e f
+  |
+  o  1 "3: change a" files: a
+  |
+  o  0 "0: add a b f" files: a b f
+  
+  $ cd ..
diff --git a/tests/test-convert.t b/tests/test-convert.t
--- a/tests/test-convert.t
+++ b/tests/test-convert.t
@@ -135,9 +135,8 @@ 
                     store original revision ID in changeset (forces target IDs
                     to change). It takes a boolean argument and defaults to
                     False.
-      convert.hg.startrev
-                    convert start revision and its descendants. It takes a hg
-                    revision identifier and defaults to 0.
+      convert.hg.revs
+                    revset specifying the source revisions to convert.
   
       CVS Source
       ##########