Add mac_framework_private_headers key to specify which headers should be copi...

Add mac_framework_private_headers key to specify which headers should be copied
into the PrivateHeaders folder in a Mac Framework.

[dglazkov, 2011-03-02 01:31:29]

This looks awesome!

[TVL, 2011-03-02 02:07:15]

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

http://codereview.chromium.org/6592088/diff/1/pylib/gyp/generator/xcode.py#ne...
pylib/gyp/generator/xcode.py:541: def AddPublicHeaderToTarget(header, pbxp,
xct):
Maybe extend this to take another arg to control public/private?  (because the
TODO would apply to the other function also)

[Mark Mentovai, 2011-03-02 21:56:35]

This looks good. I agree with TVL’s comment: merging AddPublicHeaderToTarget and
AddPrivateHeaderToTarget back into AddHeaderToTarget and making it take an extra
argument to decide between public and private seems like the way to go.

[abarth-chromium, 2011-03-03 06:38:56]

Updated.  Thoughts?

[Mark Mentovai, 2011-03-03 16:29:38]

Getting close.

http://codereview.chromium.org/6592088/diff/7001/pylib/gyp/generator/xcode.py
File pylib/gyp/generator/xcode.py (right):

http://codereview.chromium.org/6592088/diff/7001/pylib/gyp/generator/xcode.py...
pylib/gyp/generator/xcode.py:538: xct.ResourcesPhase().AddFile(resource,
settings)
This is wrong. What’s settings supposed to be here? (Is this even tested?)

http://codereview.chromium.org/6592088/diff/7001/pylib/gyp/generator/xcode.py...
pylib/gyp/generator/xcode.py:541: def AddHeaderToTarget(header, pbxp, xct,
settings):
There’s a tab on this line.

http://codereview.chromium.org/6592088/diff/7001/pylib/gyp/generator/xcode.py...
pylib/gyp/generator/xcode.py:543: # where it's used.
And this one too.

http://codereview.chromium.org/6592088/diff/7001/pylib/gyp/generator/xcode.py...
pylib/gyp/generator/xcode.py:544: xct.HeadersPhase().AddFile(header, settings)
I think the interface would be easier to use if this function accepted either a
str argument containing 'public' or 'private', or a bool argument along the
lines of is_public. Then, this function could put the known string matching the
desired setting into the AddFile call. The problem with the way it’s written now
is that the caller needs to know what the string’s supposed to look like. That’s
not so bad with just one caller for each of public and private, but if we start
wanting to add headers from elsewhere, it will quickly become untenable.

[abarth-chromium, 2011-03-03 16:45:40]

http://codereview.chromium.org/6592088/diff/7001/pylib/gyp/generator/xcode.py
File pylib/gyp/generator/xcode.py (right):

http://codereview.chromium.org/6592088/diff/7001/pylib/gyp/generator/xcode.py...
pylib/gyp/generator/xcode.py:538: xct.ResourcesPhase().AddFile(resource,
settings)
On 2011/03/03 16:29:38, Mark Mentovai wrote:
> This is wrong. What’s settings supposed to be here? (Is this even tested?)

This is left-over from a previous approach.  How do I run the tests?

http://codereview.chromium.org/6592088/diff/7001/pylib/gyp/generator/xcode.py...
pylib/gyp/generator/xcode.py:541: def AddHeaderToTarget(header, pbxp, xct,
settings):
On 2011/03/03 16:29:38, Mark Mentovai wrote:
> There’s a tab on this line.

Done.

http://codereview.chromium.org/6592088/diff/7001/pylib/gyp/generator/xcode.py...
pylib/gyp/generator/xcode.py:543: # where it's used.
On 2011/03/03 16:29:38, Mark Mentovai wrote:
> And this one too.

Done.

http://codereview.chromium.org/6592088/diff/7001/pylib/gyp/generator/xcode.py...
pylib/gyp/generator/xcode.py:544: xct.HeadersPhase().AddFile(header, settings)
On 2011/03/03 16:29:38, Mark Mentovai wrote:
> I think the interface would be easier to use if this function accepted either
a
> str argument containing 'public' or 'private', or a bool argument along the
> lines of is_public. Then, this function could put the known string matching
the
> desired setting into the AddFile call. The problem with the way it’s written
now
> is that the caller needs to know what the string’s supposed to look like.
That’s
> not so bad with just one caller for each of public and private, but if we
start
> wanting to add headers from elsewhere, it will quickly become untenable.

Done.

[Mark Mentovai, 2011-03-03 17:02:21]

LGTM. I’ll check this in.

You can run GYP tests by running gyptest.py. It’s also a good idea to run it
against Chromium and make sure it does the right thing.

http://codereview.chromium.org/6592088/diff/8002/pylib/gyp/generator/xcode.py
File pylib/gyp/generator/xcode.py (right):

http://codereview.chromium.org/6592088/diff/8002/pylib/gyp/generator/xcode.py...
pylib/gyp/generator/xcode.py:544: settings = '{ATTRIBUTES = (Private, ); }'
I’m going to make a small change here when I check it in. I’m going to transform
these three lines into
  settings = '{ATTRIBUTES = (%s, ); }' % ('Private', 'Public')[is_public]
which combines Python’s built-in string formatting with a ternary operator
equivalent.

[Mark Mentovai, 2011-03-03 17:04:42]

Committed r898. Thanks!