Patchwork commands: remove unecessary copying of list in graft()

login
register
mail settings
Submitter Hannes Oldenburg
Date Sept. 5, 2016, 9:59 a.m.
Message ID <c072f8ecd0021e849360.1473069540@localhost.localdomain>
Download mbox | patch
Permalink /patch/16548/
State Accepted
Headers show

Comments

Hannes Oldenburg - Sept. 5, 2016, 9:59 a.m.
# HG changeset patch
# User Hannes Oldenburg <hannes.christian.oldenburg@gmail.com>
# Date 1473064176 0
#      Mon Sep 05 08:29:36 2016 +0000
# Node ID c072f8ecd0021e849360c8ffdcb8a5a973beb01f
# Parent  f148bfa40489269be2e48046734f81065129847a
commands: remove unecessary copying of list in graft()
Yuya Nishihara - Sept. 6, 2016, 2:23 p.m.
On Mon, 05 Sep 2016 09:59:00 +0000, Hannes Oldenburg wrote:
> # HG changeset patch
> # User Hannes Oldenburg <hannes.christian.oldenburg@gmail.com>
> # Date 1473064176 0
> #      Mon Sep 05 08:29:36 2016 +0000
> # Node ID c072f8ecd0021e849360c8ffdcb8a5a973beb01f
> # Parent  f148bfa40489269be2e48046734f81065129847a
> commands: remove unecessary copying of list in graft()

Confirmed it's no longer an issue since 9271630f4720. Queued, thanks.
Ryan McElroy - Sept. 6, 2016, 3:01 p.m.
On 9/5/2016 10:59 AM, Hannes Oldenburg wrote:
> # HG changeset patch
> # User Hannes Oldenburg <hannes.christian.oldenburg@gmail.com>
> # Date 1473064176 0
> #      Mon Sep 05 08:29:36 2016 +0000
> # Node ID c072f8ecd0021e849360c8ffdcb8a5a973beb01f
> # Parent  f148bfa40489269be2e48046734f81065129847a
> commands: remove unecessary copying of list in graft()
>
> diff -r f148bfa40489 -r c072f8ecd002 mercurial/commands.py
> --- a/mercurial/commands.py	Tue Jul 05 09:37:07 2016 +0200
> +++ b/mercurial/commands.py	Mon Sep 05 08:29:36 2016 +0000
> @@ -4141,9 +4141,7 @@
>           # check for ancestors of dest branch
>           crev = repo['.'].rev()
>           ancestors = repo.changelog.ancestors([crev], inclusive=True)
> -        # Cannot use x.remove(y) on smart set, this has to be a list.
>           # XXX make this lazy in the future

Shouldn't this XXX line also be removed?

> -        revs = list(revs)
>           # don't mutate while iterating, create a copy
>           for rev in list(revs):
>               if rev in ancestors:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=DQIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Jw8rundaE7TbmqBYd1txIQ&m=jnTe5CaoRomlfhv96mwsuyeSe8rRozVl7DLWxwYkKeI&s=RbSbOHawubwwxGeXYiiqs9CzVHMgYRE5DpWOrzTmrJg&e=
Yuya Nishihara - Sept. 6, 2016, 3:28 p.m.
On Tue, 6 Sep 2016 16:01:17 +0100, Ryan McElroy wrote:
> On 9/5/2016 10:59 AM, Hannes Oldenburg wrote:
> > # HG changeset patch
> > # User Hannes Oldenburg <hannes.christian.oldenburg@gmail.com>
> > # Date 1473064176 0
> > #      Mon Sep 05 08:29:36 2016 +0000
> > # Node ID c072f8ecd0021e849360c8ffdcb8a5a973beb01f
> > # Parent  f148bfa40489269be2e48046734f81065129847a
> > commands: remove unecessary copying of list in graft()
> >
> > diff -r f148bfa40489 -r c072f8ecd002 mercurial/commands.py
> > --- a/mercurial/commands.py	Tue Jul 05 09:37:07 2016 +0200
> > +++ b/mercurial/commands.py	Mon Sep 05 08:29:36 2016 +0000
> > @@ -4141,9 +4141,7 @@
> >           # check for ancestors of dest branch
> >           crev = repo['.'].rev()
> >           ancestors = repo.changelog.ancestors([crev], inclusive=True)
> > -        # Cannot use x.remove(y) on smart set, this has to be a list.
> >           # XXX make this lazy in the future
> 
> Shouldn't this XXX line also be removed?

I think this comment is still legit. Probably we wouldn't want to convert
'revs' to a list at all.

Patch

diff -r f148bfa40489 -r c072f8ecd002 mercurial/commands.py
--- a/mercurial/commands.py	Tue Jul 05 09:37:07 2016 +0200
+++ b/mercurial/commands.py	Mon Sep 05 08:29:36 2016 +0000
@@ -4141,9 +4141,7 @@ 
         # check for ancestors of dest branch
         crev = repo['.'].rev()
         ancestors = repo.changelog.ancestors([crev], inclusive=True)
-        # Cannot use x.remove(y) on smart set, this has to be a list.
         # XXX make this lazy in the future
-        revs = list(revs)
         # don't mutate while iterating, create a copy
         for rev in list(revs):
             if rev in ancestors: