[iOS/GN] Convert //ios/third_party/earl_grey to build with GN.

[iOS/GN] Convert //ios/third_party/earl_grey to build with GN.

Fix the ios_framework_bundle template to create a new public config
so that anyone depending on the target will correctly link against
it. Change framework_bundle to use public_deps in order to make the
public config accessible.

Create //ios/third_party/earl_grey target.

BUG=599321
Committed: https://crrev.com/22de3c95e286c337ebdf32e424f911d00f209350
Cr-Commit-Position: refs/heads/master@{#387265}

[sdefresne, 2016-04-12 15:55:16]

Patchset #1 (id:1) has been deleted

[sdefresne, 2016-04-12 15:55:50]

sdefresne@chromium.org changed reviewers:
+ dpranke@chromium.org, rsesek@chromium.org

[sdefresne, 2016-04-12 15:55:51]

rsesek: Please take a look.

dpranke: Needs OWNERS review for BUILD.gn.

[Robert Sesek, 2016-04-12 17:38:24]

https://codereview.chromium.org/1879003002/diff/20001/ios/third_party/earl_gr...
File ios/third_party/earl_grey/BUILD.gn (right):

https://codereview.chromium.org/1879003002/diff/20001/ios/third_party/earl_gr...
ios/third_party/earl_grey/BUILD.gn:26:
ios_framework_bundle("EarlGrey.framework") {
I'm not sure the target names should have .framework in them, since that'd be
like having shard_library("libfoo.so"). Maybe we change the framework_bundle()
target name to be _framework_name ?

[sdefresne, 2016-04-12 17:53:37]

On 2016/04/12 at 17:38:24, rsesek wrote:
>
https://codereview.chromium.org/1879003002/diff/20001/ios/third_party/earl_gr...
> File ios/third_party/earl_grey/BUILD.gn (right):
> 
>
https://codereview.chromium.org/1879003002/diff/20001/ios/third_party/earl_gr...
> ios/third_party/earl_grey/BUILD.gn:26:
ios_framework_bundle("EarlGrey.framework") {
> I'm not sure the target names should have .framework in them, since that'd be
like having shard_library("libfoo.so"). Maybe we change the framework_bundle()
target name to be _framework_name ?

If I name the targets "EarlGrey" and "OCHamcrest", I get the following errors
while building:

$ gn gen --args='target_os="ios"' out/Default
$ ninja -j 1000 -C out/simulator EarlGrey
ninja: Entering directory `out/Default'
ninja: warning: multiple rules generate
obj/ios/third_party/ochamcrest/ochamcrest.stamp. builds involving this target
will not be correct; continuing anyway [-w dupbuild=warn]
ninja: error: 'obj/ios/third_party/ochamcrest/OCHamcrest.stamp', needed by
'obj/ios/third_party/ochamcrest/ochamcrest.stamp', missing and no known rule to
make it

So, my options are to leave as is (i.e. "EarlGrey.framework" and
"OCHamcrest.framework") or to remove the wrapper and name them after the
directory they are in (i.e. "earl_grey" and "ochamcrest"). Or to figure out why
I get this error, but it is a bit of a mystery. My current hypothesis is that
I'm using a case-insensitive partition for my OS X development which cause some
names to collision.

[Dirk Pranke, 2016-04-12 18:42:03]

On 2016/04/12 17:53:37, sdefresne wrote:
> On 2016/04/12 at 17:38:24, rsesek wrote:
> >
>
https://codereview.chromium.org/1879003002/diff/20001/ios/third_party/earl_gr...
> > File ios/third_party/earl_grey/BUILD.gn (right):
> > 
> >
>
https://codereview.chromium.org/1879003002/diff/20001/ios/third_party/earl_gr...
> > ios/third_party/earl_grey/BUILD.gn:26:
> ios_framework_bundle("EarlGrey.framework") {
> > I'm not sure the target names should have .framework in them, since that'd
be
> like having shard_library("libfoo.so"). Maybe we change the framework_bundle()
> target name to be _framework_name ?
> 
> If I name the targets "EarlGrey" and "OCHamcrest", I get the following errors
> while building:
> 
> $ gn gen --args='target_os="ios"' out/Default
> $ ninja -j 1000 -C out/simulator EarlGrey
> ninja: Entering directory `out/Default'
> ninja: warning: multiple rules generate
> obj/ios/third_party/ochamcrest/ochamcrest.stamp. builds involving this target
> will not be correct; continuing anyway [-w dupbuild=warn]
> ninja: error: 'obj/ios/third_party/ochamcrest/OCHamcrest.stamp', needed by
> 'obj/ios/third_party/ochamcrest/ochamcrest.stamp', missing and no known rule
to
> make it
> 
> So, my options are to leave as is (i.e. "EarlGrey.framework" and
> "OCHamcrest.framework") or to remove the wrapper and name them after the
> directory they are in (i.e. "earl_grey" and "ochamcrest"). Or to figure out
why
> I get this error, but it is a bit of a mystery. My current hypothesis is that
> I'm using a case-insensitive partition for my OS X development which cause
some
> names to collision.

You should figure out why you're getting the error. We also shouldn't be
generating two targets that different only in case, so figuring out why that
is happening is part of the task.

[sdefresne, 2016-04-13 11:31:21]

Description was changed from

==========
[iOS/GN] Convert //ios/third_party/earl_grey to build with GN.

Fix the ios_framework_bundle template to create a new public config
so that anyone depending on the target will correctly link against
it. Change framework_bundle to use public_deps in order to make the
public config accessible.

Rename "ochamcrest" target to "OCHamcrest.framework" and add a group
target (to simplify dependencies while still allowing using the full
framework name on the ninja command-line).

Create //ios/third_party/earl_grey target using the same group proxy
and real target named "EarlGrey.framework".

BUG=599321
==========

to

==========
[iOS/GN] Convert //ios/third_party/earl_grey to build with GN.

Fix the ios_framework_bundle template to create a new public config
so that anyone depending on the target will correctly link against
it. Change framework_bundle to use public_deps in order to make the
public config accessible.

Create //ios/third_party/earl_grey target.

BUG=599321
==========

[sdefresne, 2016-04-13 11:41:17]

On 2016/04/12 at 18:42:03, dpranke wrote:
> You should figure out why you're getting the error. We also shouldn't be
> generating two targets that different only in case, so figuring out why that
> is happening is part of the task.

OK, I found what was the issue. In fact there were more than one issue.

First by naming my targets "ochamcrest" and "OCHamcrest", I ran into the
case-insentivity issue of the HFS+ file system that I'm using. For each target,
gn generates a $target_name.ninja file. Since both my target had the same name,
they ended up overwriting each other, and thus the build failed (I'm not the
only one complaining about HFS+ brain damaged design, see
http://www.cio.com/article/2868393/linus-torvalds-apples-hfs-is-probably-the-...).

However, even with a case-sensitive file system, we still ran in "ninja" warning
because the earl_grey_internal_shared_library targets generates a file named
$root_out_dir/EarlGrey and there is a target named EarlGrey. Since for ninja,
all path relative to $root_out_dir are implicit targets, it ends up complaining
about to targets with the same name (this is only an issue because the EarlGrey
targets does not generate the EarlGrey file).

Since I understand the issue, I think the easiest way to fix it is to use a
different name for the target. Another option would be to change the directory
where the shared library is built, and then the implicit target won't be
generated and we can use "EarlGrey" and "OCHamcrest" names for the targets (I'll
try to do this in a followup).

Please take another look at the CL.

https://codereview.chromium.org/1879003002/diff/20001/ios/third_party/earl_gr...
File ios/third_party/earl_grey/BUILD.gn (right):

https://codereview.chromium.org/1879003002/diff/20001/ios/third_party/earl_gr...
ios/third_party/earl_grey/BUILD.gn:26:
ios_framework_bundle("EarlGrey.framework") {
On 2016/04/12 at 17:38:24, Robert Sesek wrote:
> I'm not sure the target names should have .framework in them, since that'd be
like having shard_library("libfoo.so"). Maybe we change the framework_bundle()
target name to be _framework_name ?

Removed this and went with directory name as the target name.

[Robert Sesek, 2016-04-13 15:26:27]

LGTM

Thanks for tracking down the issues!

On 2016/04/13 11:41:17, sdefresne wrote:
> However, even with a case-sensitive file system, we still ran in "ninja"
warning
> because the earl_grey_internal_shared_library targets generates a file named
> $root_out_dir/EarlGrey and there is a target named EarlGrey. Since for ninja,
> all path relative to $root_out_dir are implicit targets, it ends up
complaining
> about to targets with the same name (this is only an issue because the
EarlGrey
> targets does not generate the EarlGrey file).

Right, I think we discussed this a little on Friday. Seems like this is another
reason why we want to do the thing below...

> Since I understand the issue, I think the easiest way to fix it is to use a
> different name for the target. Another option would be to change the directory
> where the shared library is built, and then the implicit target won't be
> generated and we can use "EarlGrey" and "OCHamcrest" names for the targets
(I'll
> try to do this in a followup).

I think changing the directory is going to be the right intermediate-term
solution, based on the discussion in "[gn-dev] Location of intermediate library
outputs in bundles". At the very least we don't want the intermediate shared
library that will be put into a bundle to be a top-level target.

[Dirk Pranke, 2016-04-14 02:47:51]

lgtm. thanks for digging in to it.

[sdefresne, 2016-04-14 08:18:49]

The CQ bit was checked by sdefresne@chromium.org

[commit-bot: I haz the power, 2016-04-14 08:19:03]

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

[commit-bot: I haz the power, 2016-04-14 08:51:22]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-04-14 08:52:43]

Description was changed from

==========
[iOS/GN] Convert //ios/third_party/earl_grey to build with GN.

Fix the ios_framework_bundle template to create a new public config
so that anyone depending on the target will correctly link against
it. Change framework_bundle to use public_deps in order to make the
public config accessible.

Create //ios/third_party/earl_grey target.

BUG=599321
==========

to

==========
[iOS/GN] Convert //ios/third_party/earl_grey to build with GN.

Fix the ios_framework_bundle template to create a new public config
so that anyone depending on the target will correctly link against
it. Change framework_bundle to use public_deps in order to make the
public config accessible.

Create //ios/third_party/earl_grey target.

BUG=599321

Committed: https://crrev.com/22de3c95e286c337ebdf32e424f911d00f209350
Cr-Commit-Position: refs/heads/master@{#387265}
==========

[commit-bot: I haz the power, 2016-04-14 08:52:44]

Patchset 2 (id:??) landed as
https://crrev.com/22de3c95e286c337ebdf32e424f911d00f209350
Cr-Commit-Position: refs/heads/master@{#387265}