Patchwork [2,of,2,convert,backouts] convert: backout 81cf597dafa9 and a3545c3104aa -closemap

login
register
mail settings
Submitter Mads Kiilerich
Date April 16, 2014, 4:05 p.m.
Message ID <c0c12f7dbe25a3b82949.1397664342@mk-desktop>
Download mbox | patch
Permalink /patch/4385/
State Accepted
Commit 78b15ad2f968d4e2a55f1765ee1dac72db250324
Headers show

Comments

Mads Kiilerich - April 16, 2014, 4:05 p.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1397603408 -7200
#      Wed Apr 16 01:10:08 2014 +0200
# Node ID c0c12f7dbe25a3b8294905b072f6f27291ad26b4
# Parent  319a4229e58fba95bea881cba07a62a726723227
convert: backout 81cf597dafa9 and a3545c3104aa -closemap

Closemap solves a very specific use case. It would be better to have a more
generic solution than to have to maintain this forever.

Closemap has not been released yet and removing it now will not break any
backward compatibility contract.

There is no test coverage for closemap but it seems like the same can be
achieved with a simple and much more powerful custom extension:

import hgext.convert.hg
class source(hgext.convert.hg.mercurial_source):
    def getcommit(self, rev):
        c = super(source, self).getcommit(rev)
        if rev in ['''
d643f67092ff123f6a192d52f12e7d123dae229f
9117c6561b0bd7792fa13b50d28239d51b78e51f
f368a1c302d5b87506f7edb13769e591e063d7ea
''']:
            c.extra = c.extra.copy()
            c.extra['close'] = '1'
        return c
hgext.convert.hg.mercurial_source = source
Matt Mackall - April 16, 2014, 6:17 p.m.
On Wed, 2014-04-16 at 18:05 +0200, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1397603408 -7200
> #      Wed Apr 16 01:10:08 2014 +0200
> # Node ID c0c12f7dbe25a3b8294905b072f6f27291ad26b4
> # Parent  319a4229e58fba95bea881cba07a62a726723227
> convert: backout 81cf597dafa9 and a3545c3104aa -closemap

These are queued for default, thanks.

We should probably post (or put in contrib) a simple commented shim
extension that does dummy mappings on tags, authors, branches,
descriptions..
Mads Kiilerich - April 16, 2014, 6:56 p.m.
On 04/16/2014 08:17 PM, Matt Mackall wrote:
> On Wed, 2014-04-16 at 18:05 +0200, Mads Kiilerich wrote:
>> # HG changeset patch
>> # User Mads Kiilerich <madski@unity3d.com>
>> # Date 1397603408 -7200
>> #      Wed Apr 16 01:10:08 2014 +0200
>> # Node ID c0c12f7dbe25a3b8294905b072f6f27291ad26b4
>> # Parent  319a4229e58fba95bea881cba07a62a726723227
>> convert: backout 81cf597dafa9 and a3545c3104aa -closemap
> These are queued for default, thanks.
>
> We should probably post (or put in contrib) a simple commented shim
> extension that does dummy mappings on tags, authors, branches,
> descriptions..

That is pretty much what I tried to do at 
http://mercurial.selenic.com/wiki/ConvertExtension#Customization - do 
you have other something else in mind?

/Mads
Sean Farley - April 16, 2014, 8:08 p.m.
Mads Kiilerich <mads@kiilerich.com> writes:

> On 04/16/2014 08:17 PM, Matt Mackall wrote:
>> On Wed, 2014-04-16 at 18:05 +0200, Mads Kiilerich wrote:
>>> # HG changeset patch
>>> # User Mads Kiilerich <madski@unity3d.com>
>>> # Date 1397603408 -7200
>>> #      Wed Apr 16 01:10:08 2014 +0200
>>> # Node ID c0c12f7dbe25a3b8294905b072f6f27291ad26b4
>>> # Parent  319a4229e58fba95bea881cba07a62a726723227
>>> convert: backout 81cf597dafa9 and a3545c3104aa -closemap
>> These are queued for default, thanks.
>>
>> We should probably post (or put in contrib) a simple commented shim
>> extension that does dummy mappings on tags, authors, branches,
>> descriptions..
>
> That is pretty much what I tried to do at 
> http://mercurial.selenic.com/wiki/ConvertExtension#Customization - do 
> you have other something else in mind?

That's pretty awesome. I'll try to put my use case through it and see
how it goes.
Simon King - April 17, 2014, 9:16 a.m.
On Wed, Apr 16, 2014 at 5:05 PM, Mads Kiilerich <mads@kiilerich.com> wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1397603408 -7200
> #      Wed Apr 16 01:10:08 2014 +0200
> # Node ID c0c12f7dbe25a3b8294905b072f6f27291ad26b4
> # Parent  319a4229e58fba95bea881cba07a62a726723227
> convert: backout 81cf597dafa9 and a3545c3104aa -closemap
>
> Closemap solves a very specific use case. It would be better to have a more
> generic solution than to have to maintain this forever.
>
> Closemap has not been released yet and removing it now will not break any
> backward compatibility contract.
>
> There is no test coverage for closemap but it seems like the same can be
> achieved with a simple and much more powerful custom extension:
>
> import hgext.convert.hg
> class source(hgext.convert.hg.mercurial_source):
>     def getcommit(self, rev):
>         c = super(source, self).getcommit(rev)
>         if rev in ['''
> d643f67092ff123f6a192d52f12e7d123dae229f
> 9117c6561b0bd7792fa13b50d28239d51b78e51f
> f368a1c302d5b87506f7edb13769e591e063d7ea
> ''']:

This doesn't look quite right to me (a list containing a single
string). Shouldn't it be:

        if rev in '''
d643f67092ff123f6a192d52f12e7d123dae229f
9117c6561b0bd7792fa13b50d28239d51b78e51f
f368a1c302d5b87506f7edb13769e591e063d7ea
'''.split():

Should I make that change on the wiki?

Simon
Mads Kiilerich - April 17, 2014, 11:26 a.m.
On 04/17/2014 11:16 AM, Simon King wrote:
> On Wed, Apr 16, 2014 at 5:05 PM, Mads Kiilerich <mads@kiilerich.com> wrote:
>> # HG changeset patch
>> # User Mads Kiilerich <madski@unity3d.com>
>> # Date 1397603408 -7200
>> #      Wed Apr 16 01:10:08 2014 +0200
>> # Node ID c0c12f7dbe25a3b8294905b072f6f27291ad26b4
>> # Parent  319a4229e58fba95bea881cba07a62a726723227
>> convert: backout 81cf597dafa9 and a3545c3104aa -closemap
>>
>> Closemap solves a very specific use case. It would be better to have a more
>> generic solution than to have to maintain this forever.
>>
>> Closemap has not been released yet and removing it now will not break any
>> backward compatibility contract.
>>
>> There is no test coverage for closemap but it seems like the same can be
>> achieved with a simple and much more powerful custom extension:
>>
>> import hgext.convert.hg
>> class source(hgext.convert.hg.mercurial_source):
>>      def getcommit(self, rev):
>>          c = super(source, self).getcommit(rev)
>>          if rev in ['''
>> d643f67092ff123f6a192d52f12e7d123dae229f
>> 9117c6561b0bd7792fa13b50d28239d51b78e51f
>> f368a1c302d5b87506f7edb13769e591e063d7ea
>> ''']:
> This doesn't look quite right to me (a list containing a single
> string).

Yes, that was a bug in an "obviously correct" change, last minute and 
after testing ...

> Shouldn't it be:
>
>          if rev in '''
> d643f67092ff123f6a192d52f12e7d123dae229f
> 9117c6561b0bd7792fa13b50d28239d51b78e51f
> f368a1c302d5b87506f7edb13769e591e063d7ea
> '''.split():

almost - except for the .split which in this case (fixed length elements 
with separators) isn't necessary.

I have updated the wiki. Thanks!

/Mads

Patch

diff --git a/hgext/convert/__init__.py b/hgext/convert/__init__.py
--- a/hgext/convert/__init__.py
+++ b/hgext/convert/__init__.py
@@ -142,10 +142,6 @@  def convert(ui, src, dest=None, revmapfi
     branch names. This can be used to (for instance) move code in one
     repository from "default" to a named branch.
 
-    The closemap is a file that allows closing of a branch. This is useful if
-    you want to close a branch. Each entry contains a revision or hash
-    separated by white space.
-
     Mercurial Source
     ################
 
@@ -324,8 +320,6 @@  cmdtable = {
            _('splice synthesized history into place'), _('FILE')),
           ('', 'branchmap', '',
            _('change branch names while converting'), _('FILE')),
-          ('', 'closemap', '',
-           _('closes given revs'), _('FILE')),
           ('', 'branchsort', None, _('try to sort changesets by branches')),
           ('', 'datesort', None, _('try to sort changesets by date')),
           ('', 'sourcesort', None, _('preserve source changesets order')),
diff --git a/hgext/convert/convcmd.py b/hgext/convert/convcmd.py
--- a/hgext/convert/convcmd.py
+++ b/hgext/convert/convcmd.py
@@ -120,42 +120,6 @@  class converter(object):
 
         self.splicemap = self.parsesplicemap(opts.get('splicemap'))
         self.branchmap = mapfile(ui, opts.get('branchmap'))
-        self.closemap = self.parseclosemap(opts.get('closemap'))
-
-    def parseclosemap(self, path):
-        """ check and validate the closemap format and
-            return a list of revs to close.
-            Format checking has two parts.
-            1. generic format which is same across all source types
-            2. specific format checking which may be different for
-               different source type.  This logic is implemented in
-               checkrevformat function in source files like
-               hg.py, subversion.py etc.
-        """
-
-        if not path:
-            return []
-        m = []
-        try:
-            fp = open(path, 'r')
-            for i, line in enumerate(fp):
-                line = line.splitlines()[0].rstrip()
-                if not line:
-                    # Ignore blank lines
-                    continue
-                # split line
-                lex = shlex.shlex(line, posix=True)
-                lex.whitespace_split = True
-                lex.whitespace += ','
-                line = list(lex)
-                for part in line:
-                    self.source.checkrevformat(part, 'closemap')
-                m.extend(line)
-        # if file does not exist or error reading, exit
-        except IOError:
-            raise util.Abort(_('closemap file not found or error reading %s:')
-                               % path)
-        return m
 
     def parsesplicemap(self, path):
         """ check and validate the splicemap format and
@@ -444,9 +408,6 @@  class converter(object):
         except KeyError:
             parents = [b[0] for b in pbranches]
         source = progresssource(self.ui, self.source, len(files))
-        if self.closemap and rev in self.closemap:
-            commit.extra['close'] = 1
-
         newnode = self.dest.putcommit(files, copies, parents, commit,
                                       source, self.map)
         source.close()
diff --git a/tests/test-convert.t b/tests/test-convert.t
--- a/tests/test-convert.t
+++ b/tests/test-convert.t
@@ -122,10 +122,6 @@ 
       can be used to (for instance) move code in one repository from "default"
       to a named branch.
   
-      The closemap is a file that allows closing of a branch. This is useful if
-      you want to close a branch. Each entry contains a revision or hash
-      separated by white space.
-  
       Mercurial Source
       ################
   
@@ -271,7 +267,6 @@ 
       --filemap FILE     remap file names using contents of file
       --splicemap FILE   splice synthesized history into place
       --branchmap FILE   change branch names while converting
-      --closemap FILE    closes given revs
       --branchsort       try to sort changesets by branches
       --datesort         try to sort changesets by date
       --sourcesort       preserve source changesets order