| Submitter | Gregory Szorc |
|---|---|
| Date | June 23, 2014, 5:26 a.m. |
| Message ID | <f420697523a0ae7140e8.1403501180@77.1.168.192.in-addr.arpa> |
| Download | mbox | patch |
| Permalink | /patch/5043/ |
| State | Rejected |
| Headers | show |
Comments
Gregory Szorc writes: > # HG changeset patch > # User Gregory Szorc <gregory.szorc@gmail.com> > # Date 1403501129 25200 > # Sun Jun 22 22:25:29 2014 -0700 > # Node ID f420697523a0ae7140e86ccc013e31682bcf4ee3 > # Parent cd3c79392056a0d965236e4986a7a4b5d580f3e5 > patch: accept non-node arguments to diff() > > patch.diff() appears to work with non-bin argument values to node1 > and node2 because it feeds them into repo[]. However, this breaks > the actual generated patch because the arguments are always > hex-encoded. This may result in double hex encoding. > > I discovered this as part of implementing an extension. I have no > clue if anything in the core is impacted. I doubt it. One less > footgun for extension authors. > > diff --git a/mercurial/patch.py b/mercurial/patch.py > --- a/mercurial/patch.py > +++ b/mercurial/patch.py > @@ -1620,9 +1620,9 @@ def diff(repo, node1=None, node2=None, m > return [] > > revs = None > hexfunc = repo.ui.debugflag and hex or short > - revs = [hexfunc(node) for node in [node1, node2] if node] > + revs = [hexfunc(ctx.node()) for ctx in [ctx1, ctx2] if ctx.node()] This will conflict with my patch series here: http://www.selenic.com/pipermail/mercurial-devel/2014-June/059689.html
On Mon, 2014-06-23 at 10:37 -0500, Sean Farley wrote: > Gregory Szorc writes: > > > # HG changeset patch > > # User Gregory Szorc <gregory.szorc@gmail.com> > > # Date 1403501129 25200 > > # Sun Jun 22 22:25:29 2014 -0700 > > # Node ID f420697523a0ae7140e86ccc013e31682bcf4ee3 > > # Parent cd3c79392056a0d965236e4986a7a4b5d580f3e5 > > patch: accept non-node arguments to diff() > > > > patch.diff() appears to work with non-bin argument values to node1 > > and node2 because it feeds them into repo[]. However, this breaks > > the actual generated patch because the arguments are always > > hex-encoded. This may result in double hex encoding. > > > > I discovered this as part of implementing an extension. I have no > > clue if anything in the core is impacted. I doubt it. One less > > footgun for extension authors. > > > > diff --git a/mercurial/patch.py b/mercurial/patch.py > > --- a/mercurial/patch.py > > +++ b/mercurial/patch.py > > @@ -1620,9 +1620,9 @@ def diff(repo, node1=None, node2=None, m > > return [] > > > > revs = None > > hexfunc = repo.ui.debugflag and hex or short > > - revs = [hexfunc(node) for node in [node1, node2] if node] > > + revs = [hexfunc(ctx.node()) for ctx in [ctx1, ctx2] if ctx.node()] > > This will conflict with my patch series here: > http://www.selenic.com/pipermail/mercurial-devel/2014-June/059689.html More importantly, your conflicting patch apparently also fixes the problem.
Patch
diff --git a/mercurial/patch.py b/mercurial/patch.py --- a/mercurial/patch.py +++ b/mercurial/patch.py @@ -1620,9 +1620,9 @@ def diff(repo, node1=None, node2=None, m return [] revs = None hexfunc = repo.ui.debugflag and hex or short - revs = [hexfunc(node) for node in [node1, node2] if node] + revs = [hexfunc(ctx.node()) for ctx in [ctx1, ctx2] if ctx.node()] copy = {} if opts.git or opts.upgrade: copy = copies.pathcopies(ctx1, ctx2)