Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2060943002:
[iOS/GN] Add code signing rules to ios_app_bundle target. (Closed)
Description[iOS/GN] Add code signing rules to ios_app_bundle target.
Add a new script build/config/ios/codesign.py that copy the application
binary into the application bundle and code sign it.
Call the script from the ios_app_bundle target when ios_enable_code_signing
is true (default when targetting a device build) and remove some unused
variables (the code signing identity is generally set for the whole build).
BUG=600491
Committed: https://crrev.com/eecfedb63ad99ea127fd51bebf2c1b1e2e7785a0
Cr-Commit-Position: refs/heads/master@{#400229}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Fix code signing of "test" template #
Total comments: 12
Patch Set 3 : Address comments #
Messages
Total messages: 26 (11 generated)
sdefresne@chromium.org changed reviewers: + dpranke@chromium.org
Please take a look. Ignore the failure, I launch the CL against the bot, but since it depends changes in tools/gn the bots will fail.
On 2016/06/13 16:35:52, sdefresne wrote: > Please take a look. > > Ignore the failure, I launch the CL against the bot, but since it depends > changes in tools/gn the bots will fail. Oh, the build didn't fail because we disable code signing on device builds. Forgot about that.
sdefresne@chromium.org changed reviewers: + rsesek@chromium.org
Robert: please take a look as Mac/iOS build expert. Dirk: for OWNERS approval for testing/test.gni
https://codereview.chromium.org/2060943002/diff/1/testing/test.gni File testing/test.gni (right): https://codereview.chromium.org/2060943002/diff/1/testing/test.gni#newcode310 testing/test.gni:310: entitlements_path = "//testing/gtest_ios" "//testing/gtest_ios" is a directory and an invalid entitlements_path.
https://codereview.chromium.org/2060943002/diff/20001/build/config/ios/codesi... File build/config/ios/codesign.py (right): https://codereview.chromium.org/2060943002/diff/20001/build/config/ios/codesi... build/config/ios/codesign.py:18: nit: Remove the blank lines between class name and docstring, just like in functions. https://codereview.chromium.org/2060943002/diff/20001/build/config/ios/codesi... build/config/ios/codesign.py:168: class Entitlements(object): Document? https://codereview.chromium.org/2060943002/diff/20001/build/config/ios/codesi... build/config/ios/codesign.py:194: copy = CoreFoundation.CFDictionaryCreateMutable(None, 0, It may be a little bit easier to use plistlib to load the plist in and then use CF to write it out. The Python-CF bridge should auto-convert Python types to CF types when passed to CFPropertyListCreateData. https://codereview.chromium.org/2060943002/diff/20001/build/config/ios/codesi... build/config/ios/codesign.py:285: def CreateEntitlements(bundle, provisioning_profile, entitlements_path): Why have CreateEntitlements, Entitlements.Load, and Entitlements.__init__? https://codereview.chromium.org/2060943002/diff/20001/build/config/ios/codesi... build/config/ios/codesign.py:316: signature_file = os.path.join(bundle.path, "_CodeSignature", "CodeResources") nit: Switch to single quotes here for consistency with the rest of the file. https://codereview.chromium.org/2060943002/diff/20001/build/config/ios/codesi... build/config/ios/codesign.py:322: command = ['codesign', '--force', '--sign', args.identity, '--timestamp=none'] Should invoke codesign through xcrun.
Description was changed from ========== [iOS/GN] Add code signing rules to ios_app_bundle target. Add a new script build/config/ios/codesign.py that copy the application binary into the application bundle and code sign it (use CoreFoundation python module to process the plist files as the application Info.plist is in binary1 format and plistlib does not support that format before python 3.0). Bind the script to the ios_app_bundle target when ios_enable_code_signing is true (default when targetting a device build) and remove some unused variables (the code signing identity is generally set for the whole build). BUG=600491 ========== to ========== [iOS/GN] Add code signing rules to ios_app_bundle target. Add a new script build/config/ios/codesign.py that copy the application binary into the application bundle and code sign it. Call the script from the ios_app_bundle target when ios_enable_code_signing is true (default when targetting a device build) and remove some unused variables (the code signing identity is generally set for the whole build). BUG=600491 ==========
PTAL https://codereview.chromium.org/2060943002/diff/20001/build/config/ios/codesi... File build/config/ios/codesign.py (right): https://codereview.chromium.org/2060943002/diff/20001/build/config/ios/codesi... build/config/ios/codesign.py:18: On 2016/06/15 15:30:59, Robert Sesek wrote: > nit: Remove the blank lines between class name and docstring, just like in > functions. Done. https://codereview.chromium.org/2060943002/diff/20001/build/config/ios/codesi... build/config/ios/codesign.py:168: class Entitlements(object): On 2016/06/15 15:30:59, Robert Sesek wrote: > Document? Done. https://codereview.chromium.org/2060943002/diff/20001/build/config/ios/codesi... build/config/ios/codesign.py:194: copy = CoreFoundation.CFDictionaryCreateMutable(None, 0, On 2016/06/15 15:30:59, Robert Sesek wrote: > It may be a little bit easier to use plistlib to load the plist in and then use > CF to write it out. The Python-CF bridge should auto-convert Python types to CF > types when passed to CFPropertyListCreateData. Changed the code to use plistlib. https://codereview.chromium.org/2060943002/diff/20001/build/config/ios/codesi... build/config/ios/codesign.py:285: def CreateEntitlements(bundle, provisioning_profile, entitlements_path): On 2016/06/15 15:30:59, Robert Sesek wrote: > Why have CreateEntitlements, Entitlements.Load, and Entitlements.__init__? Removed this method (it was converting a entitlements template to a real file by doing the substitutions). https://codereview.chromium.org/2060943002/diff/20001/build/config/ios/codesi... build/config/ios/codesign.py:316: signature_file = os.path.join(bundle.path, "_CodeSignature", "CodeResources") On 2016/06/15 15:30:59, Robert Sesek wrote: > nit: Switch to single quotes here for consistency with the rest of the file. Done. https://codereview.chromium.org/2060943002/diff/20001/build/config/ios/codesi... build/config/ios/codesign.py:322: command = ['codesign', '--force', '--sign', args.identity, '--timestamp=none'] On 2016/06/15 15:30:59, Robert Sesek wrote: > Should invoke codesign through xcrun. Done.
LGTM
Description was changed from ========== [iOS/GN] Add code signing rules to ios_app_bundle target. Add a new script build/config/ios/codesign.py that copy the application binary into the application bundle and code sign it. Call the script from the ios_app_bundle target when ios_enable_code_signing is true (default when targetting a device build) and remove some unused variables (the code signing identity is generally set for the whole build). BUG=600491 ========== to ========== [iOS/GN] Add code signing rules to ios_app_bundle target. Add a new script build/config/ios/codesign.py that copy the application binary into the application bundle and code sign it. Call the script from the ios_app_bundle target when ios_enable_code_signing is true (default when targetting a device build) and remove some unused variables (the code signing identity is generally set for the whole build). BUG=600491 ==========
sdefresne@chromium.org changed reviewers: + phajdan.jr@chromium.org - dpranke@chromium.org
-dpranke, +phajdan.jr: I need OWNERS approval for testing/test.gni, thank. If LGTY, can you CQ?
sdefresne@chromium.org changed reviewers: + brettw@chromium.org - phajdan.jr@chromium.org
On 2016/06/16 09:00:02, sdefresne wrote: > -dpranke, +phajdan.jr: I need OWNERS approval for testing/test.gni, thank. > > If LGTY, can you CQ? -phajdan.jr, +brettw: I need OWNERS approval for testing/test.gni, thank. If LGTY, can you CQ?
dpranke@chromium.org changed reviewers: + dpranke@chromium.org
The CQ bit was checked by dpranke@chromium.org
lgtm
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2060943002/40001
Thank you.
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply the patch.
The CQ bit was unchecked by commit-bot@chromium.org
CQ bit was unchecked
Message was sent while issue was closed.
Description was changed from ========== [iOS/GN] Add code signing rules to ios_app_bundle target. Add a new script build/config/ios/codesign.py that copy the application binary into the application bundle and code sign it. Call the script from the ios_app_bundle target when ios_enable_code_signing is true (default when targetting a device build) and remove some unused variables (the code signing identity is generally set for the whole build). BUG=600491 ========== to ========== [iOS/GN] Add code signing rules to ios_app_bundle target. Add a new script build/config/ios/codesign.py that copy the application binary into the application bundle and code sign it. Call the script from the ios_app_bundle target when ios_enable_code_signing is true (default when targetting a device build) and remove some unused variables (the code signing identity is generally set for the whole build). BUG=600491 Committed: https://crrev.com/eecfedb63ad99ea127fd51bebf2c1b1e2e7785a0 Cr-Commit-Position: refs/heads/master@{#400229} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/eecfedb63ad99ea127fd51bebf2c1b1e2e7785a0 Cr-Commit-Position: refs/heads/master@{#400229} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
