Patchwork [1,of,2,V3] commitextras: move fb extension to core which add extras to a commit

login
register
mail settings
Submitter Pulkit Goyal
Date July 14, 2017, 8:01 p.m.
Message ID <e51d188da49636884ae6.1500062483@workspace>
Download mbox | patch
Permalink /patch/22373/
State Accepted
Headers show

Comments

Pulkit Goyal - July 14, 2017, 8:01 p.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1499799225 -19800
#      Wed Jul 12 00:23:45 2017 +0530
# Node ID e51d188da49636884ae6c0df94f501e84436b857
# Parent  80e1331a7fe970f3e56fde9044949d72d3afdf30
# EXP-Topic fbext
commitextras: move fb extension to core which add extras to a commit

This patch moves the Facebook extension to add extra fields to a commit to a
in-core extension.
Yuya Nishihara - July 17, 2017, 2:10 p.m.
On Sat, 15 Jul 2017 01:31:23 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1499799225 -19800
> #      Wed Jul 12 00:23:45 2017 +0530
> # Node ID e51d188da49636884ae6c0df94f501e84436b857
> # Parent  80e1331a7fe970f3e56fde9044949d72d3afdf30
> # EXP-Topic fbext
> commitextras: move fb extension to core which add extras to a commit

I think it's okay to start this as an extension, but I don't think this
extension should be advertised. So +1 for queueing this and marking it as
EXPERIMENTAL or ADVANCED as follow-up.

I'm also fine with adding a hidden config knob to unblock this feature.
Augie Fackler - July 17, 2017, 2:17 p.m.
On Mon, Jul 17, 2017 at 11:10:02PM +0900, Yuya Nishihara wrote:
> On Sat, 15 Jul 2017 01:31:23 +0530, Pulkit Goyal wrote:
> > # HG changeset patch
> > # User Pulkit Goyal <7895pulkit@gmail.com>
> > # Date 1499799225 -19800
> > #      Wed Jul 12 00:23:45 2017 +0530
> > # Node ID e51d188da49636884ae6c0df94f501e84436b857
> > # Parent  80e1331a7fe970f3e56fde9044949d72d3afdf30
> > # EXP-Topic fbext
> > commitextras: move fb extension to core which add extras to a commit
>
> I think it's okay to start this as an extension, but I don't think this
> extension should be advertised. So +1 for queueing this and marking it as
> EXPERIMENTAL or ADVANCED as follow-up.
>
> I'm also fine with adding a hidden config knob to unblock this feature.

I'm also fine with either approach. It makes me nervous, but we
already should be robust to corrupt extras entries in the name of
being futureproof, so let's just proceed instead of being afraid and
letting that hinder potential nice things.

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - July 17, 2017, 2:48 p.m.
On Mon, 17 Jul 2017 10:17:38 -0400, Augie Fackler wrote:
> On Mon, Jul 17, 2017 at 11:10:02PM +0900, Yuya Nishihara wrote:
> > On Sat, 15 Jul 2017 01:31:23 +0530, Pulkit Goyal wrote:
> > > # HG changeset patch
> > > # User Pulkit Goyal <7895pulkit@gmail.com>
> > > # Date 1499799225 -19800
> > > #      Wed Jul 12 00:23:45 2017 +0530
> > > # Node ID e51d188da49636884ae6c0df94f501e84436b857
> > > # Parent  80e1331a7fe970f3e56fde9044949d72d3afdf30
> > > # EXP-Topic fbext
> > > commitextras: move fb extension to core which add extras to a commit
> >
> > I think it's okay to start this as an extension, but I don't think this
> > extension should be advertised. So +1 for queueing this and marking it as
> > EXPERIMENTAL or ADVANCED as follow-up.
> >
> > I'm also fine with adding a hidden config knob to unblock this feature.
> 
> I'm also fine with either approach. It makes me nervous, but we
> already should be robust to corrupt extras entries in the name of
> being futureproof, so let's just proceed instead of being afraid and
> letting that hinder potential nice things.

Queued these, thanks.
Yuya Nishihara - July 17, 2017, 2:50 p.m.
On Sat, 15 Jul 2017 01:31:23 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1499799225 -19800
> #      Wed Jul 12 00:23:45 2017 +0530
> # Node ID e51d188da49636884ae6c0df94f501e84436b857
> # Parent  80e1331a7fe970f3e56fde9044949d72d3afdf30
> # EXP-Topic fbext
> commitextras: move fb extension to core which add extras to a commit

> +def _commit(orig, ui, repo, *pats, **opts):
> +    origcommit = repo.commit
> +    try:
> +        def _wrappedcommit(*innerpats, **inneropts):
> +            extras = opts.get('extra')
> +            if extras:
> +                for raw in extras:
> +                    k, v = raw.split('=', 1)
> +                    inneropts['extra'][k] = v
> +            return origcommit(*innerpats, **inneropts)
> +
> +        # This __dict__ logic is needed because the normal
> +        # extension.wrapfunction doesn't seem to work.
> +        repo.__dict__['commit'] = _wrappedcommit
> +        return orig(ui, repo, *pats, **opts)

Perhaps we'll need a hook point in command layer.

Patch

diff --git a/hgext/commitextras.py b/hgext/commitextras.py
new file mode 100644
--- /dev/null
+++ b/hgext/commitextras.py
@@ -0,0 +1,45 @@ 
+# commitextras.py
+#
+# Copyright 2013 Facebook, Inc.
+#
+# This software may be used and distributed according to the terms of the
+# GNU General Public License version 2 or any later version.
+
+'''adds a new flag extras to commit'''
+
+from __future__ import absolute_import
+
+from mercurial.i18n import _
+from mercurial import (
+    commands,
+    extensions,
+    registrar,
+)
+
+cmdtable = {}
+command = registrar.command(cmdtable)
+testedwith = 'ships-with-hg-core'
+
+def extsetup(ui):
+    entry = extensions.wrapcommand(commands.table, 'commit', _commit)
+    options = entry[1]
+    options.append(('', 'extra', [],
+        _('set a changeset\'s extra values'), _("KEY=VALUE")))
+
+def _commit(orig, ui, repo, *pats, **opts):
+    origcommit = repo.commit
+    try:
+        def _wrappedcommit(*innerpats, **inneropts):
+            extras = opts.get('extra')
+            if extras:
+                for raw in extras:
+                    k, v = raw.split('=', 1)
+                    inneropts['extra'][k] = v
+            return origcommit(*innerpats, **inneropts)
+
+        # This __dict__ logic is needed because the normal
+        # extension.wrapfunction doesn't seem to work.
+        repo.__dict__['commit'] = _wrappedcommit
+        return orig(ui, repo, *pats, **opts)
+    finally:
+        del repo.__dict__['commit']
diff --git a/tests/test-help.t b/tests/test-help.t
--- a/tests/test-help.t
+++ b/tests/test-help.t
@@ -254,6 +254,7 @@ 
        censor        erase file content at a given revision
        churn         command to display statistics about repository history
        clonebundles  advertise pre-generated bundles to seed clones
+       commitextras  adds a new flag extras to commit
        convert       import revisions from foreign VCS repositories into
                      Mercurial
        eol           automatically manage newlines in repository files