Add output reference to gen written files.

Add output reference to gen written files.

By adding the outputs written at gn gen time to the build.ninja representation the build tool (ninja) can know how to recreate them if and when necessary.

For example, if one or more of the <blah>.tmp files is removed, even as part of an entire hierarchy/subtree currently the build fails because ninja does not know how to re-make it.

BUG=469621
TEST=Build time only. See
https://code.google.com/p/chromium/issues/detail?id=469621#c3

Committed: https://crrev.com/c497d21798caf871326443ad98bd58f546d7b38e
Cr-Commit-Position: refs/heads/master@{#341251}

[Peter Mayo, 2015-07-28 22:18:36]

Created Reland of Add output reference to gen written files.

[Peter Mayo, 2015-07-28 22:20:39]

petermayo@chromium.org changed reviewers:
- brettw@chromium.org, dpranke@chromium.org

[Peter Mayo, 2015-07-28 22:26:31]

Committed at https://crrev.com/517ac52f86903beb206b19e6ddeedbc709b253a1
Reverted  at https://crrev.com/61fb6826c8e7b9a31c260cb3698258e4fad5b33b


From
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...

ninja: error: build.ninja:6: expected build command name
build build.ninja E:/b/build/slave/win/build/src/out/Release/obj/content...
                   ^ near here

Note the unescaped ":" from the Windows drive specification.  Similar problems
could arise from other filename weirdness (like spaces)

[Dirk Pranke, 2015-07-28 22:35:04]

dpranke@chromium.org changed reviewers:
+ dpranke@chromium.org

[Dirk Pranke, 2015-07-28 22:35:05]

lgtm.

https://codereview.chromium.org/1252403005/diff/130001/tools/gn/ninja_build_w...
File tools/gn/ninja_build_writer.cc (right):

https://codereview.chromium.org/1252403005/diff/130001/tools/gn/ninja_build_w...
tools/gn/ninja_build_writer.cc:84: out << " ";
I probably would've left line 84 out of this function and listed it separately
inside the for loop before line 164, but that's just me.

[Peter Mayo, 2015-07-28 22:37:38]

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

https://codereview.chromium.org/1252403005/diff/1/tools/gn/ninja_build_writer...
tools/gn/ninja_build_writer.cc:156: out_ << " " <<
FilePathToUTF8(build_settings_->GetFullPath(written_file));
GetFullPath caused the drive specifier issue, but there could be others lurking
here.

https://codereview.chromium.org/1252403005/diff/1/tools/gn/ninja_build_writer...
tools/gn/ninja_build_writer.cc:170: dep_out_ << " " <<
FilePathToUTF8(input_file);
This could also cause quoting issues.

https://codereview.chromium.org/1252403005/diff/1/tools/gn/ninja_build_writer...
tools/gn/ninja_build_writer.cc:175: dep_out_ << " " <<
FilePathToUTF8(other_file);
There are also potential quoting issues lurking here.

https://codereview.chromium.org/1252403005/diff/130001/tools/gn/ninja_build_w...
File tools/gn/ninja_build_writer.cc (right):

https://codereview.chromium.org/1252403005/diff/130001/tools/gn/ninja_build_w...
tools/gn/ninja_build_writer.cc:80: void WriteAnotherFile(std::ostream& out,
const base::FilePath& name) {
I'm not sure this name is good enough, but I don't have a better one.

https://codereview.chromium.org/1252403005/diff/130001/tools/gn/ninja_build_w...
tools/gn/ninja_build_writer.cc:82: path_escaping.mode = ESCAPE_NINJA_COMMAND;
This mode says it handles one "thing" for ninja, though the name makes it seem
suspect.

[Peter Mayo, 2015-07-28 22:41:38]

Dirk,

How do I get this actually exercised on a windows machine?

https://codereview.chromium.org/1252403005/diff/130001/tools/gn/ninja_build_w...
File tools/gn/ninja_build_writer.cc (right):

https://codereview.chromium.org/1252403005/diff/130001/tools/gn/ninja_build_w...
tools/gn/ninja_build_writer.cc:84: out << " ";
On 2015/07/28 22:35:05, Dirk Pranke wrote:
> I probably would've left line 84 out of this function and listed it separately
> inside the for loop before line 164, but that's just me.

And 178, and 183.  That's why I snuck it in here, and put the word "another" in
the name.

I would have moved the loop in here too to save reconstructing the EscapeOptions
each time too, but they have subtly different types.

[Dirk Pranke, 2015-07-28 22:43:11]

https://codereview.chromium.org/1252403005/diff/130001/tools/gn/ninja_build_w...
File tools/gn/ninja_build_writer.cc (right):

https://codereview.chromium.org/1252403005/diff/130001/tools/gn/ninja_build_w...
tools/gn/ninja_build_writer.cc:84: out << " ";
On 2015/07/28 22:41:38, Peter Mayo wrote:
> On 2015/07/28 22:35:05, Dirk Pranke wrote:
> > I probably would've left line 84 out of this function and listed it
separately
> > inside the for loop before line 164, but that's just me.
> 
> And 178, and 183.  That's why I snuck it in here, and put the word "another"
in
> the name.

fair enough. Yeah, I understood the naming.

> I would have moved the loop in here too to save reconstructing the
EscapeOptions
> each time too, but they have subtly different types.

Yup, understood.

[brettw, 2015-07-29 20:16:44]

brettw@chromium.org changed reviewers:
+ brettw@chromium.org

[brettw, 2015-07-29 20:16:45]

lgtm

[Peter Mayo, 2015-07-30 23:25:33]

https://codereview.chromium.org/1252403005/diff/150001/tools/gn/ninja_build_w...
File tools/gn/ninja_build_writer.cc (right):

https://codereview.chromium.org/1252403005/diff/150001/tools/gn/ninja_build_w...
tools/gn/ninja_build_writer.cc:157: out_ << " " <<
Is this the right wrapping?  dropping the << onto the next line causes follow-on
un-nesting.

[Dirk Pranke, 2015-07-30 23:37:08]

https://codereview.chromium.org/1252403005/diff/150001/tools/gn/ninja_build_w...
File tools/gn/ninja_build_writer.cc (right):

https://codereview.chromium.org/1252403005/diff/150001/tools/gn/ninja_build_w...
tools/gn/ninja_build_writer.cc:157: out_ << " " <<
On 2015/07/30 23:25:32, Peter Mayo wrote:
> Is this the right wrapping?  dropping the << onto the next line causes
follow-on
> un-nesting.

no clue :).

[Dirk Pranke, 2015-07-30 23:38:34]

lgtm

[Peter Mayo, 2015-07-31 00:47:43]

The CQ bit was checked by petermayo@chromium.org

[Peter Mayo, 2015-07-31 00:47:43]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/1252403005/#ps150001
(title: "do not escape paths in build.ninja.d")

[commit-bot: I haz the power, 2015-07-31 00:48:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1252403005/150001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1252403005/150001

[brettw, 2015-07-31 00:57:01]

https://codereview.chromium.org/1252403005/diff/150001/tools/gn/ninja_build_w...
File tools/gn/ninja_build_writer.cc (right):

https://codereview.chromium.org/1252403005/diff/150001/tools/gn/ninja_build_w...
tools/gn/ninja_build_writer.cc:157: out_ << " " <<
Seems "not wrong"

[commit-bot: I haz the power, 2015-07-31 01:05:04]

Committed patchset #3 (id:150001)

[commit-bot: I haz the power, 2015-07-31 01:05:54]

Patchset 3 (id:??) landed as
https://crrev.com/c497d21798caf871326443ad98bd58f546d7b38e
Cr-Commit-Position: refs/heads/master@{#341251}

[brettw, 2015-09-23 16:35:49]

This seems to have caused http://crbug.com/533067
If we can't find a way around the issue I think we'll have to revert this.

[Peter Mayo, 2015-09-23 18:53:34]

https://codereview.chromium.org/1252403005/diff/150001/tools/gn/ninja_build_w...
File tools/gn/ninja_build_writer.cc (right):

https://codereview.chromium.org/1252403005/diff/150001/tools/gn/ninja_build_w...
tools/gn/ninja_build_writer.cc:146: out_ << "  restat = 1\n\n";
This is important for this change.