|
|
Created:
3 years, 10 months ago by VladimirTechMan Modified:
3 years, 10 months ago CC:
webrtc-reviews_webrtc.org Target Ref:
refs/heads/master Project:
webrtc Visibility:
Public. |
DescriptionDo not produce dSYM file for the iOS Frameworks with bitcode
Though dSYM files can be generated when building applications or libraries
with bitcode. They cannot be used to symbolicate crash reports from
applications. Instead, developers need to grab the real dSYM files, which
are generated for each specific device type after uploading an iOS / tvOS
application to App Store (or to a device using Xcode). Apple clearly warns
about it in its documentation:
https://developer.apple.com/library/content/technotes/tn2151/_index.html#//apple_ref/doc/uid/DTS40008184-CH1-SYMBOLICATION-BITCODE
With that in mind, I believe that it would be better to not confuse
developers by giving them dSYM files that are not very helpful with
the bitcode-enabled framework. Thus, proposing the following modification
to the building script, to generate dSYM by default only without
the bitcode option. However, if some developers still want to get
the dSYM files as a build-process artifact, when enabling bitcode,
they can explicitly add --extra-gn-args enable_dsyms=true to the script.
Let me know if it lgty.
NOTRY=True
BUG=None
Review-Url: https://codereview.webrtc.org/2705163007
Cr-Commit-Position: refs/heads/master@{#16836}
Committed: https://chromium.googlesource.com/external/webrtc/+/d74517c52a6cd4172b1f3fdc4e624b6145ff5a0f
Patch Set 1 #
Total comments: 1
Patch Set 2 : Do not produce dSYM file for the iOS Frameworks with bitcode #Messages
Total messages: 19 (12 generated)
Description was changed from ========== Do not produce dSYM file for the iOS Frameworks with bitcode Though dSYM files can be generated when building applications or libraries with bitcode. They cannot be used to symbolicate crash reports from applications. Instead, developers need to grab the real dSYM files, which are generated for each specific device type after uploading an iOS / tvOS application to App Store (or to a device using Xcode). Apple clearly warns about it in its documentation (see the "Bitcode" section at the link below): https://developer.apple.com/library/content/technotes/tn2151/_index.html With that in mind, I believe that it would be better to not confuse developers by giving them dSYM files that are not very helpful with the bitcode-enabled framework. Thus, I propose the following modification to the building script, to enable dSYM generation by default only without the bitcode option. However, the change still allows people who want to get the dSYM files as a build-process artefact, after enabling bitcode, to do that, by adding --extra-gn-args enable_dsyms=true to the command line. Let me know if it lgty. NOTRY=True BUG=None ========== to ========== Do not produce dSYM file for the iOS Frameworks with bitcode Though dSYM files can be generated when building applications or libraries with bitcode. They cannot be used to symbolicate crash reports from applications. Instead, developers need to grab the real dSYM files, which are generated for each specific device type after uploading an iOS / tvOS application to App Store (or to a device using Xcode). Apple clearly warns about it in its documentation: https://developer.apple.com/library/content/technotes/tn2151/_index.html#//ap... With that in mind, I believe that it would be better to not confuse developers by giving them dSYM files that are not very helpful with the bitcode-enabled framework. Thus, I propose the following modification to the building script, to enable dSYM generation by default only without the bitcode option. However, the change still allows people who want to get the dSYM files as a build-process artefact, after enabling bitcode, to do that, by adding --extra-gn-args enable_dsyms=true to the command line. Let me know if it lgty. NOTRY=True BUG=None ==========
vladimirtechman@gmail.com changed reviewers: + kjellander@webrtc.org
Description was changed from ========== Do not produce dSYM file for the iOS Frameworks with bitcode Though dSYM files can be generated when building applications or libraries with bitcode. They cannot be used to symbolicate crash reports from applications. Instead, developers need to grab the real dSYM files, which are generated for each specific device type after uploading an iOS / tvOS application to App Store (or to a device using Xcode). Apple clearly warns about it in its documentation: https://developer.apple.com/library/content/technotes/tn2151/_index.html#//ap... With that in mind, I believe that it would be better to not confuse developers by giving them dSYM files that are not very helpful with the bitcode-enabled framework. Thus, I propose the following modification to the building script, to enable dSYM generation by default only without the bitcode option. However, the change still allows people who want to get the dSYM files as a build-process artefact, after enabling bitcode, to do that, by adding --extra-gn-args enable_dsyms=true to the command line. Let me know if it lgty. NOTRY=True BUG=None ========== to ========== Do not produce dSYM file for the iOS Frameworks with bitcode Though dSYM files can be generated when building applications or libraries with bitcode. They cannot be used to symbolicate crash reports from applications. Instead, developers need to grab the real dSYM files, which are generated for each specific device type after uploading an iOS / tvOS application to App Store (or to a device using Xcode). Apple clearly warns about it in its documentation: https://developer.apple.com/library/content/technotes/tn2151/_index.html#//ap... With that in mind, I believe that it would be better to not confuse developers by giving them dSYM files that are not very helpful with the bitcode-enabled framework. Thus, proposing the following modification to the building script, to generate dSYM by default only without the bitcode option. However, the change still allows people who want to get the dSYM files as a build-process artefact, after enabling bitcode, to do that, by adding --extra-gn-args enable_dsyms=true to the command line. Let me know if it lgty. NOTRY=True BUG=None ==========
Description was changed from ========== Do not produce dSYM file for the iOS Frameworks with bitcode Though dSYM files can be generated when building applications or libraries with bitcode. They cannot be used to symbolicate crash reports from applications. Instead, developers need to grab the real dSYM files, which are generated for each specific device type after uploading an iOS / tvOS application to App Store (or to a device using Xcode). Apple clearly warns about it in its documentation: https://developer.apple.com/library/content/technotes/tn2151/_index.html#//ap... With that in mind, I believe that it would be better to not confuse developers by giving them dSYM files that are not very helpful with the bitcode-enabled framework. Thus, proposing the following modification to the building script, to generate dSYM by default only without the bitcode option. However, the change still allows people who want to get the dSYM files as a build-process artefact, after enabling bitcode, to do that, by adding --extra-gn-args enable_dsyms=true to the command line. Let me know if it lgty. NOTRY=True BUG=None ========== to ========== Do not produce dSYM file for the iOS Frameworks with bitcode Though dSYM files can be generated when building applications or libraries with bitcode. They cannot be used to symbolicate crash reports from applications. Instead, developers need to grab the real dSYM files, which are generated for each specific device type after uploading an iOS / tvOS application to App Store (or to a device using Xcode). Apple clearly warns about it in its documentation: https://developer.apple.com/library/content/technotes/tn2151/_index.html#//ap... With that in mind, I believe that it would be better to not confuse developers by giving them dSYM files that are not very helpful with the bitcode-enabled framework. Thus, proposing the following modification to the building script, to generate dSYM by default only without the bitcode option. However, the change still allows to get the dSYM files as a build-process artifact, when enabling bitcode, by explicitly adding --extra-gn-args enable_dsyms=true to the command line. Let me know if it lgty. NOTRY=True BUG=None ==========
Description was changed from ========== Do not produce dSYM file for the iOS Frameworks with bitcode Though dSYM files can be generated when building applications or libraries with bitcode. They cannot be used to symbolicate crash reports from applications. Instead, developers need to grab the real dSYM files, which are generated for each specific device type after uploading an iOS / tvOS application to App Store (or to a device using Xcode). Apple clearly warns about it in its documentation: https://developer.apple.com/library/content/technotes/tn2151/_index.html#//ap... With that in mind, I believe that it would be better to not confuse developers by giving them dSYM files that are not very helpful with the bitcode-enabled framework. Thus, proposing the following modification to the building script, to generate dSYM by default only without the bitcode option. However, the change still allows to get the dSYM files as a build-process artifact, when enabling bitcode, by explicitly adding --extra-gn-args enable_dsyms=true to the command line. Let me know if it lgty. NOTRY=True BUG=None ========== to ========== Do not produce dSYM file for the iOS Frameworks with bitcode Though dSYM files can be generated when building applications or libraries with bitcode. They cannot be used to symbolicate crash reports from applications. Instead, developers need to grab the real dSYM files, which are generated for each specific device type after uploading an iOS / tvOS application to App Store (or to a device using Xcode). Apple clearly warns about it in its documentation: https://developer.apple.com/library/content/technotes/tn2151/_index.html#//ap... With that in mind, I believe that it would be better to not confuse developers by giving them dSYM files that are not very helpful with the bitcode-enabled framework. Thus, proposing the following modification to the building script, to generate dSYM by default only without the bitcode option. However, those who still want to get the dSYM files as a build-process artifact, when enabling bitcode, can explicitly add --extra-gn-args enable_dsyms=true to the command line. Let me know if it lgty. NOTRY=True BUG=None ==========
Description was changed from ========== Do not produce dSYM file for the iOS Frameworks with bitcode Though dSYM files can be generated when building applications or libraries with bitcode. They cannot be used to symbolicate crash reports from applications. Instead, developers need to grab the real dSYM files, which are generated for each specific device type after uploading an iOS / tvOS application to App Store (or to a device using Xcode). Apple clearly warns about it in its documentation: https://developer.apple.com/library/content/technotes/tn2151/_index.html#//ap... With that in mind, I believe that it would be better to not confuse developers by giving them dSYM files that are not very helpful with the bitcode-enabled framework. Thus, proposing the following modification to the building script, to generate dSYM by default only without the bitcode option. However, those who still want to get the dSYM files as a build-process artifact, when enabling bitcode, can explicitly add --extra-gn-args enable_dsyms=true to the command line. Let me know if it lgty. NOTRY=True BUG=None ========== to ========== Do not produce dSYM file for the iOS Frameworks with bitcode Though dSYM files can be generated when building applications or libraries with bitcode. They cannot be used to symbolicate crash reports from applications. Instead, developers need to grab the real dSYM files, which are generated for each specific device type after uploading an iOS / tvOS application to App Store (or to a device using Xcode). Apple clearly warns about it in its documentation: https://developer.apple.com/library/content/technotes/tn2151/_index.html#//ap... With that in mind, I believe that it would be better to not confuse developers by giving them dSYM files that are not very helpful with the bitcode-enabled framework. Thus, proposing the following modification to the building script, to generate dSYM by default only without the bitcode option. However, if some developers still want to get the dSYM files as a build-process artifact, when enabling bitcode, can explicitly add --extra-gn-args enable_dsyms=true to the command line. Let me know if it lgty. NOTRY=True BUG=None ==========
Description was changed from ========== Do not produce dSYM file for the iOS Frameworks with bitcode Though dSYM files can be generated when building applications or libraries with bitcode. They cannot be used to symbolicate crash reports from applications. Instead, developers need to grab the real dSYM files, which are generated for each specific device type after uploading an iOS / tvOS application to App Store (or to a device using Xcode). Apple clearly warns about it in its documentation: https://developer.apple.com/library/content/technotes/tn2151/_index.html#//ap... With that in mind, I believe that it would be better to not confuse developers by giving them dSYM files that are not very helpful with the bitcode-enabled framework. Thus, proposing the following modification to the building script, to generate dSYM by default only without the bitcode option. However, if some developers still want to get the dSYM files as a build-process artifact, when enabling bitcode, can explicitly add --extra-gn-args enable_dsyms=true to the command line. Let me know if it lgty. NOTRY=True BUG=None ========== to ========== Do not produce dSYM file for the iOS Frameworks with bitcode Though dSYM files can be generated when building applications or libraries with bitcode. They cannot be used to symbolicate crash reports from applications. Instead, developers need to grab the real dSYM files, which are generated for each specific device type after uploading an iOS / tvOS application to App Store (or to a device using Xcode). Apple clearly warns about it in its documentation: https://developer.apple.com/library/content/technotes/tn2151/_index.html#//ap... With that in mind, I believe that it would be better to not confuse developers by giving them dSYM files that are not very helpful with the bitcode-enabled framework. Thus, proposing the following modification to the building script, to generate dSYM by default only without the bitcode option. However, if some developers still want to get the dSYM files as a build-process artifact, when enabling bitcode, they can explicitly add --extra-gn-args enable_dsyms=true to the script. Let me know if it lgty. NOTRY=True BUG=None ==========
kjellander@webrtc.org changed reviewers: + kthelgason@webrtc.org
lgtm with comment fixed, but I want Kári's approval for this as well https://codereview.webrtc.org/2705163007/diff/1/tools-webrtc/ios/build_ios_li... File tools-webrtc/ios/build_ios_libs.py (right): https://codereview.webrtc.org/2705163007/diff/1/tools-webrtc/ios/build_ios_li... tools-webrtc/ios/build_ios_libs.py:192: if os.path.isdir(os.path.join(lib_paths[0], 'WebRTC.dSYM')): Please introduce a variable to contain os.path.join(lib_paths[0], 'WebRTC.dSYM') instead of repeating this path concatenation. Thanks for removing the other duplicated code though.
On 2017/02/24 12:40:29, kjellander_webrtc wrote: > lgtm with comment fixed, but I want Kári's approval for this as well > ... Thanks, Henrik. Yes, I should've introduced that extra variable, agree. Done now. (I also did a few more minor corrections to that script part, just to make its logic easier for future readers. Let me know if any concerns on that part.) Let's now wait for Kári's response to us, on the change in dealing with dSYM files, as per my description.
This lgtm, thanks Vladimir!
The CQ bit was checked by kthelgason@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from kjellander@webrtc.org Link to the patchset: https://codereview.webrtc.org/2705163007/#ps20001 (title: "Do not produce dSYM file for the iOS Frameworks with bitcode")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch. Bot data: {"patchset_id": 20001, "attempt_start_ts": 1488017170537950, "parent_rev": "633f6fe0046131ed815098298b9a3120bac1d7a0", "commit_rev": "d74517c52a6cd4172b1f3fdc4e624b6145ff5a0f"}
Message was sent while issue was closed.
Description was changed from ========== Do not produce dSYM file for the iOS Frameworks with bitcode Though dSYM files can be generated when building applications or libraries with bitcode. They cannot be used to symbolicate crash reports from applications. Instead, developers need to grab the real dSYM files, which are generated for each specific device type after uploading an iOS / tvOS application to App Store (or to a device using Xcode). Apple clearly warns about it in its documentation: https://developer.apple.com/library/content/technotes/tn2151/_index.html#//ap... With that in mind, I believe that it would be better to not confuse developers by giving them dSYM files that are not very helpful with the bitcode-enabled framework. Thus, proposing the following modification to the building script, to generate dSYM by default only without the bitcode option. However, if some developers still want to get the dSYM files as a build-process artifact, when enabling bitcode, they can explicitly add --extra-gn-args enable_dsyms=true to the script. Let me know if it lgty. NOTRY=True BUG=None ========== to ========== Do not produce dSYM file for the iOS Frameworks with bitcode Though dSYM files can be generated when building applications or libraries with bitcode. They cannot be used to symbolicate crash reports from applications. Instead, developers need to grab the real dSYM files, which are generated for each specific device type after uploading an iOS / tvOS application to App Store (or to a device using Xcode). Apple clearly warns about it in its documentation: https://developer.apple.com/library/content/technotes/tn2151/_index.html#//ap... With that in mind, I believe that it would be better to not confuse developers by giving them dSYM files that are not very helpful with the bitcode-enabled framework. Thus, proposing the following modification to the building script, to generate dSYM by default only without the bitcode option. However, if some developers still want to get the dSYM files as a build-process artifact, when enabling bitcode, they can explicitly add --extra-gn-args enable_dsyms=true to the script. Let me know if it lgty. NOTRY=True BUG=None Review-Url: https://codereview.webrtc.org/2705163007 Cr-Commit-Position: refs/heads/master@{#16836} Committed: https://chromium.googlesource.com/external/webrtc/+/d74517c52a6cd4172b1f3fdc4... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/external/webrtc/+/d74517c52a6cd4172b1f3fdc4...
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.webrtc.org/2719773002/ by tommi@webrtc.org. The reason for reverting is: Looks like this caused the iOS API Framework Builder to fail. https://build.chromium.org/p/client.webrtc/builders/iOS%20API%20Framework%20B... Zipping /b/rr/tmpkIyP1e/w/webrtc_ios_api_framework.zip... Traceback (most recent call last): File "/b/rr/tmpkIyP1e/rw/checkout/scripts/slave/recipe_modules/zip/resources/zip.py", line 144, in <module> sys.exit(main()) File "/b/rr/tmpkIyP1e/rw/checkout/scripts/slave/recipe_modules/zip/resources/zip.py", line 130, in main exit_code = zip_with_subprocess(root, output, entries) File "/b/rr/tmpkIyP1e/rw/checkout/scripts/slave/recipe_modules/zip/resources/zip.py", line 43, in zip_with_subprocess assert os.path.isdir(path), path AssertionError: /b/c/b/iOS_API_Framework_Builder/src/out_ios_libs/WebRTC.dSYM/ step returned non-zero exit code: 1 @@@STEP_FAILURE@@@. |