jinja2: support file references in templates.

jinja2: support file references in templates.

This allows jinja2 templates to reference other templates by path name,
e.g. to extend them or include them.

BUG=467069
Committed: https://crrev.com/031bd459554280a3653a1541221e224a6678eda2
Cr-Commit-Position: refs/heads/master@{#320963}

[newt (away), 2015-03-13 17:44:21]

newt@chromium.org changed reviewers:
+ cjhopman@chromium.org, jbudorick@chromium.org

[newt (away), 2015-03-13 17:44:21]

PTAL

[cjhopman, 2015-03-14 00:41:26]

lgtm

[newt (away), 2015-03-14 00:41:43]

The CQ bit was checked by newt@chromium.org

[commit-bot: I haz the power, 2015-03-14 00:42:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1002313002/20001

[cjhopman, 2015-03-14 00:43:11]

The CQ bit was unchecked by cjhopman@chromium.org

[cjhopman, 2015-03-14 00:43:57]

wait, not lgtm. We need a way to ensure that build steps that use this will be
retriggered when a referenced file is changed.

[cjhopman, 2015-03-14 00:49:00]

On 2015/03/14 00:43:57, cjhopman wrote:
> wait, not lgtm. We need a way to ensure that build steps that use this will be
> retriggered when a referenced file is changed.

Two approaches I can think of:

1. require users to specify all the files extended. Then, here, create a
temporary directory and link in all the allowed files and use that is the root
for the loader instead of build_utils.CHROMIUM_SRC
2. Write our own simple loader that is just like FileSystemLoader but keeps a
record of all files loaded. Then, add those paths to the depfile.

My slight preference would be for 2, but 1 would be fine too.

[newt (away), 2015-03-14 01:02:33]

On 2015/03/14 00:49:00, cjhopman wrote:
> On 2015/03/14 00:43:57, cjhopman wrote:
> > wait, not lgtm. We need a way to ensure that build steps that use this will
be
> > retriggered when a referenced file is changed.
> 
> Two approaches I can think of:
> 
> 1. require users to specify all the files extended. Then, here, create a
> temporary directory and link in all the allowed files and use that is the root
> for the loader instead of build_utils.CHROMIUM_SRC
> 2. Write our own simple loader that is just like FileSystemLoader but keeps a
> record of all files loaded. Then, add those paths to the depfile.
> 
> My slight preference would be for 2, but 1 would be fine too.

ugh. you're right.

I'll look into this on Monday. 1 sounds easier and I don't expect ever to have
more than one or two includes. Let me poke around...

[newt (away), 2015-03-17 03:17:02]

Ptal

[cjhopman, 2015-03-17 18:03:33]

https://codereview.chromium.org/1002313002/diff/40001/build/android/gyp/jinja...
File build/android/gyp/jinja_template.py (right):

https://codereview.chromium.org/1002313002/diff/40001/build/android/gyp/jinja...
build/android/gyp/jinja_template.py:25: self.loaded_templates = []
Should this be a set? Idk if ninja cares about duplicate entries in depfiles (it
probably doesn't).

https://codereview.chromium.org/1002313002/diff/40001/build/android/gyp/jinja...
build/android/gyp/jinja_template.py:28: contents, filename, uptodate =
jinja2.FileSystemLoader.get_source(
is filename relative or absolute? We should make sure that it is relative to the
current directory.

[newt (away), 2015-03-17 18:44:47]

https://codereview.chromium.org/1002313002/diff/40001/build/android/gyp/jinja...
File build/android/gyp/jinja_template.py (right):

https://codereview.chromium.org/1002313002/diff/40001/build/android/gyp/jinja...
build/android/gyp/jinja_template.py:28: contents, filename, uptodate =
jinja2.FileSystemLoader.get_source(
On 2015/03/17 18:03:33, cjhopman wrote:
> is filename relative or absolute? We should make sure that it is relative to
the
> current directory.

They're absolute paths currently. Curious: why do they need to be relative?

[newt (away), 2015-03-17 18:58:29]

https://codereview.chromium.org/1002313002/diff/40001/build/android/gyp/jinja...
File build/android/gyp/jinja_template.py (right):

https://codereview.chromium.org/1002313002/diff/40001/build/android/gyp/jinja...
build/android/gyp/jinja_template.py:25: self.loaded_templates = []
On 2015/03/17 18:03:33, cjhopman wrote:
> Should this be a set? Idk if ninja cares about duplicate entries in depfiles
(it
> probably doesn't).

Changed to set.

https://codereview.chromium.org/1002313002/diff/40001/build/android/gyp/jinja...
build/android/gyp/jinja_template.py:28: contents, filename, uptodate =
jinja2.FileSystemLoader.get_source(
On 2015/03/17 18:03:33, cjhopman wrote:
> is filename relative or absolute? We should make sure that it is relative to
the
> current directory.

Changed to relative.

[cjhopman, 2015-03-17 19:01:53]

lgtm

https://codereview.chromium.org/1002313002/diff/40001/build/android/gyp/jinja...
File build/android/gyp/jinja_template.py (right):

https://codereview.chromium.org/1002313002/diff/40001/build/android/gyp/jinja...
build/android/gyp/jinja_template.py:28: contents, filename, uptodate =
jinja2.FileSystemLoader.get_source(
On 2015/03/17 18:58:28, newt wrote:
> On 2015/03/17 18:03:33, cjhopman wrote:
> > is filename relative or absolute? We should make sure that it is relative to
> the
> > current directory.
> 
> Changed to relative.


I believe that ninja will treat a relative path and an absolute path to the same
file as different files. Then, if you have an output of one thing as the input
of another, and one specifies it as relative, the other as absolute, you will
have a bad time. And so, we make everything relative.

thakis@ really doesn't like absolute paths, but I think he usually says it's
because he tends to rename output directories and wants things to still work. I
don't know how common that is among other people and I care much more about the
first point.

[newt (away), 2015-03-17 19:11:01]

The CQ bit was checked by newt@chromium.org

[commit-bot: I haz the power, 2015-03-17 19:11:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1002313002/60001

[commit-bot: I haz the power, 2015-03-17 20:37:35]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-03-17 20:38:20]

Patchset 4 (id:??) landed as
https://crrev.com/031bd459554280a3653a1541221e224a6678eda2
Cr-Commit-Position: refs/heads/master@{#320963}