Patchwork cleanupnode: do not use generator for node mapping

login
register
mail settings
Submitter Boris Feld
Date July 9, 2017, 4:49 p.m.
Message ID <a6ed7f0670010e5a6551.1499618944@FB>
Download mbox | patch
Permalink /patch/22158/
State Accepted
Headers show

Comments

Boris Feld - July 9, 2017, 4:49 p.m.
# HG changeset patch
# User Octobus <contact@octobus.net>
# Date 1499605879 -7200
#      Sun Jul 09 15:11:19 2017 +0200
# Node ID a6ed7f0670010e5a6551d736b00dc0bf7367bba8
# Parent  4672db164c986da4442bd864cd044512d975c3f2
# EXP-Topic fix-cleanup
cleanupnode: do not use generator for node mapping

The 'successors' part of the mappings used of be a tuple. This avoid issue from
code consuming the generator "by mistake". For example, an extension inspecting the
mapping content used to be able to iterate over the successors mapping without
consequence.

Since the mapping are small we do not expect any performance impact we use tuple
again for this.
Jun Wu - July 9, 2017, 5:06 p.m.
Looks good to me. Thanks for the cleanup!

Excerpts from Boris Feld's message of 2017-07-09 18:49:04 +0200:
> # HG changeset patch
> # User Octobus <contact@octobus.net>
> # Date 1499605879 -7200
> #      Sun Jul 09 15:11:19 2017 +0200
> # Node ID a6ed7f0670010e5a6551d736b00dc0bf7367bba8
> # Parent  4672db164c986da4442bd864cd044512d975c3f2
> # EXP-Topic fix-cleanup
> cleanupnode: do not use generator for node mapping
> 
> The 'successors' part of the mappings used of be a tuple. This avoid issue from
> code consuming the generator "by mistake". For example, an extension inspecting the
> mapping content used to be able to iterate over the successors mapping without
> consequence.
> 
> Since the mapping are small we do not expect any performance impact we use tuple
> again for this.
> 
> diff -r 4672db164c98 -r a6ed7f067001 mercurial/scmutil.py
> --- a/mercurial/scmutil.py    Sat Jun 24 15:29:42 2017 -0700
> +++ b/mercurial/scmutil.py    Sun Jul 09 15:11:19 2017 +0200
> @@ -638,7 +638,7 @@
>              isobs = unfi.obsstore.successors.__contains__
>              torev = unfi.changelog.rev
>              sortfunc = lambda ns: torev(ns[0])
> -            rels = [(unfi[n], (unfi[m] for m in s))
> +            rels = [(unfi[n], tuple(unfi[m] for m in s))
>                      for n, s in sorted(mapping.items(), key=sortfunc)
>                      if s or not isobs(n)]
>              obsolete.createmarkers(repo, rels, operation=operation)
via Mercurial-devel - July 9, 2017, 7:55 p.m.
On Sun, Jul 9, 2017 at 10:06 AM, Jun Wu <quark@fb.com> wrote:
> Looks good to me. Thanks for the cleanup!

Yep. Queued, thanks!

>
> Excerpts from Boris Feld's message of 2017-07-09 18:49:04 +0200:
>> # HG changeset patch
>> # User Octobus <contact@octobus.net>
>> # Date 1499605879 -7200
>> #      Sun Jul 09 15:11:19 2017 +0200
>> # Node ID a6ed7f0670010e5a6551d736b00dc0bf7367bba8
>> # Parent  4672db164c986da4442bd864cd044512d975c3f2
>> # EXP-Topic fix-cleanup
>> cleanupnode: do not use generator for node mapping
>>
>> The 'successors' part of the mappings used of be a tuple. This avoid issue from
>> code consuming the generator "by mistake". For example, an extension inspecting the
>> mapping content used to be able to iterate over the successors mapping without
>> consequence.
>>
>> Since the mapping are small we do not expect any performance impact we use tuple
>> again for this.
>>
>> diff -r 4672db164c98 -r a6ed7f067001 mercurial/scmutil.py
>> --- a/mercurial/scmutil.py    Sat Jun 24 15:29:42 2017 -0700
>> +++ b/mercurial/scmutil.py    Sun Jul 09 15:11:19 2017 +0200
>> @@ -638,7 +638,7 @@
>>              isobs = unfi.obsstore.successors.__contains__
>>              torev = unfi.changelog.rev
>>              sortfunc = lambda ns: torev(ns[0])
>> -            rels = [(unfi[n], (unfi[m] for m in s))
>> +            rels = [(unfi[n], tuple(unfi[m] for m in s))
>>                      for n, s in sorted(mapping.items(), key=sortfunc)
>>                      if s or not isobs(n)]
>>              obsolete.createmarkers(repo, rels, operation=operation)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff -r 4672db164c98 -r a6ed7f067001 mercurial/scmutil.py
--- a/mercurial/scmutil.py	Sat Jun 24 15:29:42 2017 -0700
+++ b/mercurial/scmutil.py	Sun Jul 09 15:11:19 2017 +0200
@@ -638,7 +638,7 @@ 
             isobs = unfi.obsstore.successors.__contains__
             torev = unfi.changelog.rev
             sortfunc = lambda ns: torev(ns[0])
-            rels = [(unfi[n], (unfi[m] for m in s))
+            rels = [(unfi[n], tuple(unfi[m] for m in s))
                     for n, s in sorted(mapping.items(), key=sortfunc)
                     if s or not isobs(n)]
             obsolete.createmarkers(repo, rels, operation=operation)