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

Issue 226223006: Template invocation fixes in GN (Closed)

Created:
6 years, 8 months ago by brettw
Modified:
6 years, 8 months ago
Reviewers:
cjhopman
CC:
chromium-reviews, tfarina
Visibility:
Public.

Description

Template invocation fixes in GN This adds an error check when invoking templates which caused really confusing messages if the template invocation encountered an error (because we'd continue running). Hooks up the provider for programatically defined variables in template invocations so those can be used. Sets the current directory in a template invocation to be that of the invoking file. No longer define the target-related programatic variables in an import. Using these in an import will give the directory relative to the import, which is probabyl not what you want. Fix the Windows build by adding a missing library. Add a warning not to add more to the main list (this added .lib is pretty obscure). BUG= R=cjhopman@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=262524

Patch Set 1 #

Patch Set 2 : #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+51 lines, -21 lines) Patch
M base/BUILD.gn View 1 1 chunk +3 lines, -0 lines 1 comment Download
M build/config/BUILD.gn View 1 1 chunk +4 lines, -0 lines 0 comments Download
M tools/gn/import_manager.cc View 1 1 chunk +6 lines, -1 line 0 comments Download
M tools/gn/loader.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/scope_per_file_provider.h View 1 2 chunks +6 lines, -1 line 0 comments Download
M tools/gn/scope_per_file_provider.cc View 1 2 chunks +11 lines, -6 lines 0 comments Download
M tools/gn/scope_per_file_provider_unittest.cc View 1 2 chunks +2 lines, -2 lines 0 comments Download
M tools/gn/template.cc View 1 3 chunks +18 lines, -10 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
brettw
6 years, 8 months ago (2014-04-08 20:03:07 UTC) #1
cjhopman
lgtm https://codereview.chromium.org/226223006/diff/20001/base/BUILD.gn File base/BUILD.gn (right): https://codereview.chromium.org/226223006/diff/20001/base/BUILD.gn#newcode760 base/BUILD.gn:760: libs = [ "netapi32.lib" ] Having the windows ...
6 years, 8 months ago (2014-04-08 21:20:40 UTC) #2
brettw
On 2014/04/08 21:20:40, cjhopman wrote: > lgtm > > https://codereview.chromium.org/226223006/diff/20001/base/BUILD.gn > File base/BUILD.gn (right): > ...
6 years, 8 months ago (2014-04-08 22:33:53 UTC) #3
brettw
6 years, 8 months ago (2014-04-08 22:35:22 UTC) #4
Message was sent while issue was closed.
Committed patchset #2 manually as r262524 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698