Cleanup public includes directory.

Cleanup public includes directory.

This CL updates various files in the includes directory to ensure that (1) they do
not depend on headers in /src and (2) that they minimize their dependence on external
headers.

To ensure that we don't regress this behavior a new build target has been added to
build a single cpp file that contains all* public includes and is compiled with
only those directories in the include path.

* The exception is those includes that depend on OS specific headers

BUG=skia:2941
NOTRY=true

Committed: https://skia.googlesource.com/skia/+/0b17d6cb343cb71a3f548a02f0952c3592fc8c87

[djsollen, 2014-11-12 21:09:51]

djsollen@google.com changed reviewers:
+ reed@google.com

[djsollen, 2014-11-13 14:03:18]

djsollen@google.com changed reviewers:
+ borenet@google.com, bungeman@google.com, reed@google.com*
- reed@google.com

[djsollen, 2014-11-13 14:03:19]

Adding Ben and Eric to look at the gyp and python changes respectively

[reed1, 2014-11-13 14:06:58]

reed@google.com changed reviewers:
+ reed@google.com

[reed1, 2014-11-13 14:06:59]

non-python lgtm

[tfarina, 2014-11-13 14:56:25]

Could you include BUG=skia:2941 in the CL description?

[borenet, 2014-11-13 15:13:35]

https://codereview.chromium.org/721903002/diff/80001/gyp/tools.gyp
File gyp/tools.gyp (right):

https://codereview.chromium.org/721903002/diff/80001/gyp/tools.gyp#newcode681
gyp/tools.gyp:681: #'<@(paths_to_ignore)',
I don't think this needs to be listed here anyway.  It's possible that the Mac
generator was failing because some of the above paths are not files.

https://codereview.chromium.org/721903002/diff/80001/tools/generate_includes_...
File tools/generate_includes_cpp.py (right):

https://codereview.chromium.org/721903002/diff/80001/tools/generate_includes_...
tools/generate_includes_cpp.py:8: '''
Prefer this format, but not sure it really matters:
#!/usr/bin/env python
# Copyright (c) 2014 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

https://codereview.chromium.org/721903002/diff/80001/tools/generate_includes_...
tools/generate_includes_cpp.py:12: 
Nit: 2 lines between top-level stuff

https://codereview.chromium.org/721903002/diff/80001/tools/generate_includes_...
tools/generate_includes_cpp.py:15: if full_path.endswith(ignore_path):
Aren't some ignore_paths directories?  Should this be "if
full_path.startswith(ignore_path)"?  Or even "if ignore_path in full_path"?

https://codereview.chromium.org/721903002/diff/80001/tools/generate_includes_...
tools/generate_includes_cpp.py:24: for files ending in '.h'
Please document ignore_list as well.

https://codereview.chromium.org/721903002/diff/80001/tools/generate_includes_...
tools/generate_includes_cpp.py:35: #print "%s\n" % full_path
Should this stay or go?

https://codereview.chromium.org/721903002/diff/80001/tools/generate_includes_...
tools/generate_includes_cpp.py:40: headers.append(os.path.join(filename,
nested_header))
Optional: this would get faster if you made this function a generator.  Instead
of "headers.append(X)" you'd write "yield X".  

Then these three lines become:

for nested_header in find_header_files([full_path], ignore_list):
  yield os.path.join(filename, nested_header)

And instead of storing a pointer to the list in line 46, you'd just iterate
directly:

for header in find_header_files(include_dirs, ignore_list):

https://codereview.chromium.org/721903002/diff/80001/tools/generate_includes_...
tools/generate_includes_cpp.py:52: output.close()
Please use the "with" syntax:

with open(os.path.join(os.getcwd(), output_file), "w+") as output:
  for header in headers:
    output.write("#include <%s>\n" % header)

It's equivalent to using try/finally, so it's safer than explicitly closing
after the writes.

[djsollen, 2014-11-13 15:45:27]

https://codereview.chromium.org/721903002/diff/80001/tools/generate_includes_...
File tools/generate_includes_cpp.py (right):

https://codereview.chromium.org/721903002/diff/80001/tools/generate_includes_...
tools/generate_includes_cpp.py:12: 
On 2014/11/13 15:13:34, borenet wrote:
> Nit: 2 lines between top-level stuff

Done.

https://codereview.chromium.org/721903002/diff/80001/tools/generate_includes_...
tools/generate_includes_cpp.py:15: if full_path.endswith(ignore_path):
On 2014/11/13 15:13:34, borenet wrote:
> Aren't some ignore_paths directories?  Should this be "if
> full_path.startswith(ignore_path)"?  Or even "if ignore_path in full_path"?

Nope, we always want exact matches here. the find_header_files function will
short circuit the recursion when it matches to a directory so we'll never see
the files in those directories.

https://codereview.chromium.org/721903002/diff/80001/tools/generate_includes_...
tools/generate_includes_cpp.py:24: for files ending in '.h'
On 2014/11/13 15:13:34, borenet wrote:
> Please document ignore_list as well.

Done.

https://codereview.chromium.org/721903002/diff/80001/tools/generate_includes_...
tools/generate_includes_cpp.py:35: #print "%s\n" % full_path
On 2014/11/13 15:13:34, borenet wrote:
> Should this stay or go?

Done.

https://codereview.chromium.org/721903002/diff/80001/tools/generate_includes_...
tools/generate_includes_cpp.py:52: output.close()
On 2014/11/13 15:13:34, borenet wrote:
> Please use the "with" syntax:
> 
> with open(os.path.join(os.getcwd(), output_file), "w+") as output:
>   for header in headers:
>     output.write("#include <%s>\n" % header)
> 
> It's equivalent to using try/finally, so it's safer than explicitly closing
> after the writes.

Done.

[borenet, 2014-11-13 15:49:11]

Python and GYP lgtm

[bungeman-skia, 2014-11-13 18:13:38]

lgtm with nit

https://codereview.chromium.org/721903002/diff/140001/tools/generate_includes...
File tools/generate_includes_cpp.py (right):

https://codereview.chromium.org/721903002/diff/140001/tools/generate_includes...
tools/generate_includes_cpp.py:13: import posixpath
p should come before r (except in neighbor and way?)

[djsollen, 2014-11-13 20:31:51]

The CQ bit was checked by djsollen@google.com

[commit-bot: I haz the power, 2014-11-13 20:32:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/721903002/200001

[commit-bot: I haz the power, 2014-11-13 20:49:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-13 20:49:11]

Try jobs failed on following builders:
  Test-Ubuntu13.10-GCE-NoGPU-x86_64-Debug-Trybot on client.skia
(http://build.chromium.org/p/client.skia/builders/Test-Ubuntu13.10-GCE-NoGPU-x...)

[djsollen, 2014-11-13 20:51:42]

The CQ bit was checked by djsollen@google.com

[commit-bot: I haz the power, 2014-11-13 20:52:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/721903002/200001

[commit-bot: I haz the power, 2014-11-13 20:52:39]

Committed patchset #11 (id:200001) as
https://skia.googlesource.com/skia/+/0b17d6cb343cb71a3f548a02f0952c3592fc8c87