ninja/mac: Support iOS codesign for ninja builds.

ninja/mac: Support iOS codesign for ninja builds.

Also:
Warn for unimplemented code sign keys
    (Resource Rules, Entitlements, and Other)
Add partial support for conditional keys for loading
    CODE_SIGN_IDENTITY[sdk=iphoneos*]
Fix UIDeviceFamily extra plist bug, should be per target, not global.
Add sig_test target which will only run if valid certs are found.

R=thakis@chromium.org

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

[justincohen, 2013-10-14 14:23:39]

This feels rather hacky.  Before I go down the path of setting up unit tests,
etc, can you look over this?  

Two major changes: 

1) Allow some conditional keys on iOS ([sdk=iphoneos*]) to support
CODE_SIGN_IDENTITY

2) Add a final codesign postbuild step after running security find-identity.

WDYT?  Thanks!

[Nico, 2013-10-14 16:54:55]

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

https://codereview.chromium.org/26895006/diff/1/pylib/gyp/generator/ninja.py#...
pylib/gyp/generator/ninja.py:1217: if self.target.type == "executable" and
output_binary is not None:
Does codesigning run before or after explicit "postbuild" entries in your gyp
file? (Does chrome/ios use explicit postbuilds?)

https://codereview.chromium.org/26895006/diff/1/pylib/gyp/xcode_emulation.py
File pylib/gyp/xcode_emulation.py (right):

https://codereview.chromium.org/26895006/diff/1/pylib/gyp/xcode_emulation.py#...
pylib/gyp/xcode_emulation.py:53: else:
Why else?

[justincohen, 2013-10-14 16:59:12]

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

https://codereview.chromium.org/26895006/diff/1/pylib/gyp/generator/ninja.py#...
pylib/gyp/generator/ninja.py:1217: if self.target.type == "executable" and
output_binary is not None:
It runs after -- very last step.

On 2013/10/14 16:54:55, Nico wrote:
> Does codesigning run before or after explicit "postbuild" entries in your gyp
> file? (Does chrome/ios use explicit postbuilds?)

https://codereview.chromium.org/26895006/diff/1/pylib/gyp/xcode_emulation.py
File pylib/gyp/xcode_emulation.py (right):

https://codereview.chromium.org/26895006/diff/1/pylib/gyp/xcode_emulation.py#...
pylib/gyp/xcode_emulation.py:53: else:
I wasn't sure what to do here.  I thought it would be safe to leave the warning
for non-iOS builds.

On 2013/10/14 16:54:55, Nico wrote:
> Why else?

[Nico, 2013-10-14 17:00:07]

https://codereview.chromium.org/26895006/diff/1/pylib/gyp/xcode_emulation.py
File pylib/gyp/xcode_emulation.py (right):

https://codereview.chromium.org/26895006/diff/1/pylib/gyp/xcode_emulation.py#...
pylib/gyp/xcode_emulation.py:53: else:
On 2013/10/14 16:59:12, justincohen wrote:
> I wasn't sure what to do here.  I thought it would be safe to leave the
warning
> for non-iOS builds.

Sure, but shouldn't you have it for ios builds too?

> 
> On 2013/10/14 16:54:55, Nico wrote:
> > Why else?
>

[justincohen, 2013-10-14 17:01:49]

https://codereview.chromium.org/26895006/diff/1/pylib/gyp/xcode_emulation.py
File pylib/gyp/xcode_emulation.py (right):

https://codereview.chromium.org/26895006/diff/1/pylib/gyp/xcode_emulation.py#...
pylib/gyp/xcode_emulation.py:53: else:
I'll change it not warn for ios/sdk=iphoneos*, but warn for everything else.


On 2013/10/14 17:00:07, Nico wrote:
> On 2013/10/14 16:59:12, justincohen wrote:
> > I wasn't sure what to do here.  I thought it would be safe to leave the
> warning
> > for non-iOS builds.
> 
> Sure, but shouldn't you have it for ios builds too?
> 
> > 
> > On 2013/10/14 16:54:55, Nico wrote:
> > > Why else?
> > 
>

[Nico, 2013-10-14 17:02:24]

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

https://codereview.chromium.org/26895006/diff/1/pylib/gyp/generator/ninja.py#...
pylib/gyp/generator/ninja.py:1217: if self.target.type == "executable" and
output_binary is not None:
On 2013/10/14 16:59:12, justincohen wrote:
> It runs after -- very last step.

Hm, that's annoying :-/

Maybe we should change the emulation api to be like:

postbuilds = self.xcode_settings.AddImplicitPostbuilds(postbuilds, ...)

and then the strip and codesign steps could both be added there (the first in
front, the second in the back). Having two calls to add implicit postbuilds is
somewhat lame

Be sure to have tests that check that codesigning runs after regular postbuilds.

> 
> On 2013/10/14 16:54:55, Nico wrote:
> > Does codesigning run before or after explicit "postbuild" entries in your
gyp
> > file? (Does chrome/ios use explicit postbuilds?)
>

[justincohen, 2013-10-14 22:29:09]

Do we have, or can we add, certificates to the bots so we can codesign builds?

That may prevent usable unit tests.

I changed the ninja to use AddImplicitPostbuilds which combines
GetTargetPostbuilds and _GetIOSPostbuilds, and I changed the way we handle
conditional keys.  PTAL!

[Nico, 2013-10-15 02:12:56]

I like this CL.

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation.py
File pylib/gyp/xcode_emulation.py (right):

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:58: def _ConvertConditionalKeys(self, configname):
docstring

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:62: # If you need more, speak up at
http://crbug.com/122592
indented too far

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:757: def GetTargetPostbuilds(self, configname,
output, output_binary, quiet=False):
Can you  change make.py to call AddImplicitPostbuilds() too and then make this
private (i.e. prepend a '_')?

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:769: if not (self.isIOS and is_app):
this can read self.spec['type'], no need to for the is_app parameter

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:775: return []
now self.configname is wrong

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:791: return ["/usr/bin/codesign -v --force --sign
%s %s" %
the rest of this file uses ' quotes

why "/usr/bin"?

why "-v"? do you want "--verify"? (`man codesign`: "if other actions (sign,
display, etc.) are also requested, -v is interpreted to mean --verbose.", and I
think you have --sign here)

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:796: postbuilds, quiet=False):
docstring

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:800: return (pre + postbuilds + post)
no parens

https://codereview.chromium.org/26895006/diff/29001/test/ios/app-bundle/test-...
File test/ios/app-bundle/test-device.gyp (right):

https://codereview.chromium.org/26895006/diff/29001/test/ios/app-bundle/test-...
test/ios/app-bundle/test-device.gyp:38: },
can you add a target that has a postbuild and code signing and tests that
codesigning happens after postbuilds? (by having the postbuild check for a
signature, I suppose)

[Nico, 2013-10-15 02:19:28]

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation.py
File pylib/gyp/xcode_emulation.py (right):

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:778: '-v'], stdout=subprocess.PIPE)
why -v here?

[Nico, 2013-10-15 02:22:28]

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation.py
File pylib/gyp/xcode_emulation.py (right):

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:778: '-v'], stdout=subprocess.PIPE)
On 2013/10/15 02:19:29, Nico wrote:
> why -v here?

Ignore this, I misread the man page for `security`.

[Nico, 2013-10-15 02:31:31]

Do you want to add "not implemented" warnings for CODE_SIGN_RESOURCE_RULES_PATH
, OTHER_CODE_SIGN_FLAGS , CODE_SIGN_ENTITLEMENTS ?

[Nico, 2013-10-15 02:33:27]

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation.py
File pylib/gyp/xcode_emulation.py (right):

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:775: return []
On 2013/10/15 02:12:57, Nico wrote:
> now self.configname is wrong

Also, setting this to "" (the empty string) apparently disables code signing, so
'' might be a better default value than None

[Nico, 2013-10-15 03:21:17]

For the keys, I think for now you can build the targets needing signing only if
`security  find-identity -p codesigning  -v` finds certs. Then you (and others)
can run these tests locally, the bots will run them if we install some cert
there (we might or might not), and it's good enough for now.

To not lose coverage on the bots, please add a new target instead of adding code
signing to an existing target.

[justincohen, 2013-10-15 16:47:33]

Done!  PTAL!

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation.py
File pylib/gyp/xcode_emulation.py (right):

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:58: def _ConvertConditionalKeys(self, configname):
On 2013/10/15 02:12:57, Nico wrote:
> docstring

Done.

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:62: # If you need more, speak up at
http://crbug.com/122592
On 2013/10/15 02:12:57, Nico wrote:
> indented too far

Done.

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:757: def GetTargetPostbuilds(self, configname,
output, output_binary, quiet=False):
On 2013/10/15 02:12:57, Nico wrote:
> Can you  change make.py to call AddImplicitPostbuilds() too and then make this
> private (i.e. prepend a '_')?

Done.

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:769: if not (self.isIOS and is_app):
On 2013/10/15 02:12:57, Nico wrote:
> this can read self.spec['type'], no need to for the is_app parameter

Done.

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:775: return []
On 2013/10/15 02:12:57, Nico wrote:
> now self.configname is wrong

Done.

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:775: return []
Done

On 2013/10/15 02:33:27, Nico wrote:
> On 2013/10/15 02:12:57, Nico wrote:
> > now self.configname is wrong
> 
> Also, setting this to "" (the empty string) apparently disables code signing,
so
> '' might be a better default value than None

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:778: '-v'], stdout=subprocess.PIPE)
On 2013/10/15 02:19:29, Nico wrote:
> why -v here?

Done.

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:778: '-v'], stdout=subprocess.PIPE)
On 2013/10/15 02:22:28, Nico wrote:
> On 2013/10/15 02:19:29, Nico wrote:
> > why -v here?
> 
> Ignore this, I misread the man page for `security`.

Done.

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:791: return ["/usr/bin/codesign -v --force --sign
%s %s" %
Yeah, it was as verbose.  I removed it.

On 2013/10/15 02:12:57, Nico wrote:
> the rest of this file uses ' quotes
> 
> why "/usr/bin"?
> 
> why "-v"? do you want "--verify"? (`man codesign`: "if other actions (sign,
> display, etc.) are also requested, -v is interpreted to mean --verbose.", and
I
> think you have --sign here)

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:796: postbuilds, quiet=False):
On 2013/10/15 02:12:57, Nico wrote:
> docstring

Done.

https://codereview.chromium.org/26895006/diff/29001/pylib/gyp/xcode_emulation...
pylib/gyp/xcode_emulation.py:800: return (pre + postbuilds + post)
On 2013/10/15 02:12:57, Nico wrote:
> no parens

Done.

https://codereview.chromium.org/26895006/diff/29001/test/ios/app-bundle/test-...
File test/ios/app-bundle/test-device.gyp (right):

https://codereview.chromium.org/26895006/diff/29001/test/ios/app-bundle/test-...
test/ios/app-bundle/test-device.gyp:38: },
On 2013/10/15 02:12:57, Nico wrote:
> can you add a target that has a postbuild and code signing and tests that
> codesigning happens after postbuilds? (by having the postbuild check for a
> signature, I suppose)

Done.

[Nico, 2013-10-15 17:05:47]

lgtm