GN: Add ability to specify a depfile for custom targets.

GN: Add ability to specify a depfile for custom targets.

BUG=297671
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=233435

[brettw, 2013-11-04 05:22:14]

LGTM!

https://codereview.chromium.org/55633002/diff/1/tools/gn/ninja_script_target_...
File tools/gn/ninja_script_target_writer.cc (right):

https://codereview.chromium.org/55633002/diff/1/tools/gn/ninja_script_target_...
tools/gn/ninja_script_target_writer.cc:39: out_ << "build ";
I've been standardizing on the guy who's appending something writes the space.
You can keep WriteDepfile not writing a space if you want, but then write the
extra one inside the if block below, or else you'll get two spaces when there's
no depfile.

https://codereview.chromium.org/55633002/diff/1/tools/gn/value_extractors.h
File tools/gn/value_extractors.h (right):

https://codereview.chromium.org/55633002/diff/1/tools/gn/value_extractors.h#n...
tools/gn/value_extractors.h:63: SourceFile* file, Err* err);
Nit: put Err on the next line.

https://codereview.chromium.org/55633002/diff/1/tools/gn/variables.cc
File tools/gn/variables.cc (right):

https://codereview.chromium.org/55633002/diff/1/tools/gn/variables.cc#newcode400
tools/gn/variables.cc:400: "depfile: [string] File name for input dependencies
for custom targets.\n"
I've been putting a blank line after this in the other ones.

[koz (OOO until 15th September), 2013-11-06 07:29:10]

Thanks, Brett!

https://codereview.chromium.org/55633002/diff/1/tools/gn/ninja_script_target_...
File tools/gn/ninja_script_target_writer.cc (right):

https://codereview.chromium.org/55633002/diff/1/tools/gn/ninja_script_target_...
tools/gn/ninja_script_target_writer.cc:39: out_ << "build ";
On 2013/11/04 05:22:14, brettw wrote:
> I've been standardizing on the guy who's appending something writes the space.
> You can keep WriteDepfile not writing a space if you want, but then write the
> extra one inside the if block below, or else you'll get two spaces when
there's
> no depfile.

Done.

https://codereview.chromium.org/55633002/diff/1/tools/gn/value_extractors.h
File tools/gn/value_extractors.h (right):

https://codereview.chromium.org/55633002/diff/1/tools/gn/value_extractors.h#n...
tools/gn/value_extractors.h:63: SourceFile* file, Err* err);
On 2013/11/04 05:22:14, brettw wrote:
> Nit: put Err on the next line.

Done.

https://codereview.chromium.org/55633002/diff/1/tools/gn/variables.cc
File tools/gn/variables.cc (right):

https://codereview.chromium.org/55633002/diff/1/tools/gn/variables.cc#newcode400
tools/gn/variables.cc:400: "depfile: [string] File name for input dependencies
for custom targets.\n"
On 2013/11/04 05:22:14, brettw wrote:
> I've been putting a blank line after this in the other ones.

Done.

[commit-bot: I haz the power, 2013-11-06 07:32:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/koz@chromium.org/55633002/80001

[commit-bot: I haz the power, 2013-11-06 10:59:31]

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

[commit-bot: I haz the power, 2013-11-06 23:10:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/koz@chromium.org/55633002/80001

[commit-bot: I haz the power, 2013-11-07 00:31:38]

Change committed as 233435