Include included files in depfiles. (issue 1396493003 by dpranke@chromium.org)

1 view
Skip to first unread message

dpr...@chromium.org

unread,
Oct 8, 2015, 6:12:16 PM10/8/15
to bre...@chromium.org, grit-de...@googlegroups.com
Reviewers: brettw,

Description:
Include included files in depfiles.

Previously, we would record the dependencies on files that were being
pulled in
via <structure> nodes but not <include> nodes.

R=bre...@chromium.org
BUG=http://crbug.com/541158

Please review this at https://codereview.chromium.org/1396493003/

Base URL: https://chromium.googlesource.com/external/grit-i18n.git@master

Affected files (+29, -2 lines):
M grit/node/misc.py
M grit/node/misc_unittest.py


Index: grit/node/misc.py
diff --git a/grit/node/misc.py b/grit/node/misc.py
index
1003b877548e243a4b8a8f9ddf9f3742c1aced04..bd999709bdc58275e36997797f30b997391f87f1
100755
--- a/grit/node/misc.py
+++ b/grit/node/misc.py
@@ -336,8 +336,10 @@ class GritNode(base.Node):
input_files.add(self.ToRealPath(input_path))

# If it's a flattened node, grab inlined resources too.
- if node.name == 'structure' and node.attrs['flattenhtml']
== 'true':
- node.RunPreSubstitutionGatherer()
+ if ((node.name == 'structure' or node.name == 'include')
+ and node.attrs['flattenhtml'] == 'true'):
+ if node.name == 'structure':
+ node.RunPreSubstitutionGatherer()
input_files.update(node.GetHtmlResourceFilenames())

self.SetOutputLanguage(old_output_language)
Index: grit/node/misc_unittest.py
diff --git a/grit/node/misc_unittest.py b/grit/node/misc_unittest.py
index
dea81b65fd3b5128e3efaa79f84649195d44c723..005cbf6cf4fe63cbaf6236ac01437978c3a8663d
100644
--- a/grit/node/misc_unittest.py
+++ b/grit/node/misc_unittest.py
@@ -81,6 +81,31 @@ class GritNodeUnittest(unittest.TestCase):
actual = [path.replace('\\', '/') for path in actual]
self.assertEquals(expected, actual)

+ # Verifies that GetInputFiles() returns the correct list of files
+ # when files include other files.
+ def testGetInputFilesFromIncludes(self):
+ chrome_html_path = util.PathFromRoot('grit/testdata/chrome_html.html')
+ xml = '''<?xml version="1.0" encoding="utf-8"?>
+ <grit latest_public_release="0" current_release="1">
+ <outputs>
+ <output filename="default.pak" type="data_package"
context="default_100_percent" />
+ <output filename="special.pak" type="data_package"
context="special_100_percent" fallback_to_default_layout="false" />
+ </outputs>
+ <release seq="1">
+ <includes>
+ <include name="IDR_TESTDATA_CHROME_HTML" file="%s"
flattenhtml="true"
+ allowexternalscript="true" type="BINDATA" />
+ </includes>
+ </release>
+ </grit>''' % chrome_html_path
+
+ grd = grd_reader.Parse(StringIO.StringIO(xml),
util.PathFromRoot('grit/testdata'))
+ expected = ['chrome_html.html', 'included_sample.html']
+ actual = [os.path.relpath(path, util.PathFromRoot('grit/testdata'))
for path in grd.GetInputFiles()]
+ # Convert path separator for Windows paths.
+ actual = [path.replace('\\', '/') for path in actual]
+ self.assertEquals(expected, actual)
+

class IfNodeUnittest(unittest.TestCase):
def testIffyness(self):


bre...@chromium.org

unread,
Oct 8, 2015, 6:18:36 PM10/8/15
to dpr...@chromium.org, grit-de...@googlegroups.com

dpr...@chromium.org

unread,
Oct 8, 2015, 7:44:20 PM10/8/15
to bre...@chromium.org, grit-de...@googlegroups.com
Committed manually by brettw@ in the grit svn repo @ r200.

https://codereview.chromium.org/1396493003/
Reply all
Reply to author
Forward
0 new messages