Address more TODO(GYP) in the GN build.

Address more TODO(GYP) in the GN build.

Addresses another batch of TODO(GYP) annotations in GN files.

Removes all references to msvs_shard which is not necesasary in the GN build.

A number of these were related to things to support the nacl64 build. In GN we just do cross-compiles of the fill targets. This seems to be working OK. Probably the sizes can be brought down a bit by optimizing this and producing nacl-specific targets with only the bare necessities, but it seems the marginal benefit doesn't outweigh the build complexity.

TBR=thestig@chromium.org for linux/build (comment change)

Committed: https://crrev.com/57399c53d558cc84c865ad48a211fe9e92c2ccf9
Cr-Commit-Position: refs/heads/master@{#392660}

[brettw, 2016-05-06 17:29:13]

Description was changed from

==========
Address more TODO(GYP) in the GN build.

Addresses another batch of TODO(GYP) annotations in GN files.

Removes all references to msvs_shard which is not necesasary in the GN build.
==========

to

==========
Address more TODO(GYP) in the GN build.

Addresses another batch of TODO(GYP) annotations in GN files.

Removes all references to msvs_shard which is not necesasary in the GN build.

A number of these were related to things to support the nacl64 build. In GN we
just do cross-compiles of the fill targets. This seems to be working OK.
Probably the sizes can be brought down a bit by optimizing this, but it seems
the marginal benefit doesn't outweigh the build complexity.
==========

[brettw, 2016-05-06 17:30:26]

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

[brettw, 2016-05-06 17:30:31]

The CQ bit was checked by brettw@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-06 17:31:02]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1957683002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1957683002/1

[commit-bot: I haz the power, 2016-05-06 17:34:55]

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

[commit-bot: I haz the power, 2016-05-06 17:34:56]

Dry run: Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)
  android_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  ios-simulator-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)

[brucedawson, 2016-05-06 18:38:36]

I took a brief look but I can look in more detail once it's passing the try
jobs.

"Probably the sizes can be brought down a bit by optimizing this" - what sizes?

https://codereview.chromium.org/1957683002/diff/1/components/crash/content/ap...
File components/crash/content/app/BUILD.gn (right):

https://codereview.chromium.org/1957683002/diff/1/components/crash/content/ap...
components/crash/content/app/BUILD.gn:208: "crash_keys_win_unittest.cc",
Note that this is fixing part of bug 604060, so add that reference to the BUG=
part of the description.

[brucedawson, 2016-05-09 19:09:18]

https://codereview.chromium.org/1957683002/diff/1/components/crash/content/ap...
File components/crash/content/app/BUILD.gn (right):

https://codereview.chromium.org/1957683002/diff/1/components/crash/content/ap...
components/crash/content/app/BUILD.gn:208: "crash_keys_win_unittest.cc",
On 2016/05/06 18:38:36, brucedawson wrote:
> Note that this is fixing part of bug 604060, so add that reference to the BUG=
> part of the description.

Also, the file name suggests that this is a Windows only file, but in gyp builds
it was compiled for all. ???

[brettw, 2016-05-09 21:07:54]

fix

[brettw, 2016-05-09 21:08:28]

Description was changed from

==========
Address more TODO(GYP) in the GN build.

Addresses another batch of TODO(GYP) annotations in GN files.

Removes all references to msvs_shard which is not necesasary in the GN build.

A number of these were related to things to support the nacl64 build. In GN we
just do cross-compiles of the fill targets. This seems to be working OK.
Probably the sizes can be brought down a bit by optimizing this, but it seems
the marginal benefit doesn't outweigh the build complexity.
==========

to

==========
Address more TODO(GYP) in the GN build.

Addresses another batch of TODO(GYP) annotations in GN files.

Removes all references to msvs_shard which is not necesasary in the GN build.

A number of these were related to things to support the nacl64 build. In GN we
just do cross-compiles of the fill targets. This seems to be working OK.
Probably the sizes can be brought down a bit by optimizing this and producing
nacl-specific targets with only the bare necessities, but it seems the marginal
benefit doesn't outweigh the build complexity.
==========

[brettw, 2016-05-09 21:08:34]

PTAL

https://codereview.chromium.org/1957683002/diff/1/components/crash/content/ap...
File components/crash/content/app/BUILD.gn (right):

https://codereview.chromium.org/1957683002/diff/1/components/crash/content/ap...
components/crash/content/app/BUILD.gn:208: "crash_keys_win_unittest.cc",
The platform-filter should remove "*_win_unittest.cc" files from both GYP and GN
builds.

[brettw, 2016-05-09 21:16:36]

The CQ bit was checked by brettw@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-09 21:17:11]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1957683002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1957683002/20001

[commit-bot: I haz the power, 2016-05-09 21:23:38]

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

[commit-bot: I haz the power, 2016-05-09 21:23:38]

Dry run: Try jobs failed on following builders:
  ios-device-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)

[brucedawson, 2016-05-09 21:30:33]

A couple of comments. Basically looks good but isn't passing the bots yet :-(

https://codereview.chromium.org/1957683002/diff/20001/chrome/installer/util/B...
File chrome/installer/util/BUILD.gn (right):

https://codereview.chromium.org/1957683002/diff/20001/chrome/installer/util/B...
chrome/installer/util/BUILD.gn:340: "//build/win:default_exe_manifest",
When I build this with gn I see that it already has a manifest, which means that
this line is not necessary. I think that is because test() implies the default
manifest.

https://codereview.chromium.org/1957683002/diff/20001/chrome/test/BUILD.gn
File chrome/test/BUILD.gn (right):

https://codereview.chromium.org/1957683002/diff/20001/chrome/test/BUILD.gn#ne...
chrome/test/BUILD.gn:712: # on Mac, which does use hunspell by default.
Is this supposed to be "does not use hunspell by default"?

I know you're just copying the comment but it seems like it should be "does not
use", or perhaps "does use" to make intent clearer. Or just ignore this comment.

[brettw, 2016-05-09 22:26:44]

The CQ bit was checked by brettw@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-09 22:27:17]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1957683002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1957683002/40001

[brettw, 2016-05-09 22:27:19]

Done, PTAL (I put back one of the Mac dependencies that I think was causing
problems, and added a long comment about what I figured out about it).

[commit-bot: I haz the power, 2016-05-09 22:34:46]

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

[commit-bot: I haz the power, 2016-05-09 22:34:47]

Dry run: Try jobs failed on following builders:
  ios-device-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)

[brettw, 2016-05-09 23:55:00]

The CQ bit was checked by brettw@chromium.org to run a CQ dry run

[brettw, 2016-05-09 23:55:02]

iOS

[commit-bot: I haz the power, 2016-05-09 23:55:25]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1957683002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1957683002/60001

[commit-bot: I haz the power, 2016-05-10 00:56:50]

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

[commit-bot: I haz the power, 2016-05-10 00:56:51]

Dry run: Try jobs failed on following builders:
  ios-simulator-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)

[brettw, 2016-05-10 16:43:05]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2016-05-10 16:43:25]

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

[commit-bot: I haz the power, 2016-05-10 16:43:27]

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

[commit-bot: I haz the power, 2016-05-10 16:43:28]

No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[brettw, 2016-05-10 16:56:21]

The CQ bit was checked by brettw@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-10 16:57:07]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1957683002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1957683002/60001

[commit-bot: I haz the power, 2016-05-10 17:31:33]

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

[commit-bot: I haz the power, 2016-05-10 17:31:34]

Dry run: This issue passed the CQ dry run.

[brucedawson, 2016-05-10 17:39:06]

On 2016/05/10 17:31:34, commit-bot: I haz the power wrote:
> Dry run: This issue passed the CQ dry run.

Looking now...

[brucedawson, 2016-05-10 18:04:25]

lgtm

[brettw, 2016-05-10 18:08:17]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2016-05-10 18:08:56]

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

[commit-bot: I haz the power, 2016-05-10 18:18:31]

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

[commit-bot: I haz the power, 2016-05-10 18:18:33]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[brettw, 2016-05-10 18:26:29]

Description was changed from

==========
Address more TODO(GYP) in the GN build.

Addresses another batch of TODO(GYP) annotations in GN files.

Removes all references to msvs_shard which is not necesasary in the GN build.

A number of these were related to things to support the nacl64 build. In GN we
just do cross-compiles of the fill targets. This seems to be working OK.
Probably the sizes can be brought down a bit by optimizing this and producing
nacl-specific targets with only the bare necessities, but it seems the marginal
benefit doesn't outweigh the build complexity.
==========

to

==========
Address more TODO(GYP) in the GN build.

Addresses another batch of TODO(GYP) annotations in GN files.

Removes all references to msvs_shard which is not necesasary in the GN build.

A number of these were related to things to support the nacl64 build. In GN we
just do cross-compiles of the fill targets. This seems to be working OK.
Probably the sizes can be brought down a bit by optimizing this and producing
nacl-specific targets with only the bare necessities, but it seems the marginal
benefit doesn't outweigh the build complexity.

TBR=thestig@chromium.org for linux/build (comment change)
==========

[brettw, 2016-05-10 18:27:07]

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

[brettw, 2016-05-10 18:27:07]

TBR thestig for chrome/installer/linux/BUILD.gn (trivial comment update)

[brettw, 2016-05-10 18:27:13]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2016-05-10 18:28:04]

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

[commit-bot: I haz the power, 2016-05-10 18:34:16]

Description was changed from

==========
Address more TODO(GYP) in the GN build.

Addresses another batch of TODO(GYP) annotations in GN files.

Removes all references to msvs_shard which is not necesasary in the GN build.

A number of these were related to things to support the nacl64 build. In GN we
just do cross-compiles of the fill targets. This seems to be working OK.
Probably the sizes can be brought down a bit by optimizing this and producing
nacl-specific targets with only the bare necessities, but it seems the marginal
benefit doesn't outweigh the build complexity.

TBR=thestig@chromium.org for linux/build (comment change)
==========

to

==========
Address more TODO(GYP) in the GN build.

Addresses another batch of TODO(GYP) annotations in GN files.

Removes all references to msvs_shard which is not necesasary in the GN build.

A number of these were related to things to support the nacl64 build. In GN we
just do cross-compiles of the fill targets. This seems to be working OK.
Probably the sizes can be brought down a bit by optimizing this and producing
nacl-specific targets with only the bare necessities, but it seems the marginal
benefit doesn't outweigh the build complexity.

TBR=thestig@chromium.org for linux/build (comment change)
==========

[commit-bot: I haz the power, 2016-05-10 18:34:17]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-10 18:35:45]

Description was changed from

==========
Address more TODO(GYP) in the GN build.

Addresses another batch of TODO(GYP) annotations in GN files.

Removes all references to msvs_shard which is not necesasary in the GN build.

A number of these were related to things to support the nacl64 build. In GN we
just do cross-compiles of the fill targets. This seems to be working OK.
Probably the sizes can be brought down a bit by optimizing this and producing
nacl-specific targets with only the bare necessities, but it seems the marginal
benefit doesn't outweigh the build complexity.

TBR=thestig@chromium.org for linux/build (comment change)
==========

to

==========
Address more TODO(GYP) in the GN build.

Addresses another batch of TODO(GYP) annotations in GN files.

Removes all references to msvs_shard which is not necesasary in the GN build.

A number of these were related to things to support the nacl64 build. In GN we
just do cross-compiles of the fill targets. This seems to be working OK.
Probably the sizes can be brought down a bit by optimizing this and producing
nacl-specific targets with only the bare necessities, but it seems the marginal
benefit doesn't outweigh the build complexity.

TBR=thestig@chromium.org for linux/build (comment change)

Committed: https://crrev.com/57399c53d558cc84c865ad48a211fe9e92c2ccf9
Cr-Commit-Position: refs/heads/master@{#392660}
==========

[commit-bot: I haz the power, 2016-05-10 18:35:47]

Patchset 4 (id:??) landed as
https://crrev.com/57399c53d558cc84c865ad48a211fe9e92c2ccf9
Cr-Commit-Position: refs/heads/master@{#392660}

[Lei Zhang, 2016-05-10 18:42:52]

On 2016/05/10 18:27:07, brettw wrote:
> TBR thestig for chrome/installer/linux/BUILD.gn (trivial comment update)

lgtm