Use jinja2 for make_runtime_features and make_internal_runtime_flags

Use jinja2 for make_runtime_features and make_internal_runtime_flags

Begin moving python code generators to jinja2.
Some previous discussion on https://codereview.chromium.org/14905002/.

BUG=

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=150285

[Timothy Loh, 2013-05-10 07:50:19]

Hi guys,

Now that jinja2 is in chromium's third_party, I'm starting to move over the
existing python code generators to use it, so expect to see patches for those
over the next couple of days, as well as for my StyleBuilder generator.

Including jinja2 is still slightly awkward, but it looks like the rest of code
in chromium using third_party python libraries does it the same way.

[abarth-chromium, 2013-05-10 16:18:40]

Looks cool!  I'll let Eric give the official stamp.

https://codereview.chromium.org/15095002/diff/1/Source/core/scripts/template_...
File Source/core/scripts/template_expander.py (right):

https://codereview.chromium.org/15095002/diff/1/Source/core/scripts/template_...
Source/core/scripts/template_expander.py:37: def applytemplate(pathtotemplate,
params):
applytemplate -> apply_template
pathtotemplate -> path_to_template

(We use PEP8 style, except for the line length limit)

https://codereview.chromium.org/15095002/diff/1/Source/core/scripts/templates...
File Source/core/scripts/templates/InternalRuntimeFlags.h.tmpl (right):

https://codereview.chromium.org/15095002/diff/1/Source/core/scripts/templates...
Source/core/scripts/templates/InternalRuntimeFlags.h.tmpl:3: {{ license }}
We should probably just make this a subtemplate.  It's a constant.  There's no
reason to bring in the value via script.

[Timothy Loh, 2013-05-13 03:13:08]

On 2013/05/10 16:18:40, abarth wrote:
> applytemplate -> apply_template
> pathtotemplate -> path_to_template

Fixed

>
https://codereview.chromium.org/15095002/diff/1/Source/core/scripts/templates...
> Source/core/scripts/templates/InternalRuntimeFlags.h.tmpl:3: {{ license }}
> We should probably just make this a subtemplate.  It's a constant.  There's no
> reason to bring in the value via script.

Moved to macros.tmpl. This probably is only constant until 2014 ;)

eseidel: r?

[eseidel, 2013-05-13 06:38:33]

This looks OK.  I'm sure we'll iterate more.  the generate_header thing is
confusing to me still.

https://codereview.chromium.org/15095002/diff/7001/Source/core/scripts/make_r...
File Source/core/scripts/make_runtime_features.py (right):

https://codereview.chromium.org/15095002/diff/7001/Source/core/scripts/make_r...
Source/core/scripts/make_runtime_features.py:76: def generate_header(self):
Why are these named "generate header" if they return a dicxtionary?

https://codereview.chromium.org/15095002/diff/7001/Source/core/scripts/templa...
File Source/core/scripts/template_expander.py (right):

https://codereview.chromium.org/15095002/diff/7001/Source/core/scripts/templa...
Source/core/scripts/template_expander.py:38: import jinja2
Why is this import in the function?

https://codereview.chromium.org/15095002/diff/7001/Source/core/scripts/templa...
Source/core/scripts/template_expander.py:41: env =
jinja2.Environment(loader=jinja2.FileSystemLoader([dirname, path_to_templates]))
I would call this jinja_env or something so as not to confuse it with os.env.

[Timothy Loh, 2013-05-13 06:55:00]

On 2013/05/13 06:38:33, Eric Seidel wrote:
> This looks OK.  I'm sure we'll iterate more.  the generate_header thing is
> confusing to me still.
> 
>
https://codereview.chromium.org/15095002/diff/7001/Source/core/scripts/make_r...
> File Source/core/scripts/make_runtime_features.py (right):
> 
>
https://codereview.chromium.org/15095002/diff/7001/Source/core/scripts/make_r...
> Source/core/scripts/make_runtime_features.py:76: def generate_header(self):
> Why are these named "generate header" if they return a dicxtionary?

This is the overriding the same generate_header as was there previously; I've
edited write_file in in_generator.py so that if we have a dictionary then we
apply it as arguments to the templates/(filename).tmpl, so we don't need to
manually call jinja2 in the various generate_* functions. Perhaps it's too
nonobvious and I should just explicitly call the template_expander?

>
https://codereview.chromium.org/15095002/diff/7001/Source/core/scripts/templa...
> Source/core/scripts/template_expander.py:38: import jinja2
> Why is this import in the function?

Fixed

>
https://codereview.chromium.org/15095002/diff/7001/Source/core/scripts/templa...
> Source/core/scripts/template_expander.py:41: env =
> jinja2.Environment(loader=jinja2.FileSystemLoader([dirname,
path_to_templates]))
> I would call this jinja_env or something so as not to confuse it with os.env.

Okay, renamed.

[eseidel, 2013-05-13 07:01:11]

On Sun, May 12, 2013 at 11:55 PM,  <timloh@chromium.org> wrote:
> On 2013/05/13 06:38:33, Eric Seidel wrote:
>>
>> This looks OK.  I'm sure we'll iterate more.  the generate_header thing is
>> confusing to me still.
>
>
>
>
https://codereview.chromium.org/15095002/diff/7001/Source/core/scripts/make_r...
>>
>> File Source/core/scripts/make_runtime_features.py (right):
>
>
>
>
https://codereview.chromium.org/15095002/diff/7001/Source/core/scripts/make_r...
>>
>> Source/core/scripts/make_runtime_features.py:76: def
>> generate_header(self):
>> Why are these named "generate header" if they return a dicxtionary?
>
>
> This is the overriding the same generate_header as was there previously;
> I've
> edited write_file in in_generator.py so that if we have a dictionary then we
> apply it as arguments to the templates/(filename).tmpl, so we don't need to
> manually call jinja2 in the various generate_* functions. Perhaps it's too
> nonobvious and I should just explicitly call the template_expander?

No, it's fine that we use the same pattern, we should just rename the
method.  It doesn't have to be in this patch, but presumably a
follow-up.

def header_template_parameters(self)

would be one possible name.  I'm sure we could think of better/shorter names.

>
>
https://codereview.chromium.org/15095002/diff/7001/Source/core/scripts/templa...
>>
>> Source/core/scripts/template_expander.py:38: import jinja2
>> Why is this import in the function?
>
>
> Fixed
>
>
>
>
https://codereview.chromium.org/15095002/diff/7001/Source/core/scripts/templa...
>>
>> Source/core/scripts/template_expander.py:41: env =
>> jinja2.Environment(loader=jinja2.FileSystemLoader([dirname,
>
> path_to_templates]))
>>
>> I would call this jinja_env or something so as not to confuse it with
>> os.env.
>
>
> Okay, renamed.
>
> https://codereview.chromium.org/15095002/

[Timothy Loh, 2013-05-13 07:16:19]

On 2013/05/13 07:01:11, Eric Seidel wrote:
> No, it's fine that we use the same pattern, we should just rename the
> method.  It doesn't have to be in this patch, but presumably a
> follow-up.
> 
> def header_template_parameters(self)
> 
> would be one possible name.  I'm sure we could think of better/shorter names.

I think we'll still want to be able to return a string for the output (e.g.
make_css_property_names.py would explicitly expand CSSPropertyNames.gperf.tmpl
and run it through gperf), so perhaps we could have both generate_x and
x_template_parameters, and have write_x use whichever returns something (if
either do)?

[Timothy Loh, 2013-05-13 08:16:01]

Okay perhaps we'll leave this patch as it is at the moment and then iterate on
it later. Maybe a better way than having all the possible files to create listed
in in_generator is to simply search the class' methods for those named
generate_*, which would return the output file name and contents, and we could
decorate those which require template expansion?

In any case, could we start with landing this patch? :)

[eseidel, 2013-05-14 00:24:36]

lgtm

Thanks!  Tim says he plans to iterate further on these.  I'm a big fan of doing
things in small patches, so I support that effort!

[commit-bot: I haz the power, 2013-05-14 00:24:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/timloh@chromium.org/15095002/20001

[commit-bot: I haz the power, 2013-05-14 01:07:14]

Retried try job too often on linux_layout_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_layo...

[commit-bot: I haz the power, 2013-05-14 01:15:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/timloh@chromium.org/15095002/20001

[commit-bot: I haz the power, 2013-05-14 03:16:33]

Change committed as 150285