[GN] Add support for generating Xcode projects.

[GN] Add support for generating Xcode projects.

tools/gn/xcode_object.{cc,h}

  Implements a class hierarchy mimicking the structure of an Xcode
  pbxproj file with methods to dump them as text file.

tools/gn/xcode_writer.{cc,h}

  Implements generation of Xcode project structure from GN settings
  and for dumping the output to pbxproj and xcworkspacedata files.

tools/gn/*

  Add product_type field to create_bundle target. This is used when
  generating the Xcode project file to use the correct product type
  in Xcode.

  Fix a minor DCHECK failure in create_bundle_target_generator.cc.

build/config/compiler/BUILD.gn

  Remove -fdebug-prefix-map from the clang command-line as it does
  not work with Xcode (and is unnecessary as the path given to the
  compiler are relative already).

  With this flag, Xcode cannot find the source file when debbugging
  the application, without it breakpoint and displaying code work.

BUG=597975
Committed: https://crrev.com/6ee04b5ce48981fd997b94c58240602df1a329d6
Cr-Commit-Position: refs/heads/master@{#391377}

[sdefresne, 2016-03-25 18:22:12]

Description was changed from

==========
Proof of concept to generate xcodeproject via "gn gen --ide=xcode".

BUG=597975
==========

to

==========
Proof of concept to generate xcodeproject via "gn gen --ide=xcode".

NOT READY FOR LANDING, NEEDS TO BE REFACTORED, CLEANED UP AND SOME
DESIGN ON HOW TO PASS INFORMATION TO THE GENERATOR (iOS vs OS X)
HAVE TO BE ADDRESSED FIRST.

BUG=597975
==========

[sdefresne, 2016-04-26 17:49:50]

Description was changed from

==========
Proof of concept to generate xcodeproject via "gn gen --ide=xcode".

NOT READY FOR LANDING, NEEDS TO BE REFACTORED, CLEANED UP AND SOME
DESIGN ON HOW TO PASS INFORMATION TO THE GENERATOR (iOS vs OS X)
HAVE TO BE ADDRESSED FIRST.

BUG=597975
==========

to

==========
[GN] Add support for generating Xcode projects.

tools/gn/xcode_object.{cc,h}

  Implements a class hierarchy mimicking the structure of an Xcode
  pbxproj file with methods to dump them as text file.

tools/gn/xcode_writer.{cc,h}

  Implements generation of Xcode project structure from GN settings
  and for dumping the output to pbxproj and xcworkspacedata files.

tools/gn/*

  Add product_type field to create_bundle target. This is used when
  generating the Xcode project file to use the correct product type
  in Xcode.

  Fix a minor DCHECK failure in create_bundle_target_generator.cc.

build/config/compiler/BUILD.gn

  Remove -fdebug-prefix-map from the clang command-line as it does
  not work with Xcode (and is unnecessary as the path given to the
  compiler are relative already).

  With this flag, Xcode cannot find the source file when debbugging
  the application, without it breakpoint and displaying code work.

BUG=597975
==========

[sdefresne, 2016-04-27 17:31:33]

Patchset #3 (id:40001) has been deleted

[sdefresne, 2016-04-27 17:41:09]

Patchset #4 (id:80001) has been deleted

[sdefresne, 2016-04-27 17:41:14]

Patchset #3 (id:60001) has been deleted

[sdefresne, 2016-04-27 17:41:19]

Patchset #1 (id:1) has been deleted

[sdefresne, 2016-04-27 17:41:23]

Patchset #1 (id:20001) has been deleted

[sdefresne, 2016-04-27 17:41:52]

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

[sdefresne, 2016-04-27 17:41:54]

Brett, can you take a look?

https://codereview.chromium.org/1827103005/diff/100001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1827103005/diff/100001/BUILD.gn#newcode228
BUILD.gn:228: "//tools/gn(${host_toolchain})",
This is for convenience when working with iOS and on gn at the same time.

https://codereview.chromium.org/1827103005/diff/100001/build/config/compiler/...
File build/config/compiler/BUILD.gn (right):

https://codereview.chromium.org/1827103005/diff/100001/build/config/compiler/...
build/config/compiler/BUILD.gn:366: cflags += [
"-fdebug-prefix-map=$absolute_path=." ]
This flag cause Xcode to be unable to map object code to source file. I appear
to be unnecessary too as all the path given to clang are relative to
root_out_dir, and according to strings, all .cc path are relative.

https://codereview.chromium.org/1827103005/diff/100001/build/config/ios/rules...
File build/config/ios/rules.gni (right):

https://codereview.chromium.org/1827103005/diff/100001/build/config/ios/rules...
build/config/ios/rules.gni:160: assert(product_type != "", "force use of
product_type")
I'll either remove this in a later CL or split my CL in two depending on what
you prefer.

https://codereview.chromium.org/1827103005/diff/100001/tools/gn/create_bundle...
File tools/gn/create_bundle_target_generator.cc (right):

https://codereview.chromium.org/1827103005/diff/100001/tools/gn/create_bundle...
tools/gn/create_bundle_target_generator.cc:64: if (!str.empty() &&
str[str.size() - 1] != '/')
Without this "bundle_dir->SwapValue(&str);" causes a DCHECK because the BUILD.gn
files do not use trailing "/". Is it better to fix the BUILD.gn files or to fix
the code here? The DCHECK is pre-existing my CL (it was introduced by recent
change by rsesek).

[brettw, 2016-04-27 23:09:30]

Sorry, I made almost no progress on this today. Will do tomorrow.

https://codereview.chromium.org/1827103005/diff/100001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1827103005/diff/100001/BUILD.gn#newcode228
BUILD.gn:228: "//tools/gn(${host_toolchain})",
I think this makes it a bit confusing about what these lists of targets are for.
I usually find it easiest to build GN itself into a different output directory
than you're targeting anyway.

[sdefresne, 2016-04-28 09:29:12]

https://codereview.chromium.org/1827103005/diff/100001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1827103005/diff/100001/BUILD.gn#newcode228
BUILD.gn:228: "//tools/gn(${host_toolchain})",
On 2016/04/27 at 23:09:30, brettw wrote:
> I think this makes it a bit confusing about what these lists of targets are
for. I usually find it easiest to build GN itself into a different output
directory than you're targeting anyway.

Ack, removed.

[brettw, 2016-04-28 21:16:17]

https://codereview.chromium.org/1827103005/diff/160001/build/config/compiler/...
File build/config/compiler/BUILD.gn (right):

https://codereview.chromium.org/1827103005/diff/160001/build/config/compiler/...
build/config/compiler/BUILD.gn:364: if (is_clang && !is_win && !is_nacl &&
!is_ios && !is_mac) {
Seems like we should just change this to:
  if (is_clang && is_linux) {

https://codereview.chromium.org/1827103005/diff/160001/build/config/ios/rules...
File build/config/ios/rules.gni (right):

https://codereview.chromium.org/1827103005/diff/160001/build/config/ios/rules...
build/config/ios/rules.gni:160: assert(product_type != "", "force use of
product_type")
I don't understand this line. You just set it 4 lines above.

https://codereview.chromium.org/1827103005/diff/160001/tools/gn/xcode_object.cc
File tools/gn/xcode_object.cc (right):

https://codereview.chromium.org/1827103005/diff/160001/tools/gn/xcode_object....
tools/gn/xcode_object.cc:18: namespace {
Blank line below

https://codereview.chromium.org/1827103005/diff/160001/tools/gn/xcode_object....
tools/gn/xcode_object.cc:138: if (iter != extension_map.end()) {
No {}

https://codereview.chromium.org/1827103005/diff/160001/tools/gn/xcode_object....
tools/gn/xcode_object.cc:231: }  // namespace
Blank line before.

https://codereview.chromium.org/1827103005/diff/160001/tools/gn/xcode_writer.cc
File tools/gn/xcode_writer.cc (right):

https://codereview.chromium.org/1827103005/diff/160001/tools/gn/xcode_writer....
tools/gn/xcode_writer.cc:32: namespace {
Blank line after here.

https://codereview.chromium.org/1827103005/diff/160001/tools/gn/xcode_writer....
tools/gn/xcode_writer.cc:120: for (size_t i = 0; i < hash.size() / 4; i++) {
No {}

https://codereview.chromium.org/1827103005/diff/160001/tools/gn/xcode_writer....
tools/gn/xcode_writer.cc:139: }  // namespace
Blank line above here.

https://codereview.chromium.org/1827103005/diff/160001/tools/gn/xcode_writer....
tools/gn/xcode_writer.cc:209: std::unique_ptr<RE2> pattern;
Can you use GN "label_pattern"s here instead? This makes it consistent with the
VS generator, visibility, assert_no_deps, etc.

Then we can remove the re2 dependency which will additionally make the
standalone build more complicated.

I would suggest factoring out the code in VisualStudioWriter::RunAndWriteFiles
that gets the list of targets to write into a helper that they can both share,
and also sharing the command line flag.

https://codereview.chromium.org/1827103005/diff/160001/tools/gn/xcode_writer....
tools/gn/xcode_writer.cc:228: std::sort(targets.begin(), targets.end());
Can you add a comment here about what you're doing with the sort and the lower
build? I was initially confused about why you were sorting by pointer.

[sdefresne, 2016-04-29 13:51:38]

Patchset #5 (id:180001) has been deleted

[sdefresne, 2016-04-30 11:25:03]

Please take another look.

https://codereview.chromium.org/1827103005/diff/160001/build/config/compiler/...
File build/config/compiler/BUILD.gn (right):

https://codereview.chromium.org/1827103005/diff/160001/build/config/compiler/...
build/config/compiler/BUILD.gn:364: if (is_clang && !is_win && !is_nacl &&
!is_ios && !is_mac) {
On 2016/04/28 at 21:16:17, brettw wrote:
> Seems like we should just change this to:
>   if (is_clang && is_linux) {

Done.

https://codereview.chromium.org/1827103005/diff/160001/build/config/ios/rules...
File build/config/ios/rules.gni (right):

https://codereview.chromium.org/1827103005/diff/160001/build/config/ios/rules...
build/config/ios/rules.gni:160: assert(product_type != "", "force use of
product_type")
On 2016/04/28 at 21:16:17, brettw wrote:
> I don't understand this line. You just set it 4 lines above.

Oh, this is because the current patchset introduce the "product_type" property
to "create_bundle" targets. So the bot complain about unused variable
"product_type" as they run "gn gen". This force an use of the variable so that
the bot do not complain. An alternative is to move the "product_type" assignment
to a separate CL.

https://codereview.chromium.org/1827103005/diff/160001/tools/gn/xcode_writer.cc
File tools/gn/xcode_writer.cc (right):

https://codereview.chromium.org/1827103005/diff/160001/tools/gn/xcode_writer....
tools/gn/xcode_writer.cc:32: namespace {
On 2016/04/28 at 21:16:17, brettw wrote:
> Blank line after here.

Done.

https://codereview.chromium.org/1827103005/diff/160001/tools/gn/xcode_writer....
tools/gn/xcode_writer.cc:120: for (size_t i = 0; i < hash.size() / 4; i++) {
On 2016/04/28 at 21:16:17, brettw wrote:
> No {}

Done.

https://codereview.chromium.org/1827103005/diff/160001/tools/gn/xcode_writer....
tools/gn/xcode_writer.cc:139: }  // namespace
On 2016/04/28 at 21:16:17, brettw wrote:
> Blank line above here.

Done.

https://codereview.chromium.org/1827103005/diff/160001/tools/gn/xcode_writer....
tools/gn/xcode_writer.cc:209: std::unique_ptr<RE2> pattern;
On 2016/04/28 at 21:16:17, brettw wrote:
> Can you use GN "label_pattern"s here instead? This makes it consistent with
the VS generator, visibility, assert_no_deps, etc.
> 
> Then we can remove the re2 dependency which will additionally make the
standalone build more complicated.
> 
> I would suggest factoring out the code in VisualStudioWriter::RunAndWriteFiles
that gets the list of targets to write into a helper that they can both share,
and also sharing the command line flag.

Done.

https://codereview.chromium.org/1827103005/diff/160001/tools/gn/xcode_writer....
tools/gn/xcode_writer.cc:228: std::sort(targets.begin(), targets.end());
On 2016/04/28 at 21:16:17, brettw wrote:
> Can you add a comment here about what you're doing with the sort and the lower
build? I was initially confused about why you were sorting by pointer.

Done.

[brettw, 2016-05-03 20:28:14]

lgtm

https://codereview.chromium.org/1827103005/diff/240001/build/config/ios/rules...
File build/config/ios/rules.gni (right):

https://codereview.chromium.org/1827103005/diff/240001/build/config/ios/rules...
build/config/ios/rules.gni:175: assert(product_type != "", "force use of
product_type")
Ah, in that case, can you add a better comment here in case you forget to come
back and delete this line when the new build is rolled?

[sdefresne, 2016-05-03 21:08:01]

Thank you for the review.

https://codereview.chromium.org/1827103005/diff/240001/build/config/ios/rules...
File build/config/ios/rules.gni (right):

https://codereview.chromium.org/1827103005/diff/240001/build/config/ios/rules...
build/config/ios/rules.gni:175: assert(product_type != "", "force use of
product_type")
On 2016/05/03 20:28:14, brettw wrote:
> Ah, in that case, can you add a better comment here in case you forget to come
> back and delete this line when the new build is rolled?

Done.

[sdefresne, 2016-05-03 21:08:45]

The CQ bit was checked by sdefresne@chromium.org

[sdefresne, 2016-05-03 21:08:46]

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/1827103005/#ps260001
(title: "Improve comment about "assert(product_type != "")"")

[commit-bot: I haz the power, 2016-05-03 21:09:04]

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

[commit-bot: I haz the power, 2016-05-03 22:20:27]

Committed patchset #8 (id:260001)

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

Description was changed from

==========
[GN] Add support for generating Xcode projects.

tools/gn/xcode_object.{cc,h}

  Implements a class hierarchy mimicking the structure of an Xcode
  pbxproj file with methods to dump them as text file.

tools/gn/xcode_writer.{cc,h}

  Implements generation of Xcode project structure from GN settings
  and for dumping the output to pbxproj and xcworkspacedata files.

tools/gn/*

  Add product_type field to create_bundle target. This is used when
  generating the Xcode project file to use the correct product type
  in Xcode.

  Fix a minor DCHECK failure in create_bundle_target_generator.cc.

build/config/compiler/BUILD.gn

  Remove -fdebug-prefix-map from the clang command-line as it does
  not work with Xcode (and is unnecessary as the path given to the
  compiler are relative already).

  With this flag, Xcode cannot find the source file when debbugging
  the application, without it breakpoint and displaying code work.

BUG=597975
==========

to

==========
[GN] Add support for generating Xcode projects.

tools/gn/xcode_object.{cc,h}

  Implements a class hierarchy mimicking the structure of an Xcode
  pbxproj file with methods to dump them as text file.

tools/gn/xcode_writer.{cc,h}

  Implements generation of Xcode project structure from GN settings
  and for dumping the output to pbxproj and xcworkspacedata files.

tools/gn/*

  Add product_type field to create_bundle target. This is used when
  generating the Xcode project file to use the correct product type
  in Xcode.

  Fix a minor DCHECK failure in create_bundle_target_generator.cc.

build/config/compiler/BUILD.gn

  Remove -fdebug-prefix-map from the clang command-line as it does
  not work with Xcode (and is unnecessary as the path given to the
  compiler are relative already).

  With this flag, Xcode cannot find the source file when debbugging
  the application, without it breakpoint and displaying code work.

BUG=597975

Committed: https://crrev.com/6ee04b5ce48981fd997b94c58240602df1a329d6
Cr-Commit-Position: refs/heads/master@{#391377}
==========

[commit-bot: I haz the power, 2016-05-03 22:22:10]

Patchset 8 (id:??) landed as
https://crrev.com/6ee04b5ce48981fd997b94c58240602df1a329d6
Cr-Commit-Position: refs/heads/master@{#391377}