Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
912633003:
libjingle build fixes to enable_webrtc in the Linux GN build. (Closed)
|
Created:
5 years, 10 months ago by Dirk Pranke Modified:
5 years, 10 months ago CC:
chromium-reviews, darin-cc_chromium.org, jam Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptionlibjingle build fixes to enable_webrtc in the Linux GN build.
R=kjellander@chromium.org, brettw@chromium.org
BUG=456815
CQ_EXTRA_TRYBOTS=tryserver.chromium.linux:android_chromium_gn_compile_dbg,android_chromium_gn_compile_rel;tryserver.chromium.win:win8_chromium_gn_rel,win8_chromium_gn_dbg;tryserver.chromium.mac:mac_chromium_gn_rel,mac_chromium_gn_dbg
Committed: https://crrev.com/c989ea61869c438b2569f1722c3c05cfe72e6f46
Cr-Commit-Position: refs/heads/master@{#317929}
Patch Set 1 #
Total comments: 4
Patch Set 2 : remove stray outdated comment #Patch Set 3 : fix one more target for libjingle #Patch Set 4 : add missing import for android_library #Patch Set 5 : fix import #Patch Set 6 : fix path to aura unittest #Patch Set 7 : disable webrtc on mac #Patch Set 8 : disable webrtc on android for now #Patch Set 9 : gn format #
Messages
Total messages: 41 (19 generated)
This, along with https://webrtc-codereview.appspot.com/36039004/, looks to be enough to get WebRTC linking w/ Chromium GN. I have no idea if these are really the right fixes :). Please take a look?
On 2015/02/10 02:18:43, Dirk Pranke wrote: > This, along with https://webrtc-codereview.appspot.com/36039004/, looks to be > enough > to get WebRTC linking w/ Chromium GN. > > I have no idea if these are really the right fixes :). > > Please take a look? Oh, obviously the WebRTC fix needs to land first and be rolled into chromium. Also, I have no idea if this builds on anything other than Linux; I will need to test that.
https://codereview.chromium.org/912633003/diff/1/build/config/features.gni File build/config/features.gni (right): https://codereview.chromium.org/912633003/diff/1/build/config/features.gni#ne... build/config/features.gni:38: enable_webrtc = !is_ios Shouldn't you remove the commented lines above if you're changing to this? https://codereview.chromium.org/912633003/diff/1/third_party/libjingle/BUILD.gn File third_party/libjingle/BUILD.gn (right): https://codereview.chromium.org/912633003/diff/1/third_party/libjingle/BUILD.... third_party/libjingle/BUILD.gn:1: # Copyright 2014 The Chromium Authors. All rights reserved. We really should create BUILD.gn files in the WebRTC repo instead of having this one living in Chromium. I filed https://code.google.com/p/webrtc/issues/detail?id=4160 to track that, but I haven't been able to prioritize it.
https://codereview.chromium.org/912633003/diff/1/build/config/features.gni File build/config/features.gni (right): https://codereview.chromium.org/912633003/diff/1/build/config/features.gni#ne... build/config/features.gni:38: enable_webrtc = !is_ios On 2015/02/10 12:01:49, kjellander wrote: > Shouldn't you remove the commented lines above if you're changing to this? Whoops. Yes :). https://codereview.chromium.org/912633003/diff/1/third_party/libjingle/BUILD.gn File third_party/libjingle/BUILD.gn (right): https://codereview.chromium.org/912633003/diff/1/third_party/libjingle/BUILD.... third_party/libjingle/BUILD.gn:1: # Copyright 2014 The Chromium Authors. All rights reserved. I assume you're fine w/ this landing in the meantime?
lgtm
On 2015/02/10 17:36:06, Dirk Pranke wrote: > https://codereview.chromium.org/912633003/diff/1/build/config/features.gni > File build/config/features.gni (right): > > https://codereview.chromium.org/912633003/diff/1/build/config/features.gni#ne... > build/config/features.gni:38: enable_webrtc = !is_ios > On 2015/02/10 12:01:49, kjellander wrote: > > Shouldn't you remove the commented lines above if you're changing to this? > > Whoops. Yes :). > > https://codereview.chromium.org/912633003/diff/1/third_party/libjingle/BUILD.gn > File third_party/libjingle/BUILD.gn (right): > > https://codereview.chromium.org/912633003/diff/1/third_party/libjingle/BUILD.... > third_party/libjingle/BUILD.gn:1: # Copyright 2014 The Chromium Authors. All > rights reserved. > > I assume you're fine w/ this landing in the meantime? If this builds I'm fine with this landing. We have a roll ongoing right now (https://codereview.chromium.org/914163002/) that will roll in the WebRTC revision for the video render/capture fix: https://code.google.com/p/webrtc/source/detail?r=8323
On 2015/02/11 15:14:48, kjellander wrote: > On 2015/02/10 17:36:06, Dirk Pranke wrote: > > https://codereview.chromium.org/912633003/diff/1/build/config/features.gni > > File build/config/features.gni (right): > > > > > https://codereview.chromium.org/912633003/diff/1/build/config/features.gni#ne... > > build/config/features.gni:38: enable_webrtc = !is_ios > > On 2015/02/10 12:01:49, kjellander wrote: > > > Shouldn't you remove the commented lines above if you're changing to this? > > > > Whoops. Yes :). > > > > > https://codereview.chromium.org/912633003/diff/1/third_party/libjingle/BUILD.gn > > File third_party/libjingle/BUILD.gn (right): > > > > > https://codereview.chromium.org/912633003/diff/1/third_party/libjingle/BUILD.... > > third_party/libjingle/BUILD.gn:1: # Copyright 2014 The Chromium Authors. All > > rights reserved. > > > > I assume you're fine w/ this landing in the meantime? > > If this builds I'm fine with this landing. We have a roll ongoing right now > (https://codereview.chromium.org/914163002/) that will roll in the WebRTC > revision for the video render/capture fix: > https://code.google.com/p/webrtc/source/detail?r=8323 Forgot the lgtm
The CQ bit was checked by dpranke@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org, kjellander@chromium.org Link to the patchset: https://codereview.chromium.org/912633003/#ps40001 (title: "fix one more target for libjingle")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/912633003/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_chromium_gn_compile_dbg on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
The CQ bit was checked by dpranke@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from kjellander@chromium.org, brettw@chromium.org Link to the patchset: https://codereview.chromium.org/912633003/#ps60001 (title: "add missing import for android_library")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/912633003/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
The CQ bit was checked by dpranke@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from kjellander@chromium.org, brettw@chromium.org Link to the patchset: https://codereview.chromium.org/912633003/#ps80001 (title: "fix import")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/912633003/80001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by dpranke@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from kjellander@chromium.org, brettw@chromium.org Link to the patchset: https://codereview.chromium.org/912633003/#ps100001 (title: "fix path to aura unittest")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/912633003/100001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_gn_dbg on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_d...)
The CQ bit was checked by dpranke@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from kjellander@chromium.org, brettw@chromium.org Link to the patchset: https://codereview.chromium.org/912633003/#ps120001 (title: "disable webrtc on mac")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/912633003/120001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_chromium_gn_compile_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
The CQ bit was checked by dpranke@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from kjellander@chromium.org, brettw@chromium.org Link to the patchset: https://codereview.chromium.org/912633003/#ps160001 (title: "gn format")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/912633003/160001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios_rel_device_ninja_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
The CQ bit was checked by dpranke@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/912633003/160001
Message was sent while issue was closed.
Committed patchset #9 (id:160001)
Message was sent while issue was closed.
Patchset 9 (id:??) landed as https://crrev.com/c989ea61869c438b2569f1722c3c05cfe72e6f46 Cr-Commit-Position: refs/heads/master@{#317929} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
