Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2052333002:
[iOS/GN] Convert .strings file to binary1 plist format. (Closed)
Description[iOS/GN] Convert .strings file to binary1 plist format.
The OS expects the localized .strings file to be in binary1 plist format
on iOS devices, so convert the files when copying the to the application
bundle (use CoreFoundation module to do the conversion instead of plutil
as the module is already loaded to validate the file format).
Rename functions in copy_bundle_data.py to follow the python style guide.
Fixes 3D touch shortcuts on Chrome on iOS when building with GN.
BUG=618398
Committed: https://crrev.com/2063e083d9fed1165fe30a5e791de4fc7b960090
Cr-Commit-Position: refs/heads/master@{#400221}
Patch Set 1 #
Total comments: 7
Patch Set 2 : Address comments #
Messages
Total messages: 25 (12 generated)
Description was changed from ========== WIP: Code signing on iOS... BUG= ========== to ========== [iOS/GN] Convert .strings file to binary1 plist format. The OS expects the localized .strings file to be in binary1 plist format on iOS devices, so convert the files when copying the to the application bundle (use CoreFoundation module to do the conversion instead of plutil as the module is already loaded to validate the file format). Rename functions in copy_bundle_data.py to follow the python style guide. Fixes 3D touch shortcuts on Chrome on iOS when building with GN. BUG=618398 ==========
Patchset #1 (id:1) has been deleted
Description was changed from ========== [iOS/GN] Convert .strings file to binary1 plist format. The OS expects the localized .strings file to be in binary1 plist format on iOS devices, so convert the files when copying the to the application bundle (use CoreFoundation module to do the conversion instead of plutil as the module is already loaded to validate the file format). Rename functions in copy_bundle_data.py to follow the python style guide. Fixes 3D touch shortcuts on Chrome on iOS when building with GN. BUG=618398 ========== to ========== [iOS/GN] Convert .strings file to binary1 plist format. The OS expects the localized .strings file to be in binary1 plist format on iOS devices, so convert the files when copying the to the application bundle (use CoreFoundation module to do the conversion instead of plutil as the module is already loaded to validate the file format). Rename functions in copy_bundle_data.py to follow the python style guide. Fixes 3D touch shortcuts on Chrome on iOS when building with GN. BUG=618398 ==========
sdefresne@chromium.org changed reviewers: + rsesek@chromium.org
Robert: Please take a look.
https://codereview.chromium.org/2052333002/diff/20001/build/toolchain/mac/BUI... File build/toolchain/mac/BUILD.gn (right): https://codereview.chromium.org/2052333002/diff/20001/build/toolchain/mac/BUI... build/toolchain/mac/BUILD.gn:286: command = "python $_tool {{source}} {{output}} ${_extra_args}" Put this before {{source}} ? https://codereview.chromium.org/2052333002/diff/20001/build/toolchain/mac/cop... File build/toolchain/mac/copy_bundle_data.py (right): https://codereview.chromium.org/2052333002/diff/20001/build/toolchain/mac/cop... build/toolchain/mac/copy_bundle_data.py:49: import CoreFoundation "as CF" could shorten up some of your callsites. https://codereview.chromium.org/2052333002/diff/20001/build/toolchain/mac/cop... build/toolchain/mac/copy_bundle_data.py:71: data = CoreFoundation.CFDataGetBytes( GetBytes will do a copy. Is it possible to use GetBytePtr in Python?
PTAL https://codereview.chromium.org/2052333002/diff/20001/build/toolchain/mac/BUI... File build/toolchain/mac/BUILD.gn (right): https://codereview.chromium.org/2052333002/diff/20001/build/toolchain/mac/BUI... build/toolchain/mac/BUILD.gn:286: command = "python $_tool {{source}} {{output}} ${_extra_args}" On 2016/06/13 16:50:57, Robert Sesek wrote: > Put this before {{source}} ? Done. https://codereview.chromium.org/2052333002/diff/20001/build/toolchain/mac/cop... File build/toolchain/mac/copy_bundle_data.py (right): https://codereview.chromium.org/2052333002/diff/20001/build/toolchain/mac/cop... build/toolchain/mac/copy_bundle_data.py:49: import CoreFoundation On 2016/06/13 16:50:57, Robert Sesek wrote: > "as CF" could shorten up some of your callsites. Done. https://codereview.chromium.org/2052333002/diff/20001/build/toolchain/mac/cop... build/toolchain/mac/copy_bundle_data.py:71: data = CoreFoundation.CFDataGetBytes( On 2016/06/13 16:50:57, Robert Sesek wrote: > GetBytes will do a copy. Is it possible to use GetBytePtr in Python? I tried, but when I pass the value to dest_file.write() I get the following error: Traceback (most recent call last): File "../../build/toolchain/mac/copy_bundle_data.py", line 117, in <module> sys.exit(Main()) File "../../build/toolchain/mac/copy_bundle_data.py", line 114, in Main CopyFile(args.source, args.dest, args.strings_format) File "../../build/toolchain/mac/copy_bundle_data.py", line 98, in CopyFile CopyStringsFile(source, dest, strings_format) File "../../build/toolchain/mac/copy_bundle_data.py", line 73, in CopyStringsFile dest_file.write(data) TypeError: must be string or buffer, not objc.varlist The obc.varlist is a wrapper around a uint8_t*, so I would have to do the following: data = CoreFoundation.CFDataGetBytePtr(cfdata) with open(dest, 'wb') as dest_file: for i in xrange(CoreFoundation.CFDataGetLength(cfdata)): dest_file.write(data[i]) I think that looping and appending byte per byte in python is going to be slower than slurping the value in a python array in the C module and writing it to a file in one go. The .strings file are generally relatively small (a few Mb at most) and keeping a couple of copies in memory would not be too problematic. Invoking "plutil" to do the conversion would also involve having the data multiple time in memory (though in different processes) and will also require spawning a new process (which is a bit slow), so probably not better than the current code. WDYT?
LGTM https://codereview.chromium.org/2052333002/diff/20001/build/toolchain/mac/cop... File build/toolchain/mac/copy_bundle_data.py (right): https://codereview.chromium.org/2052333002/diff/20001/build/toolchain/mac/cop... build/toolchain/mac/copy_bundle_data.py:71: data = CoreFoundation.CFDataGetBytes( On 2016/06/14 07:27:40, sdefresne wrote: > WDYT? Eesh, what you have here is fine, then.
The CQ bit was checked by sdefresne@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2052333002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
sdefresne@chromium.org changed reviewers: + dpranke@chromium.org
Dirk, can you review as OWNERS of //build/toolchain?
sdefresne@chromium.org changed reviewers: + brettw@chromium.org - dpranke@chromium.org
-dpranke, +brettw: I need OWNERS approval, thank. Can you CQ if LGTY?
dpranke@chromium.org changed reviewers: + dpranke@chromium.org
The CQ bit was checked by dpranke@chromium.org
lgtm whoops, thought I had approved this. LGTM.
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2052333002/40001
Message was sent while issue was closed.
Description was changed from ========== [iOS/GN] Convert .strings file to binary1 plist format. The OS expects the localized .strings file to be in binary1 plist format on iOS devices, so convert the files when copying the to the application bundle (use CoreFoundation module to do the conversion instead of plutil as the module is already loaded to validate the file format). Rename functions in copy_bundle_data.py to follow the python style guide. Fixes 3D touch shortcuts on Chrome on iOS when building with GN. BUG=618398 ========== to ========== [iOS/GN] Convert .strings file to binary1 plist format. The OS expects the localized .strings file to be in binary1 plist format on iOS devices, so convert the files when copying the to the application bundle (use CoreFoundation module to do the conversion instead of plutil as the module is already loaded to validate the file format). Rename functions in copy_bundle_data.py to follow the python style guide. Fixes 3D touch shortcuts on Chrome on iOS when building with GN. BUG=618398 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001)
Message was sent while issue was closed.
CQ bit was unchecked
Message was sent while issue was closed.
Description was changed from ========== [iOS/GN] Convert .strings file to binary1 plist format. The OS expects the localized .strings file to be in binary1 plist format on iOS devices, so convert the files when copying the to the application bundle (use CoreFoundation module to do the conversion instead of plutil as the module is already loaded to validate the file format). Rename functions in copy_bundle_data.py to follow the python style guide. Fixes 3D touch shortcuts on Chrome on iOS when building with GN. BUG=618398 ========== to ========== [iOS/GN] Convert .strings file to binary1 plist format. The OS expects the localized .strings file to be in binary1 plist format on iOS devices, so convert the files when copying the to the application bundle (use CoreFoundation module to do the conversion instead of plutil as the module is already loaded to validate the file format). Rename functions in copy_bundle_data.py to follow the python style guide. Fixes 3D touch shortcuts on Chrome on iOS when building with GN. BUG=618398 Committed: https://crrev.com/2063e083d9fed1165fe30a5e791de4fc7b960090 Cr-Commit-Position: refs/heads/master@{#400221} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/2063e083d9fed1165fe30a5e791de4fc7b960090 Cr-Commit-Position: refs/heads/master@{#400221} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
