Patchwork [V2] convert: config option to control Git committer actions

login
register
mail settings
Submitter Gregory Szorc
Date Jan. 14, 2017, 7:21 a.m.
Message ID <3cfb4ac1f3c79e876238.1484378483@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/18217/
State Accepted
Headers show

Comments

Gregory Szorc - Jan. 14, 2017, 7:21 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1484378470 28800
#      Fri Jan 13 23:21:10 2017 -0800
# Node ID 3cfb4ac1f3c79e876238855822a88da1b199da99
# Parent  8614546154cbffe8df6a56d53e404491fa092636
convert: config option to control Git committer actions

When converting a Git repository to Mercurial at Mozilla, I encountered
a scenario where I didn't want `hg convert` to automatically add the
"committer: <committer>" line to commit messages. While I can hack around
this by rewriting the Git commit before it is fed into `hg convert`,
I figured it would be a useful knob to control.

This patch introduces a config option that allows lots of control
over the committer value. I initially implemented this as a single
boolean flag to control whether to save the committer message. But
then there was feedback that it would be useful to save the committer
in extra data. While this patch doesn't implement support for saving
in extra data, it does add a mechanism for extending which actions
to take on the committer field. We should be able to easily add
actions to save in extra data.

Some of the implemented features weren't asked for. But I figured they
could be useful. If nothing else they demonstrate the extensibility
of this mechanism.
Yuya Nishihara - Jan. 14, 2017, 1:53 p.m.
On Fri, 13 Jan 2017 23:21:23 -0800, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1484378470 28800
> #      Fri Jan 13 23:21:10 2017 -0800
> # Node ID 3cfb4ac1f3c79e876238855822a88da1b199da99
> # Parent  8614546154cbffe8df6a56d53e404491fa092636
> convert: config option to control Git committer actions

Looks good. Queued this, thanks.

> @@ -327,6 +327,39 @@ def convert(ui, src, dest=None, revmapfi
>          ``convert.git.similarity`` is greater than 0. The default is
>          ``400``.
>  
> +    :convert.git.committeractions: list of actions to take when processing
> +        author and committer values.
> +
> +        Git commits have separate author (who wrote the commit) and committer
> +        (who applied the commit) fields. Not all destinations support separate
> +        author and committer fields (including Mercurial). This config option
> +        controls what to do with these author and committer fields during
> +        conversion.
> +
> +        A value of ``messagedifferent`` will append a ``committer: ...``
> +        line to the commit message if the Git committer is different from the
> +        author. The prefix of that line can be specified using the syntax
> +        ``messagedifferent=<prefix>``. e.g. ``messagedifferent=git-committer:``.
> +        When a prefix is specified, a space will always be inserted between the
> +        prefix and the value.
> +
> +        ``messagealways`` behaves like ``messagedifferent`` except it will
> +        always result in a ``committer: ...`` line being appended to the commit
> +        message. This value is mutually exclusive with ``messagedifferent``.
> +
> +        ``dropcommitter`` will remove references to the committer. Only
> +        references to the author will remain. Actions that add references
> +        to the committer will have no effect when this is set.
> +
> +        ``replaceauthor`` will replace the value of the author field with
> +        the committer. Other actions that add references to the committer
> +        will still take effect when this is set.
> +
> +        ``replacecommitter`` will replace the value of the committer field
> +        with the author.
> +
> +        The default is ``messagewhendifferent``.

I did s/messagewhendifferent/messagedifferent/ in flight.

BTW, I'm a bit curious how replacecommitter could be used.
Gregory Szorc - Jan. 14, 2017, 6:05 p.m.
On Sat, Jan 14, 2017 at 5:53 AM, Yuya Nishihara <yuya@tcha.org> wrote:

> On Fri, 13 Jan 2017 23:21:23 -0800, Gregory Szorc wrote:
> > # HG changeset patch
> > # User Gregory Szorc <gregory.szorc@gmail.com>
> > # Date 1484378470 28800
> > #      Fri Jan 13 23:21:10 2017 -0800
> > # Node ID 3cfb4ac1f3c79e876238855822a88da1b199da99
> > # Parent  8614546154cbffe8df6a56d53e404491fa092636
> > convert: config option to control Git committer actions
>
> Looks good. Queued this, thanks.
>
> > @@ -327,6 +327,39 @@ def convert(ui, src, dest=None, revmapfi
> >          ``convert.git.similarity`` is greater than 0. The default is
> >          ``400``.
> >
> > +    :convert.git.committeractions: list of actions to take when
> processing
> > +        author and committer values.
> > +
> > +        Git commits have separate author (who wrote the commit) and
> committer
> > +        (who applied the commit) fields. Not all destinations support
> separate
> > +        author and committer fields (including Mercurial). This config
> option
> > +        controls what to do with these author and committer fields
> during
> > +        conversion.
> > +
> > +        A value of ``messagedifferent`` will append a ``committer: ...``
> > +        line to the commit message if the Git committer is different
> from the
> > +        author. The prefix of that line can be specified using the
> syntax
> > +        ``messagedifferent=<prefix>``. e.g.
> ``messagedifferent=git-committer:``.
> > +        When a prefix is specified, a space will always be inserted
> between the
> > +        prefix and the value.
> > +
> > +        ``messagealways`` behaves like ``messagedifferent`` except it
> will
> > +        always result in a ``committer: ...`` line being appended to
> the commit
> > +        message. This value is mutually exclusive with
> ``messagedifferent``.
> > +
> > +        ``dropcommitter`` will remove references to the committer. Only
> > +        references to the author will remain. Actions that add
> references
> > +        to the committer will have no effect when this is set.
> > +
> > +        ``replaceauthor`` will replace the value of the author field
> with
> > +        the committer. Other actions that add references to the
> committer
> > +        will still take effect when this is set.
> > +
> > +        ``replacecommitter`` will replace the value of the committer
> field
> > +        with the author.
> > +
> > +        The default is ``messagewhendifferent``.
>
> I did s/messagewhendifferent/messagedifferent/ in flight.
>
> BTW, I'm a bit curious how replacecommitter could be used.
>

Good question! I implemented it out of a habit for completeness. I don't
really see a compelling use case for it. I'll try to send a follow-up patch
to remove it in the next few hours.

Patch

diff --git a/hgext/convert/__init__.py b/hgext/convert/__init__.py
--- a/hgext/convert/__init__.py
+++ b/hgext/convert/__init__.py
@@ -327,6 +327,39 @@  def convert(ui, src, dest=None, revmapfi
         ``convert.git.similarity`` is greater than 0. The default is
         ``400``.
 
+    :convert.git.committeractions: list of actions to take when processing
+        author and committer values.
+
+        Git commits have separate author (who wrote the commit) and committer
+        (who applied the commit) fields. Not all destinations support separate
+        author and committer fields (including Mercurial). This config option
+        controls what to do with these author and committer fields during
+        conversion.
+
+        A value of ``messagedifferent`` will append a ``committer: ...``
+        line to the commit message if the Git committer is different from the
+        author. The prefix of that line can be specified using the syntax
+        ``messagedifferent=<prefix>``. e.g. ``messagedifferent=git-committer:``.
+        When a prefix is specified, a space will always be inserted between the
+        prefix and the value.
+
+        ``messagealways`` behaves like ``messagedifferent`` except it will
+        always result in a ``committer: ...`` line being appended to the commit
+        message. This value is mutually exclusive with ``messagedifferent``.
+
+        ``dropcommitter`` will remove references to the committer. Only
+        references to the author will remain. Actions that add references
+        to the committer will have no effect when this is set.
+
+        ``replaceauthor`` will replace the value of the author field with
+        the committer. Other actions that add references to the committer
+        will still take effect when this is set.
+
+        ``replacecommitter`` will replace the value of the committer field
+        with the author.
+
+        The default is ``messagewhendifferent``.
+
     :convert.git.extrakeys: list of extra keys from commit metadata to copy to
         the destination. Some Git repositories store extra metadata in commits.
         By default, this non-default metadata will be lost during conversion.
diff --git a/hgext/convert/git.py b/hgext/convert/git.py
--- a/hgext/convert/git.py
+++ b/hgext/convert/git.py
@@ -110,6 +110,55 @@  class convert_git(common.converter_sourc
             raise error.Abort(_('copying of extra key is forbidden: %s') %
                               _(', ').join(sorted(banned)))
 
+        committeractions = self.ui.configlist('convert', 'git.committeractions',
+                                              'messagedifferent')
+
+        messagedifferent = None
+        messagealways = None
+        for a in committeractions:
+            if a.startswith(('messagedifferent', 'messagealways')):
+                k = a
+                v = None
+                if '=' in a:
+                    k, v = a.split('=', 1)
+
+                if k == 'messagedifferent':
+                    messagedifferent = v or 'committer:'
+                elif k == 'messagealways':
+                    messagealways = v or 'committer:'
+
+        if messagedifferent and messagealways:
+            raise error.Abort(_('committeractions cannot define both '
+                                'messagedifferent and messagealways'))
+
+        dropcommitter = 'dropcommitter' in committeractions
+        replaceauthor = 'replaceauthor' in committeractions
+        replacecommitter = 'replacecommitter' in committeractions
+
+        if dropcommitter and (replaceauthor or replacecommitter):
+            raise error.Abort(_('committeractions cannot define both '
+                                'dropcommitter and '
+                                'replaceauthor/replacecommitter'))
+
+        if dropcommitter and messagealways:
+            raise error.Abort(_('committeractions cannot define both '
+                                'dropcommitter and messagealways'))
+
+        if replaceauthor and replacecommitter:
+            raise error.Abort(_('committeractions cannot define both '
+                                'replaceauthor and replacecommitter'))
+
+        if not messagedifferent and not messagealways:
+            messagedifferent = 'committer:'
+
+        self.committeractions = {
+            'dropcommitter': dropcommitter,
+            'replaceauthor': replaceauthor,
+            'replacecommitter': replacecommitter,
+            'messagedifferent': messagedifferent,
+            'messagealways': messagealways,
+        }
+
     def after(self):
         for f in self.catfilepipe:
             f.close()
@@ -317,8 +366,22 @@  class convert_git(common.converter_sourc
             if n in self.copyextrakeys:
                 extra[n] = v
 
-        if committer and committer != author:
-            message += "\ncommitter: %s\n" % committer
+        if self.committeractions['dropcommitter']:
+            committer = None
+
+        if self.committeractions['replacecommitter']:
+            committer = author
+        elif self.committeractions['replaceauthor']:
+            author = committer
+
+        if committer:
+            messagealways = self.committeractions['messagealways']
+            messagedifferent = self.committeractions['messagedifferent']
+            if messagealways:
+                message += '\n%s %s\n' % (messagealways, committer)
+            elif messagedifferent and author != committer:
+                message += '\n%s %s\n' % (messagedifferent, committer)
+
         tzs, tzh, tzm = tz[-5:-4] + "1", tz[-4:-2], tz[-2:]
         tz = -int(tzs) * (int(tzh) * 3600 + int(tzm))
         date = tm + " " + str(tz)
diff --git a/tests/test-convert-git.t b/tests/test-convert-git.t
--- a/tests/test-convert-git.t
+++ b/tests/test-convert-git.t
@@ -482,6 +482,229 @@  convert author committer
   
   
 
+Various combinations of committeractions fail
+
+  $ hg --config convert.git.committeractions=messagedifferent,messagealways convert git-repo4 bad-committer
+  initializing destination bad-committer repository
+  abort: committeractions cannot define both messagedifferent and messagealways
+  [255]
+
+  $ hg --config convert.git.committeractions=dropcommitter,replaceauthor convert git-repo4 bad-committer
+  initializing destination bad-committer repository
+  abort: committeractions cannot define both dropcommitter and replaceauthor/replacecommitter
+  [255]
+
+  $ hg --config convert.git.committeractions=dropcommitter,replacecommitter convert git-repo4 bad-committer
+  initializing destination bad-committer repository
+  abort: committeractions cannot define both dropcommitter and replaceauthor/replacecommitter
+  [255]
+
+  $ hg --config convert.git.committeractions=dropcommitter,messagealways convert git-repo4 bad-committer
+  initializing destination bad-committer repository
+  abort: committeractions cannot define both dropcommitter and messagealways
+  [255]
+
+  $ hg --config convert.git.committeractions=replaceauthor,replacecommitter convert git-repo4 bad-committer
+  initializing destination bad-committer repository
+  abort: committeractions cannot define both replaceauthor and replacecommitter
+  [255]
+
+custom prefix on messagedifferent works
+
+  $ hg --config convert.git.committeractions=messagedifferent=different: convert git-repo4 git-repo4-hg-messagedifferentprefix
+  initializing destination git-repo4-hg-messagedifferentprefix repository
+  scanning source...
+  sorting...
+  converting...
+  1 addfoo
+  0 addfoo2
+  updating bookmarks
+
+  $ hg -R git-repo4-hg-messagedifferentprefix log -v
+  changeset:   1:2fe0c98a109d
+  bookmark:    master
+  tag:         tip
+  user:        nottest <test@example.org>
+  date:        Mon Jan 01 00:00:21 2007 +0000
+  files:       foo
+  description:
+  addfoo2
+  
+  different: test <test@example.org>
+  
+  
+  changeset:   0:0735477b0224
+  user:        test <test@example.org>
+  date:        Mon Jan 01 00:00:20 2007 +0000
+  files:       foo
+  description:
+  addfoo
+  
+  
+
+messagealways will always add the "committer: " line even if committer identical
+
+  $ hg --config convert.git.committeractions=messagealways convert git-repo4 git-repo4-hg-messagealways
+  initializing destination git-repo4-hg-messagealways repository
+  scanning source...
+  sorting...
+  converting...
+  1 addfoo
+  0 addfoo2
+  updating bookmarks
+
+  $ hg -R git-repo4-hg-messagealways log -v
+  changeset:   1:8db057d8cd37
+  bookmark:    master
+  tag:         tip
+  user:        nottest <test@example.org>
+  date:        Mon Jan 01 00:00:21 2007 +0000
+  files:       foo
+  description:
+  addfoo2
+  
+  committer: test <test@example.org>
+  
+  
+  changeset:   0:8f71fe9c98be
+  user:        test <test@example.org>
+  date:        Mon Jan 01 00:00:20 2007 +0000
+  files:       foo
+  description:
+  addfoo
+  
+  committer: test <test@example.org>
+  
+  
+
+custom prefix on messagealways works
+
+  $ hg --config convert.git.committeractions=messagealways=always: convert git-repo4 git-repo4-hg-messagealwaysprefix
+  initializing destination git-repo4-hg-messagealwaysprefix repository
+  scanning source...
+  sorting...
+  converting...
+  1 addfoo
+  0 addfoo2
+  updating bookmarks
+
+  $ hg -R git-repo4-hg-messagealwaysprefix log -v
+  changeset:   1:83c17174de79
+  bookmark:    master
+  tag:         tip
+  user:        nottest <test@example.org>
+  date:        Mon Jan 01 00:00:21 2007 +0000
+  files:       foo
+  description:
+  addfoo2
+  
+  always: test <test@example.org>
+  
+  
+  changeset:   0:2ac9bcb3534a
+  user:        test <test@example.org>
+  date:        Mon Jan 01 00:00:20 2007 +0000
+  files:       foo
+  description:
+  addfoo
+  
+  always: test <test@example.org>
+  
+  
+
+replaceauthor replaces author with committer
+
+  $ hg --config convert.git.committeractions=replaceauthor convert git-repo4 git-repo4-hg-replaceauthor
+  initializing destination git-repo4-hg-replaceauthor repository
+  scanning source...
+  sorting...
+  converting...
+  1 addfoo
+  0 addfoo2
+  updating bookmarks
+
+  $ hg -R git-repo4-hg-replaceauthor log -v
+  changeset:   1:122c1d8999ea
+  bookmark:    master
+  tag:         tip
+  user:        test <test@example.org>
+  date:        Mon Jan 01 00:00:21 2007 +0000
+  files:       foo
+  description:
+  addfoo2
+  
+  
+  changeset:   0:0735477b0224
+  user:        test <test@example.org>
+  date:        Mon Jan 01 00:00:20 2007 +0000
+  files:       foo
+  description:
+  addfoo
+  
+  
+
+replacecommitter replaces committer with author
+
+  $ hg --config convert.git.committeractions=replacecommitter convert git-repo4 git-repo4-hg-replacecommitter
+  initializing destination git-repo4-hg-replacecommitter repository
+  scanning source...
+  sorting...
+  converting...
+  1 addfoo
+  0 addfoo2
+  updating bookmarks
+
+  $ hg -R git-repo4-hg-replacecommitter log -v
+  changeset:   1:190b2da396cc
+  bookmark:    master
+  tag:         tip
+  user:        nottest <test@example.org>
+  date:        Mon Jan 01 00:00:21 2007 +0000
+  files:       foo
+  description:
+  addfoo2
+  
+  
+  changeset:   0:0735477b0224
+  user:        test <test@example.org>
+  date:        Mon Jan 01 00:00:20 2007 +0000
+  files:       foo
+  description:
+  addfoo
+  
+  
+
+dropcommitter removes the committer
+
+  $ hg --config convert.git.committeractions=dropcommitter convert git-repo4 git-repo4-hg-dropcommitter
+  initializing destination git-repo4-hg-dropcommitter repository
+  scanning source...
+  sorting...
+  converting...
+  1 addfoo
+  0 addfoo2
+  updating bookmarks
+
+  $ hg -R git-repo4-hg-dropcommitter log -v
+  changeset:   1:190b2da396cc
+  bookmark:    master
+  tag:         tip
+  user:        nottest <test@example.org>
+  date:        Mon Jan 01 00:00:21 2007 +0000
+  files:       foo
+  description:
+  addfoo2
+  
+  
+  changeset:   0:0735477b0224
+  user:        test <test@example.org>
+  date:        Mon Jan 01 00:00:20 2007 +0000
+  files:       foo
+  description:
+  addfoo
+  
+  
+
 --sourceorder should fail
 
   $ hg convert --sourcesort git-repo4 git-repo4-sourcesort-hg
diff --git a/tests/test-convert.t b/tests/test-convert.t
--- a/tests/test-convert.t
+++ b/tests/test-convert.t
@@ -268,6 +268,40 @@ 
                     computation on large projects. The option is only relevant
                     if "convert.git.similarity" is greater than 0. The default
                     is "400".
+      convert.git.committeractions
+                    list of actions to take when processing author and committer
+                    values.
+  
+          Git commits have separate author (who wrote the commit) and committer
+          (who applied the commit) fields. Not all destinations support separate
+          author and committer fields (including Mercurial). This config option
+          controls what to do with these author and committer fields during
+          conversion.
+  
+          A value of "messagedifferent" will append a "committer: ..." line to
+          the commit message if the Git committer is different from the author.
+          The prefix of that line can be specified using the syntax
+          "messagedifferent=<prefix>". e.g. "messagedifferent=git-committer:".
+          When a prefix is specified, a space will always be inserted between
+          the prefix and the value.
+  
+          "messagealways" behaves like "messagedifferent" except it will always
+          result in a "committer: ..." line being appended to the commit
+          message. This value is mutually exclusive with "messagedifferent".
+  
+          "dropcommitter" will remove references to the committer. Only
+          references to the author will remain. Actions that add references to
+          the committer will have no effect when this is set.
+  
+          "replaceauthor" will replace the value of the author field with the
+          committer. Other actions that add references to the committer will
+          still take effect when this is set.
+  
+          "replacecommitter" will replace the value of the committer field with
+          the author.
+  
+          The default is "messagewhendifferent".
+  
       convert.git.extrakeys
                     list of extra keys from commit metadata to copy to the
                     destination. Some Git repositories store extra metadata in