Chromium Code Reviews
|
|
Chromium Code Reviews|
Created:
4 years, 4 months ago by Nico Modified:
4 years, 4 months ago Reviewers:
kjellander_webrtc CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com, qiang.lu, niklas.enbom, yujie_mao (webrtc), peah-webrtc Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
Descriptiongn: Do not include "webrtc" in Chromium/iOS builds.
It's not part of the gyp build either, and `ninja -C out/gnios webrtc`
causes several build errors, at least in Chromium builds (which means
gn/ios bots building 'all' are broken).
BUG=chromium:633316
R=kjellander@webrtc.org
Committed: https://crrev.com/3ab32dc7752cbcdbfb13183486f7238b4a89363b
Cr-Commit-Position: refs/heads/master@{#13609}
Patch Set 1 #
Created: 4 years, 4 months ago
Messages
Total messages: 11 (5 generated)
thakis@chromium.org changed reviewers: + kjellander@chromium.org
Description was changed from ========== gn: Do not include "webrtc" in Chromium/iOS builds. It's not part of the gyp build either, and `ninja -C out/gnios webrtc` causes several build errors, at least in Chromium builds (which means gn/ios bots building 'all' are broken). BUG=chromium:633316 ========== to ========== gn: Do not include "webrtc" in Chromium/iOS builds. It's not part of the gyp build either, and `ninja -C out/gnios webrtc` causes several build errors, at least in Chromium builds (which means gn/ios bots building 'all' are broken). BUG=chromium:633316 ==========
kjellander@webrtc.org changed reviewers: + kjellander@webrtc.org - kjellander@chromium.org
I'm not sure about this, at least one place in GYP-land depends on this target: https://cs.chromium.org/chromium/src/remoting/remoting_client.gypi?rcl=0&l=202 so I can't tell for sure what happens if we remove it (probably nothing). I dislike the targets existence at all and my goal is to move all these "all"-like targets up to WebRTC's all.gyp/root BUILD.gn (that sits above the webrtc/ dir in our checkout, so not more than necessary is pulled in when depending on //third_party/webrtc. That's ongoing work but we have to focus on switching to GN first.
On 2016/08/02 07:56:37, kjellander_webrtc wrote: > I'm not sure about this, at least one place in GYP-land depends on this target: > https://cs.chromium.org/chromium/src/remoting/remoting_client.gypi?rcl=0&l=202 > so I can't tell for sure what happens if we remove it (probably nothing). > > I dislike the targets existence at all and my goal is to move all these > "all"-like targets up to WebRTC's all.gyp/root BUILD.gn (that sits above the > webrtc/ dir in our checkout, so not more than necessary is pulled in when > depending on //third_party/webrtc. That's ongoing work but we have to focus on > switching to GN first. Since https://bugs.chromium.org/p/chromium/issues/detail?id=633316 fails only on libjpeg_turbo, isn't it better to tackle just that problem for WebRTC on iOS as well, similar to how you did for libyuv?
On 2016/08/02 07:56:37, kjellander_webrtc wrote: > I'm not sure about this, at least one place in GYP-land depends on this target: > https://cs.chromium.org/chromium/src/remoting/remoting_client.gypi?rcl=0&l=202 > so I can't tell for sure what happens if we remove it (probably nothing). Remoting isn't part of the iOS build either. (Also, I did try building chrome with this patch locally before sending it.) > I dislike the targets existence at all and my goal is to move all these > "all"-like targets up to WebRTC's all.gyp/root BUILD.gn (that sits above the > webrtc/ dir in our checkout, so not more than necessary is pulled in when > depending on //third_party/webrtc. That's ongoing work but we have to focus on > switching to GN first. Cool, but my bots are actively red right now since they were switched to gn, so I need to do something today. > Since https://bugs.chromium.org/p/chromium/issues/detail?id=633316 fails only on > libjpeg_turbo I only put the first build failure on that bug. There are a bunch more (~20) when building webrtc. I'm treating that bug as "`ninja -C out/gnios` fails in gn builds".
On 2016/08/02 10:55:18, Nico wrote: > On 2016/08/02 07:56:37, kjellander_webrtc wrote: > > I'm not sure about this, at least one place in GYP-land depends on this > target: > > https://cs.chromium.org/chromium/src/remoting/remoting_client.gypi?rcl=0&l=202 > > so I can't tell for sure what happens if we remove it (probably nothing). > > Remoting isn't part of the iOS build either. (Also, I did try building chrome > with this patch locally before sending it.) Cool, let's go with this then. I see > > > I dislike the targets existence at all and my goal is to move all these > > "all"-like targets up to WebRTC's all.gyp/root BUILD.gn (that sits above the > > webrtc/ dir in our checkout, so not more than necessary is pulled in when > > depending on //third_party/webrtc. That's ongoing work but we have to focus on > > switching to GN first. > > Cool, but my bots are actively red right now since they were switched to gn, so > I need to do something today. > > > Since https://bugs.chromium.org/p/chromium/issues/detail?id=633316 fails only > on > > libjpeg_turbo > > I only put the first build failure on that bug. There are a bunch more (~20) > when building webrtc. I'm treating that bug as "`ninja -C out/gnios` fails in gn > builds". OK, then I'm fine with this. lgtm
Description was changed from ========== gn: Do not include "webrtc" in Chromium/iOS builds. It's not part of the gyp build either, and `ninja -C out/gnios webrtc` causes several build errors, at least in Chromium builds (which means gn/ios bots building 'all' are broken). BUG=chromium:633316 ========== to ========== gn: Do not include "webrtc" in Chromium/iOS builds. It's not part of the gyp build either, and `ninja -C out/gnios webrtc` causes several build errors, at least in Chromium builds (which means gn/ios bots building 'all' are broken). BUG=chromium:633316 R=kjellander@webrtc.org Committed: https://chromium.googlesource.com/external/webrtc/+/3ab32dc7752cbcdbfb1318348... ==========
Message was sent while issue was closed.
Description was changed from ========== gn: Do not include "webrtc" in Chromium/iOS builds. It's not part of the gyp build either, and `ninja -C out/gnios webrtc` causes several build errors, at least in Chromium builds (which means gn/ios bots building 'all' are broken). BUG=chromium:633316 R=kjellander@webrtc.org Committed: https://chromium.googlesource.com/external/webrtc/+/3ab32dc7752cbcdbfb1318348... ========== to ========== gn: Do not include "webrtc" in Chromium/iOS builds. It's not part of the gyp build either, and `ninja -C out/gnios webrtc` causes several build errors, at least in Chromium builds (which means gn/ios bots building 'all' are broken). BUG=chromium:633316 R=kjellander@webrtc.org Committed: https://crrev.com/3ab32dc7752cbcdbfb13183486f7238b4a89363b Cr-Commit-Position: refs/heads/master@{#13609} ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) manually as 3ab32dc7752cbcdbfb13183486f7238b4a89363b (presubmit successful). |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
