Enable custom suffixes for Mac bundles.

Enable custom suffixes for Mac bundles.

[jsartisohn, 2014-07-17 14:11:53]

Can someone of the gyp reviewers pleasse comment on this issue, whether Mac
bundles are intentionally left out of the suffix treatment or if this is really
a bug.

One of our projects creates multiple sub-bundles within a "main" bundle that
have all the same name but different extensions (and content). When we try to
create them with Gyp, we get an error saying "Found multiple children with path"
(in xcodeproj_file.py:1146).
This patch fixes the issue and doesn't break any local buildbot tests.

[jsartisohn, 2014-07-24 13:31:59]

Added some random reviewers, because no one replied to this review up until now.

[Mark Mentovai, 2014-07-24 13:44:13]

Can you add a test?

[jsartisohn, 2014-07-24 13:48:16]

On 2014/07/24 13:44:13, Mark Mentovai wrote:
> Can you add a test?

You mean a test that reproduces the bug? Sure, I'll have to talk to cschiemann
first, but I'll report back.

Thanks for the reply.

[Mark Mentovai, 2014-07-24 13:49:36]

I mean something to put in test/mac that runs when you run gyptest.py. It should
be something that fails before this change and passes afterwards.

[jsartisohn, 2014-07-24 13:50:12]

On 2014/07/24 13:49:36, Mark Mentovai wrote:
> I mean something to put in test/mac that runs when you run gyptest.py. It
should
> be something that fails before this change and passes afterwards.

Yeah right. I'll do that.

[Nico, 2014-07-24 14:17:39]

Does product_extension not do what you want? (Just set it to 'app.foo'.)

If that doesn't work, you can look at test/mac/gyptest-loadable-module.py /
test/mac/loadable-module/test.gyp for a rough example test (the first check in
that test is probably all you need.) Maybe you could even add it to that test /
gyp file.

[jsartisohn, 2014-07-29 11:01:55]

On 2014/07/24 14:17:39, Nico (away) wrote:
> Does product_extension not do what you want? (Just set it to 'app.foo'.)
> 
> If that doesn't work, you can look at test/mac/gyptest-loadable-module.py /
> test/mac/loadable-module/test.gyp for a rough example test (the first check in
> that test is probably all you need.) Maybe you could even add it to that test
/
> gyp file.

I opted for doing a separate test. This isn't really about if the content of the
loadable_modules if correct, but rather about the creation of targets with
identical product_names but different product_extensions.
And yes, as you said, product extension does exactly what I need, but currently
product_extension is ignored for loadable_modules in mac_bundles.

The last question is, whether I should submit the test separately to verify that
it fails and afterwards submit the fix, if it is OK to submit them both at the
same time, like the current patch set (2) does?

Thanks for reviewing.

[Mark Mentovai, 2014-07-29 12:55:08]

LG with these changes.

https://codereview.chromium.org/401633002/diff/20001/test/mac/gyptest-loadabl...
File test/mac/gyptest-loadable-module-collision.py (right):

https://codereview.chromium.org/401633002/diff/20001/test/mac/gyptest-loadabl...
test/mac/gyptest-loadable-module-collision.py:1: #!/usr/bin/env python
The filename should say something about product_extension. The current name
makes it sound like you’re trying to make things collide (and would expect a
failure).

https://codereview.chromium.org/401633002/diff/20001/test/mac/loadable-module...
File test/mac/loadable-module-collision/test.gyp (right):

https://codereview.chromium.org/401633002/diff/20001/test/mac/loadable-module...
test/mac/loadable-module-collision/test.gyp:1: {
We normally stick the copyright boilerplate in these too, I think.

https://codereview.chromium.org/401633002/diff/20001/test/mac/loadable-module...
test/mac/loadable-module-collision/test.gyp:2: 'targets': [{
And this scope is normally indented by +2.

[Mark Mentovai, 2014-07-29 12:55:30]

https://codereview.chromium.org/401633002/diff/20001/test/mac/loadable-module...
File test/mac/loadable-module-collision/src.cc (right):

https://codereview.chromium.org/401633002/diff/20001/test/mac/loadable-module...
test/mac/loadable-module-collision/src.cc:1: // Copyright (c) 2014 Google Inc.
All rights reserved.
(In case it wasn’t obvious, the naming comment applies to this directory too.)

[jsartisohn, 2014-07-29 13:37:10]

Hope this name is more appropriate. If not pleas suggest a correction.

https://codereview.chromium.org/401633002/diff/20001/test/mac/gyptest-loadabl...
File test/mac/gyptest-loadable-module-collision.py (right):

https://codereview.chromium.org/401633002/diff/20001/test/mac/gyptest-loadabl...
test/mac/gyptest-loadable-module-collision.py:1: #!/usr/bin/env python
On 2014/07/29 12:55:08, Mark Mentovai wrote:
> The filename should say something about product_extension. The current name
> makes it sound like you’re trying to make things collide (and would expect a
> failure).

Done.

https://codereview.chromium.org/401633002/diff/20001/test/mac/loadable-module...
File test/mac/loadable-module-collision/src.cc (right):

https://codereview.chromium.org/401633002/diff/20001/test/mac/loadable-module...
test/mac/loadable-module-collision/src.cc:1: // Copyright (c) 2014 Google Inc.
All rights reserved.
On 2014/07/29 12:55:30, Mark Mentovai wrote:
> (In case it wasn’t obvious, the naming comment applies to this directory too.)

Done.

https://codereview.chromium.org/401633002/diff/20001/test/mac/loadable-module...
File test/mac/loadable-module-collision/test.gyp (right):

https://codereview.chromium.org/401633002/diff/20001/test/mac/loadable-module...
test/mac/loadable-module-collision/test.gyp:1: {
On 2014/07/29 12:55:08, Mark Mentovai wrote:
> We normally stick the copyright boilerplate in these too, I think.

Done.

https://codereview.chromium.org/401633002/diff/20001/test/mac/loadable-module...
test/mac/loadable-module-collision/test.gyp:2: 'targets': [{
On 2014/07/29 12:55:08, Mark Mentovai wrote:
> And this scope is normally indented by +2.

Sure. Forgot to change this.
Internally we don't indent the first level to save some horizontal space.
Done.

[Mark Mentovai, 2014-07-29 13:54:47]

LGTM

[jsartisohn, 2014-07-29 13:56:57]

On 2014/07/29 13:54:47, Mark Mentovai wrote:
> LGTM

How does the submit process work? I don't have write permissions for the gyp
repo. Also I cannot tigger the Trybots.

[Mark Mentovai, 2014-07-29 14:02:32]

Committed r1957

https://codereview.chromium.org/401633002/diff/60001/test/mac/gyptest-loadabl...
File test/mac/gyptest-loadable-module-bundle-product-extension.py (right):

https://codereview.chromium.org/401633002/diff/60001/test/mac/gyptest-loadabl...
test/mac/gyptest-loadable-module-bundle-product-extension.py:21: CHDIR =
'loadable-module-collision'
I had to fix this line.

[jsartisohn, 2014-07-29 14:13:29]

On 2014/07/29 14:02:32, Mark Mentovai wrote:
> Committed r1957
> 
>
https://codereview.chromium.org/401633002/diff/60001/test/mac/gyptest-loadabl...
> File test/mac/gyptest-loadable-module-bundle-product-extension.py (right):
> 
>
https://codereview.chromium.org/401633002/diff/60001/test/mac/gyptest-loadabl...
> test/mac/gyptest-loadable-module-bundle-product-extension.py:21: CHDIR =
> 'loadable-module-collision'
> I had to fix this line.

Meh, yea, missed that one. Thanks.