Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(832)

Issue 223783005: Add support for reading .gypi files. (Closed)

Created:
6 years, 8 months ago by brettw
Modified:
6 years, 8 months ago
CC:
chromium-reviews, rginda+watch_chromium.org, yoshiki+watch_chromium.org
Visibility:
Public.

Description

Add support for reading .gypi files. Adds support for a new value type, a dictionary. To script it looks the same as a scope, but the existing scope didn't have the right memory management semantics for this. Adds a new input conversion mode to interpret the result of a script as a GN block (basically an eval) and get the result in a dictionary. This allows returning named sets of things from a script. Updates the accessor and defined stuff to work with this new type. Adds some features to the input file manager so that when we do dynamic reading from a script we keep the text of the read around which means we can give proper errors that refer to the script output, and don't have to do the weird recursive set origin thing to clear out the pointers that became invalid when the InputFile went out of scope. This allows us to give better error messages when parsing input. The error handling in the input conversion code is changed accordingly. Checks for unused variables both in code that instantiates a template, and in the template code itself. Adds a shared python script for outputting values from Python to GN. Adds a python script to read .gypi files and return the value as a GN scope. BUG= R=dpranke@chromium.org, scottmg@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=262045

Patch Set 1 #

Patch Set 2 : #

Total comments: 19

Patch Set 3 : #

Patch Set 4 : #

Total comments: 2

Patch Set 5 : Now with much improved python #

Total comments: 2

Patch Set 6 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+445 lines, -128 lines) Patch
A build/gn_helpers.py View 1 2 3 4 5 1 chunk +41 lines, -0 lines 0 comments Download
A build/gypi_to_gn.py View 1 2 3 4 1 chunk +87 lines, -0 lines 0 comments Download
M tools/gn/function_exec_script.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/function_read_file.cc View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M tools/gn/functions_unittest.cc View 1 2 2 chunks +3 lines, -3 lines 0 comments Download
M tools/gn/input_conversion.h View 1 2 2 chunks +3 lines, -1 line 0 comments Download
M tools/gn/input_conversion.cc View 1 2 3 8 chunks +79 lines, -72 lines 0 comments Download
M tools/gn/input_conversion_unittest.cc View 1 2 4 chunks +85 lines, -23 lines 0 comments Download
M tools/gn/input_file_manager.h View 2 chunks +30 lines, -0 lines 0 comments Download
M tools/gn/input_file_manager.cc View 2 chunks +14 lines, -0 lines 0 comments Download
M tools/gn/parse_tree_unittest.cc View 1 2 2 chunks +4 lines, -3 lines 0 comments Download
M tools/gn/template.cc View 1 2 2 chunks +39 lines, -7 lines 0 comments Download
M tools/gn/value.h View 1 2 3 5 chunks +22 lines, -6 lines 0 comments Download
M tools/gn/value.cc View 1 2 8 chunks +35 lines, -11 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
brettw
Dirk: Python code Scott: C++ code
6 years, 8 months ago (2014-04-03 18:32:36 UTC) #1
scottmg
https://codereview.chromium.org/223783005/diff/20001/build/gypi_to_gn.py File build/gypi_to_gn.py (right): https://codereview.chromium.org/223783005/diff/20001/build/gypi_to_gn.py#newcode51 build/gypi_to_gn.py:51: ExceptionAppend(e, 'while reading ' + path) this isn't imported ...
6 years, 8 months ago (2014-04-03 20:44:40 UTC) #2
Dirk Pranke
Here's a bunch of comments on the python side. https://codereview.chromium.org/223783005/diff/20001/build/gn_helpers.py File build/gn_helpers.py (right): https://codereview.chromium.org/223783005/diff/20001/build/gn_helpers.py#newcode10 build/gn_helpers.py:10: ...
6 years, 8 months ago (2014-04-03 22:01:28 UTC) #3
brettw
https://codereview.chromium.org/223783005/diff/20001/tools/gn/value.h File tools/gn/value.h (right): https://codereview.chromium.org/223783005/diff/20001/tools/gn/value.h#newcode31 tools/gn/value.h:31: // scopes from files and we actually want to ...
6 years, 8 months ago (2014-04-03 23:19:24 UTC) #4
scottmg
On 2014/04/03 23:19:24, brettw wrote: > https://codereview.chromium.org/223783005/diff/20001/tools/gn/value.h > File tools/gn/value.h (right): > > https://codereview.chromium.org/223783005/diff/20001/tools/gn/value.h#newcode31 > ...
6 years, 8 months ago (2014-04-03 23:22:39 UTC) #5
brettw
On 2014/04/03 23:22:39, scottmg wrote: > Yeah, I was thinking #4, but I don't fully ...
6 years, 8 months ago (2014-04-04 04:33:47 UTC) #6
brettw
Dirk, this doesn't change any Python, will do a new patch soon. Scott: PTAL C++. ...
6 years, 8 months ago (2014-04-04 20:04:40 UTC) #7
scottmg
C++ lgtm with rewritten CL description. https://codereview.chromium.org/223783005/diff/60001/tools/gn/template.cc File tools/gn/template.cc (right): https://codereview.chromium.org/223783005/diff/60001/tools/gn/template.cc#newcode86 tools/gn/template.cc:86: // This is ...
6 years, 8 months ago (2014-04-04 20:15:46 UTC) #8
brettw
Dirk: new Python up, PTAL https://codereview.chromium.org/223783005/diff/20001/build/gypi_to_gn.py File build/gypi_to_gn.py (right): https://codereview.chromium.org/223783005/diff/20001/build/gypi_to_gn.py#newcode48 build/gypi_to_gn.py:48: e.filename = path Dunnow, ...
6 years, 8 months ago (2014-04-04 20:32:27 UTC) #9
brettw
https://codereview.chromium.org/223783005/diff/60001/tools/gn/template.cc File tools/gn/template.cc (right): https://codereview.chromium.org/223783005/diff/60001/tools/gn/template.cc#newcode86 tools/gn/template.cc:86: // This is a bit tricky because it's theoretically ...
6 years, 8 months ago (2014-04-04 20:35:12 UTC) #10
Dirk Pranke
python code is much cleaner :). I noted a couple of nits to clean up ...
6 years, 8 months ago (2014-04-04 23:01:11 UTC) #11
brettw
6 years, 8 months ago (2014-04-06 04:35:19 UTC) #12
Message was sent while issue was closed.
Committed patchset #6 manually as r262045 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698