Adds generator support for Xcode 5 xctest targets.

Adds generator support for Xcode 5 xctest targets.

Update the xcode generator to set the target productType in the pbxproj file to "com.apple.product-type.bundle.unit-test", if the target sets the "xctest_bundle" property.

Patch by Jon Wall <jonwall@google.com>;

BUG=
R=mark@chromium.org

Committed: https://code.google.com/p/gyp/source/detail?r=1774

[Mark Mentovai, 2013-10-30 16:40:27]

https://codereview.chromium.org/51413002/diff/1/pylib/gyp/generator/xcode.py
File pylib/gyp/generator/xcode.py (right):

https://codereview.chromium.org/51413002/diff/1/pylib/gyp/generator/xcode.py#...
pylib/gyp/generator/xcode.py:664: type_bundle_key += '+xctest'
This code snippet seems to say that loadable_module+bundle+xctest can be
produced, but that doesn’t seem right, and you don’t handle that case.

https://codereview.chromium.org/51413002/diff/1/pylib/gyp/generator/xcode.py#...
pylib/gyp/generator/xcode.py:730: 'xctest_host (%s) must be listed as a
dependency. (target %s)' %
Why? Can’t it be an indirect dependency?

https://codereview.chromium.org/51413002/diff/1/pylib/gyp/generator/xcode.py#...
pylib/gyp/generator/xcode.py:735: # Prune any duplicated dependencies.
Why?

[jonwall, 2013-10-30 19:07:38]

https://codereview.chromium.org/51413002/diff/1/pylib/gyp/generator/xcode.py
File pylib/gyp/generator/xcode.py (right):

https://codereview.chromium.org/51413002/diff/1/pylib/gyp/generator/xcode.py#...
pylib/gyp/generator/xcode.py:664: type_bundle_key += '+xctest'
On 2013/10/30 16:40:27, Mark Mentovai wrote:
> This code snippet seems to say that loadable_module+bundle+xctest can be
> produced, but that doesn’t seem right, and you don’t handle that case.

Done.

https://codereview.chromium.org/51413002/diff/1/pylib/gyp/generator/xcode.py#...
pylib/gyp/generator/xcode.py:730: 'xctest_host (%s) must be listed as a
dependency. (target %s)' %
On 2013/10/30 16:40:27, Mark Mentovai wrote:
> Why? Can’t it be an indirect dependency?

It looks like spec['dependencies'] includes indirect dependencies, if I'm
understanding you correctly.

https://codereview.chromium.org/51413002/diff/1/pylib/gyp/generator/xcode.py#...
pylib/gyp/generator/xcode.py:735: # Prune any duplicated dependencies.
On 2013/10/30 16:40:27, Mark Mentovai wrote:
> Why?

If you comment out this line, you'll see the test case I've added fails because
a category method cannot be found at runtime. The test case has this setup:

App target depends on Classes, which depends on shared.
Test target depends on App and test_support, which depends on shared.

By default this results in gyp setting up the Test target to link to
libtest_support.a and libshared.a. None of the dependencies from App are linked
in because App is a linkable target. This can work in many cases, but problems
can occur by linking to objects already linked to the bundle loader (in this
case libshared.a). The test case has a "Shared" class defined in libshared.a,
and category methods are added in libclasses.a and libtest_support.a. Without
this pruning of libshared.a the test bundle uses the definition of Shared class
from the test bundle (via libshared.a), and cannot find the category method
defined in the App bundle (via libclasses.a). Pruning out dependencies linked to
the xctest_host target (in this case, libshared.a) avoids this issue because the
test bundle uses the Shared class from the app bundle.

[Mark Mentovai, 2013-10-30 19:57:31]

https://codereview.chromium.org/51413002/diff/1/pylib/gyp/generator/xcode.py
File pylib/gyp/generator/xcode.py (right):

https://codereview.chromium.org/51413002/diff/1/pylib/gyp/generator/xcode.py#...
pylib/gyp/generator/xcode.py:730: 'xctest_host (%s) must be listed as a
dependency. (target %s)' %
jonwall wrote:
> On 2013/10/30 16:40:27, Mark Mentovai wrote:
> > Why? Can’t it be an indirect dependency?
> 
> It looks like spec['dependencies'] includes indirect dependencies, if I'm
> understanding you correctly.

I didn’t think that was true. You’re saying that if target a depends on b, and b
depends on c, when reaching here for target a, spec['dependencies'] will include
c?

https://codereview.chromium.org/51413002/diff/1/pylib/gyp/generator/xcode.py#...
pylib/gyp/generator/xcode.py:735: # Prune any duplicated dependencies.
This sounds like a generic problem with making any loadable_module depend on
something its bundle_loader also provides. It’s not a reason to start pruning
dependencies from loadable_modules (or a subset of loadable_modules) here in
some automated fashion in the generator. The problem should be corrected in the
.gyp files, not by this workaround.

If you find yourself in this sort of situation, a better generic solution to the
generic problem is often to make the “shared” target in your example into a
shared_library. Another option more specific to your example is to make
“test_support” not depend on “shared,” or to have a variant of test_support that
doesn’t depend on shared, for use in this situation.

[jonwall, 2013-10-30 21:04:57]

Updated to remove the dependency pruning logic. PTAL.

https://codereview.chromium.org/51413002/diff/1/pylib/gyp/generator/xcode.py
File pylib/gyp/generator/xcode.py (right):

https://codereview.chromium.org/51413002/diff/1/pylib/gyp/generator/xcode.py#...
pylib/gyp/generator/xcode.py:730: 'xctest_host (%s) must be listed as a
dependency. (target %s)' %
On 2013/10/30 19:57:31, Mark Mentovai wrote:
> jonwall wrote:
> > On 2013/10/30 16:40:27, Mark Mentovai wrote:
> > > Why? Can’t it be an indirect dependency?
> > 
> > It looks like spec['dependencies'] includes indirect dependencies, if I'm
> > understanding you correctly.
> 
> I didn’t think that was true. You’re saying that if target a depends on b, and
b
> depends on c, when reaching here for target a, spec['dependencies'] will
include
> c?

Yes.

https://codereview.chromium.org/51413002/diff/1/pylib/gyp/generator/xcode.py#...
pylib/gyp/generator/xcode.py:735: # Prune any duplicated dependencies.
Okay. It looks like I can get this to work by cloning the executable target and
adding any test dependencies to the executable target.

[Mark Mentovai, 2013-10-30 21:31:16]

Thanks. I can LG this after these minor changes.

https://codereview.chromium.org/51413002/diff/170001/pylib/gyp/generator/xcod...
File pylib/gyp/generator/xcode.py (right):

https://codereview.chromium.org/51413002/diff/170001/pylib/gyp/generator/xcod...
pylib/gyp/generator/xcode.py:656: is_bundle = int(spec.get('mac_bundle', 0))
The name mac_bundle was chosen because platform-specific things like this are
named with a platform prefix (like how generator-specific things are named with
generator prefixes). This is a huge help in cross-platform projects.

xctest_bundle, by contrast, doesn’t make as much sense from a self-documentation
perspective in a cross-platform project. The name according to existing GYP
usage would be better as 'mac_xctest', or perhaps 'mac_xctest_bundle'.

https://codereview.chromium.org/51413002/diff/170001/pylib/gyp/generator/xcod...
pylib/gyp/generator/xcode.py:783: if is_bundle:
It’s still possible for something to be both xctest and bundle to get things
like mac_bundle_resources here. That’s fine by me, but do you think it might
make sense to have all of these other is_bundle-related things also be turned on
for xctests that don’t explicitly specify that they’re mac_bundles?

https://codereview.chromium.org/51413002/diff/170001/test/mac/gyptest-xctest.py
File test/mac/gyptest-xctest.py (right):

https://codereview.chromium.org/51413002/diff/170001/test/mac/gyptest-xctest....
test/mac/gyptest-xctest.py:37: 
Drop the blank line here…

https://codereview.chromium.org/51413002/diff/170001/test/mac/xctest/MyClass.h
File test/mac/xctest/MyClass.h (right):

https://codereview.chromium.org/51413002/diff/170001/test/mac/xctest/MyClass....
test/mac/xctest/MyClass.h:4: #import <Foundation/Foundation.h>
Add a blank here, and in the rest of your files after the boilerplate stuff…

[jonwall, 2013-10-30 22:03:14]

https://codereview.chromium.org/51413002/diff/170001/pylib/gyp/generator/xcod...
File pylib/gyp/generator/xcode.py (right):

https://codereview.chromium.org/51413002/diff/170001/pylib/gyp/generator/xcod...
pylib/gyp/generator/xcode.py:656: is_bundle = int(spec.get('mac_bundle', 0))
On 2013/10/30 21:31:16, Mark Mentovai wrote:
> The name mac_bundle was chosen because platform-specific things like this are
> named with a platform prefix (like how generator-specific things are named
with
> generator prefixes). This is a huge help in cross-platform projects.
> 
> xctest_bundle, by contrast, doesn’t make as much sense from a
self-documentation
> perspective in a cross-platform project. The name according to existing GYP
> usage would be better as 'mac_xctest', or perhaps 'mac_xctest_bundle'.

Done.

https://codereview.chromium.org/51413002/diff/170001/pylib/gyp/generator/xcod...
pylib/gyp/generator/xcode.py:783: if is_bundle:
On 2013/10/30 21:31:16, Mark Mentovai wrote:
> It’s still possible for something to be both xctest and bundle to get things
> like mac_bundle_resources here. That’s fine by me, but do you think it might
> make sense to have all of these other is_bundle-related things also be turned
on
> for xctests that don’t explicitly specify that they’re mac_bundles?

Done - hopefully I understood your intention here?

https://codereview.chromium.org/51413002/diff/170001/test/mac/gyptest-xctest.py
File test/mac/gyptest-xctest.py (right):

https://codereview.chromium.org/51413002/diff/170001/test/mac/gyptest-xctest....
test/mac/gyptest-xctest.py:37: 
On 2013/10/30 21:31:16, Mark Mentovai wrote:
> Drop the blank line here…

Done.

https://codereview.chromium.org/51413002/diff/170001/test/mac/xctest/MyClass.h
File test/mac/xctest/MyClass.h (right):

https://codereview.chromium.org/51413002/diff/170001/test/mac/xctest/MyClass....
test/mac/xctest/MyClass.h:4: #import <Foundation/Foundation.h>
On 2013/10/30 21:31:16, Mark Mentovai wrote:
> Add a blank here, and in the rest of your files after the boilerplate stuff…

Done.

[Mark Mentovai, 2013-10-30 22:08:36]

Perfect. LGTM.

[Mark Mentovai, 2013-10-30 22:16:51]

https://codereview.chromium.org/51413002/diff/340001/test/prune_targets/lib1.cc
File test/prune_targets/lib1.cc (right):

https://codereview.chromium.org/51413002/diff/340001/test/prune_targets/lib1....
test/prune_targets/lib1.cc:5: #import "MyClass.h"
Ah, there’s something weird with your patch. I assume that this was supposed to
be the new file MyClass.m, and not an edit of lib1.cc in a completely unrelated
test.

[Mark Mentovai, 2013-10-30 22:45:15]

Committed patchset #7 manually as r1774 (presubmit successful).

[Mark Mentovai, 2013-10-30 22:45:33]

Committed GYP r1774.

https://codereview.chromium.org/51413002/diff/450001/test/mac/xctest/test.xco...
File test/mac/xctest/test.xcodeproj/xcshareddata/xcschemes/classes.xcscheme
(right):

https://codereview.chromium.org/51413002/diff/450001/test/mac/xctest/test.xco...
test/mac/xctest/test.xcodeproj/xcshareddata/xcschemes/classes.xcscheme:1: <?xml
version="1.0" encoding="UTF-8"?>
It might be nice, for future additions, to not have to supply this file but to
have it be generated somehow.