Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2478663002:
Add build flag to enable bitcode on ios (Closed)
|
Created:
4 years, 1 month ago by kthelgason Modified:
4 years, 1 month ago CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com, qiang.lu, niklas.enbom, yujie_mao (webrtc), peah-webrtc Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionAdd build flag to enable bitcode on ios
BUG=webrtc:5085
NOTRY=true
Committed: https://crrev.com/592baaf89aaf79245a55bfd3dbf47371b486cd03
Cr-Commit-Position: refs/heads/master@{#14997}
Patch Set 1 #
Total comments: 4
Patch Set 2 : Add clarifying comment #
Messages
Total messages: 19 (6 generated)
kthelgason@webrtc.org changed reviewers: + kjellander@webrtc.org, magjed@webrtc.org
I know this is not very pretty, but I found no better way to do this. I think we would eventually have been forced to introduce our own build config files, and not rely only on chromiums.
lgtm, I'm fine with this solution.
But kjellander@ knows this better and is a better reviewer for this of course.
https://codereview.webrtc.org/2478663002/diff/1/webrtc/build/ios/BUILD.gn File webrtc/build/ios/BUILD.gn (right): https://codereview.webrtc.org/2478663002/diff/1/webrtc/build/ios/BUILD.gn#new... webrtc/build/ios/BUILD.gn:9: # this file is only meant to be included on iOS. Start comment with capital letter. https://codereview.webrtc.org/2478663002/diff/1/webrtc/build/ios/BUILD.gn#new... webrtc/build/ios/BUILD.gn:13: rtc_ios_enable_bitcode = false Please add a line documenting what this does and why one would want to enable it.
https://codereview.webrtc.org/2478663002/diff/1/webrtc/build/ios/BUILD.gn File webrtc/build/ios/BUILD.gn (right): https://codereview.webrtc.org/2478663002/diff/1/webrtc/build/ios/BUILD.gn#new... webrtc/build/ios/BUILD.gn:9: # this file is only meant to be included on iOS. On 2016/11/07 11:17:35, kjellander_webrtc wrote: > Start comment with capital letter. Done. https://codereview.webrtc.org/2478663002/diff/1/webrtc/build/ios/BUILD.gn#new... webrtc/build/ios/BUILD.gn:13: rtc_ios_enable_bitcode = false On 2016/11/07 11:17:35, kjellander_webrtc wrote: > Please add a line documenting what this does and why one would want to enable > it. Done.
lgtm
On 2016/11/09 11:14:29, kjellander_webrtc wrote: > lgtm I suggest run a few bots and land with notry
Description was changed from ========== Add build flag to enable bitcode on ios BUG=webrtc:5085 ========== to ========== Add build flag to enable bitcode on ios BUG=webrtc:5085 NOTRY=true ==========
The CQ bit was checked by kthelgason@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org Link to the patchset: https://codereview.webrtc.org/2478663002/#ps20001 (title: "Add clarifying comment")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Message was sent while issue was closed.
Description was changed from ========== Add build flag to enable bitcode on ios BUG=webrtc:5085 NOTRY=true ========== to ========== Add build flag to enable bitcode on ios BUG=webrtc:5085 NOTRY=true ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Add build flag to enable bitcode on ios BUG=webrtc:5085 NOTRY=true ========== to ========== Add build flag to enable bitcode on ios BUG=webrtc:5085 NOTRY=true Committed: https://crrev.com/592baaf89aaf79245a55bfd3dbf47371b486cd03 Cr-Commit-Position: refs/heads/master@{#14997} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/592baaf89aaf79245a55bfd3dbf47371b486cd03 Cr-Commit-Position: refs/heads/master@{#14997}
Message was sent while issue was closed.
On 2016/11/09 12:58:23, commit-bot: I haz the power wrote: > Patchset 2 (id:??) landed as > https://crrev.com/592baaf89aaf79245a55bfd3dbf47371b486cd03 > Cr-Commit-Position: refs/heads/master@{#14997} Can you check that this doesn't break the framework builder?
Message was sent while issue was closed.
On 2016/11/09 19:18:15, tkchin_webrtc wrote: > On 2016/11/09 12:58:23, commit-bot: I haz the power wrote: > > Patchset 2 (id:??) landed as > > https://crrev.com/592baaf89aaf79245a55bfd3dbf47371b486cd03 > > Cr-Commit-Position: refs/heads/master@{#14997} > > Can you check that this doesn't break the framework builder? It didn't (that builder is now enabled - although it would have been better to run the trybot to be safe)
Message was sent while issue was closed.
On 2016/11/10 06:03:55, kjellander_webrtc wrote: > On 2016/11/09 19:18:15, tkchin_webrtc wrote: > > On 2016/11/09 12:58:23, commit-bot: I haz the power wrote: > > > Patchset 2 (id:??) landed as > > > https://crrev.com/592baaf89aaf79245a55bfd3dbf47371b486cd03 > > > Cr-Commit-Position: refs/heads/master@{#14997} > > > > Can you check that this doesn't break the framework builder? > > It didn't (that builder is now enabled - although it would have been better to > run the trybot to be safe) I'll setup a separate bot that builds with this flag enabled, to ensure we don't regress on it. Do we need to build the iOS API framework with the flag enabled as well or is a regular (let's say ARM64 Debug) build enough? |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
