Patchwork templater: do not pre-evaluate generator keyword at runsymbol (issue4868)

login
register
mail settings
Submitter Yuya Nishihara
Date Oct. 8, 2015, 3:42 p.m.
Message ID <5103e4e21355334459cc.1444318951@mimosa>
Download mbox | patch
Permalink /patch/10878/
State Accepted
Headers show

Comments

Yuya Nishihara - Oct. 8, 2015, 3:42 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1444314278 -32400
#      Thu Oct 08 23:24:38 2015 +0900
# Node ID 5103e4e21355334459cc1e5d648273eca1cd35c1
# Parent  89b4acf69d7619220dda12fc8a9482587746310d
templater: do not pre-evaluate generator keyword at runsymbol (issue4868)

It was introduced by e06e9fd2d99f, but the important code was removed by
a3c2d9211294. So there was no positive effect other than exhausting memory.

The problem spotted by e06e9fd2d99f is that you can't use a generator keyword
more than once. For example, in hgweb template, "{child} {child}" doesn't work
because the first "{child}" consumes the generator. But as a3c2d9211294 says,
the fix was wrong because it could overwrite a callable keyword that returns
a generator. Also the fix didn't work for a generator of generator such as
"{diff}" keyword. So, the proper fix for that problem would be to not put
a generator in a keyword table. Instead, it should be a factory of a generator.

Note that this should fix the memory issue in hgweb, but my firefox killed by
OOM in place. Be careful to not use a modern web browser to test the issue4868.
Augie Fackler - Oct. 8, 2015, 5:37 p.m.
On Fri, Oct 09, 2015 at 12:42:31AM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1444314278 -32400
> #      Thu Oct 08 23:24:38 2015 +0900
> # Node ID 5103e4e21355334459cc1e5d648273eca1cd35c1
> # Parent  89b4acf69d7619220dda12fc8a9482587746310d
> templater: do not pre-evaluate generator keyword at runsymbol (issue4868)
>

queued for stable, many thanks

>
> It was introduced by e06e9fd2d99f, but the important code was removed by
> a3c2d9211294. So there was no positive effect other than exhausting memory.
>
> The problem spotted by e06e9fd2d99f is that you can't use a generator keyword
> more than once. For example, in hgweb template, "{child} {child}" doesn't work
> because the first "{child}" consumes the generator. But as a3c2d9211294 says,
> the fix was wrong because it could overwrite a callable keyword that returns
> a generator. Also the fix didn't work for a generator of generator such as
> "{diff}" keyword. So, the proper fix for that problem would be to not put
> a generator in a keyword table. Instead, it should be a factory of a generator.
>
> Note that this should fix the memory issue in hgweb, but my firefox killed by
> OOM in place. Be careful to not use a modern web browser to test the issue4868.
>
> diff --git a/mercurial/templater.py b/mercurial/templater.py
> --- a/mercurial/templater.py
> +++ b/mercurial/templater.py
> @@ -237,8 +237,6 @@ def runsymbol(context, mapping, key):
>              v = ''
>      if callable(v):
>          return v(**mapping)
> -    if isinstance(v, types.GeneratorType):
> -        v = list(v)
>      return v
>
>  def buildtemplate(exp, context):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/templater.py b/mercurial/templater.py
--- a/mercurial/templater.py
+++ b/mercurial/templater.py
@@ -237,8 +237,6 @@  def runsymbol(context, mapping, key):
             v = ''
     if callable(v):
         return v(**mapping)
-    if isinstance(v, types.GeneratorType):
-        v = list(v)
     return v
 
 def buildtemplate(exp, context):