GN: Put BoringSSL Chromium specific targets inside a condition

GN: Put BoringSSL Chromium specific targets inside a condition

The boringssl_unittests target depends on Chromium's base,
so having it being processed by default results in errors
for other projects reusing the GN files for BoringSSL that
are located in the Chromium repo.

BUG=webrtc:5829, 606944

[kjellander_chromium, 2016-04-26 20:07:28]

kjellander@chromium.org changed reviewers:
+ davidben@chromium.org, dpranke@chromium.org

[kjellander_chromium, 2016-04-26 20:07:50]

The CQ bit was checked by kjellander@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-26 20:08:13]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1919383002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1919383002/1

[Dirk Pranke, 2016-04-26 20:13:49]

lgtm if you add a TODO to reference some bug to figure out whether we should get
rid of boringssl_unittests altogether or get rid of the dependency on base.

[kjellander_chromium, 2016-04-26 20:47:34]

Description was changed from

==========
GN: Add include_boringssl_unittests variable.

The boringssl_unittests target depends on Chromium's base,
so having them being processed by default results in errors
for other projects reusing the GN files for BoringSSL that
are located in the Chromium repo.

BUG=webrtc:5829
==========

to

==========
GN: Add include_boringssl_unittests variable.

The boringssl_unittests target depends on Chromium's base,
so having them being processed by default results in errors
for other projects reusing the GN files for BoringSSL that
are located in the Chromium repo.

BUG=webrtc:5829, 606944
==========

[davidben, 2016-04-26 21:05:11]

lgtm

https://codereview.chromium.org/1919383002/diff/1/third_party/boringssl/BUILD.gn
File third_party/boringssl/BUILD.gn (right):

https://codereview.chromium.org/1919383002/diff/1/third_party/boringssl/BUILD...
third_party/boringssl/BUILD.gn:154: component("boringssl_fuzzer") {
Do you want to omit all these other bits too? (boringssl_fuzzer and the
fuzzer_test foreach.) I'm actually kind of confused how that is working at all,
but I bet it's only working on accident.

[commit-bot: I haz the power, 2016-04-26 21:05:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-26 21:05:54]

Dry run: This issue passed the CQ dry run.

[davidben, 2016-04-26 21:07:56]

Er, actually, not lgtm. (I really want to just clear the l-g-t-m, but Rietveld
doesn't let me do this.)

https://codereview.chromium.org/1919383002/diff/1/third_party/boringssl/BUILD.gn
File third_party/boringssl/BUILD.gn (right):

https://codereview.chromium.org/1919383002/diff/1/third_party/boringssl/BUILD...
third_party/boringssl/BUILD.gn:12: include_boringssl_unittests = false
Oh. Actually, I would rather we at least build this, even if it's not getting
run. We at least get compile-testing and, as discussed out-of-band, it's work
along the way to getting in-Chromium embeddings of tests. That they're not
running right now is something I was actively fixing before the WebRTC circular
dependency came up.

[davidben, 2016-04-26 21:12:12]

https://codereview.chromium.org/1919383002/diff/1/third_party/boringssl/BUILD.gn
File third_party/boringssl/BUILD.gn (right):

https://codereview.chromium.org/1919383002/diff/1/third_party/boringssl/BUILD...
third_party/boringssl/BUILD.gn:12: include_boringssl_unittests = false
On 2016/04/26 21:07:56, davidben wrote:
> Oh. Actually, I would rather we at least build this, even if it's not getting
> run. We at least get compile-testing and, as discussed out-of-band, it's work
> along the way to getting in-Chromium embeddings of tests. That they're not
> running right now is something I was actively fixing before the WebRTC
circular
> dependency came up.

That or we should decide this is not a thing we can support at all and we just
take the targets out completely. But since we already have the Chromium-specific
fuzzer tests and those have become invaluable quickly, I think the idea that we
cannot have any Chromium-specific BoringSSL targets at all is untenable.

[kjellander_chromium, 2016-04-26 21:15:48]

Description was changed from

==========
GN: Add include_boringssl_unittests variable.

The boringssl_unittests target depends on Chromium's base,
so having them being processed by default results in errors
for other projects reusing the GN files for BoringSSL that
are located in the Chromium repo.

BUG=webrtc:5829, 606944
==========

to

==========
GN: Put BoringSSL Chromium specific targets inside a condition

The boringssl_unittests target depends on Chromium's base,
so having it being processed by default results in errors
for other projects reusing the GN files for BoringSSL that
are located in the Chromium repo.

BUG=webrtc:5829, 606944
==========

[kjellander_chromium, 2016-04-26 21:21:51]

On 2016/04/26 21:12:12, davidben wrote:
>
https://codereview.chromium.org/1919383002/diff/1/third_party/boringssl/BUILD.gn
> File third_party/boringssl/BUILD.gn (right):
> 
>
https://codereview.chromium.org/1919383002/diff/1/third_party/boringssl/BUILD...
> third_party/boringssl/BUILD.gn:12: include_boringssl_unittests = false
> On 2016/04/26 21:07:56, davidben wrote:
> > Oh. Actually, I would rather we at least build this, even if it's not
getting
> > run. We at least get compile-testing and, as discussed out-of-band, it's
work
> > along the way to getting in-Chromium embeddings of tests. That they're not
> > running right now is something I was actively fixing before the WebRTC
> circular
> > dependency came up.
> 
> That or we should decide this is not a thing we can support at all and we just
> take the targets out completely. But since we already have the
Chromium-specific
> fuzzer tests and those have become invaluable quickly, I think the idea that
we
> cannot have any Chromium-specific BoringSSL targets at all is untenable.

PTAL@PS#4. I think that's what we need (and I've verified it works for WebRTC
too).

FYI: Our WebRTC fuzzers are located here:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc...

[kjellander_chromium, 2016-04-26 21:24:17]

The CQ bit was checked by kjellander@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-26 21:25:01]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1919383002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1919383002/60001

[davidben, 2016-04-26 21:25:17]

lgtm, but probably make sure Dirk is happy with this too.

https://codereview.chromium.org/1919383002/diff/60001/build_overrides/build.gni
File build_overrides/build.gni (right):

https://codereview.chromium.org/1919383002/diff/60001/build_overrides/build.g...
build_overrides/build.gni:15: build_with_chromium = true
Should this be in a boringssl.gni or maybe third_party/boringssl include
webrtc.gni? I'll defer to you all since I don't know what the convention is
here.

[commit-bot: I haz the power, 2016-04-26 21:29:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-26 21:29:20]

Dry run: Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)
  android_chromium_gn_compile_rel on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Dirk Pranke, 2016-04-26 21:43:22]

lgtm

https://codereview.chromium.org/1919383002/diff/60001/build_overrides/build.gni
File build_overrides/build.gni (right):

https://codereview.chromium.org/1919383002/diff/60001/build_overrides/build.g...
build_overrides/build.gni:15: build_with_chromium = true
On 2016/04/26 21:25:16, davidben wrote:
> Should this be in a boringssl.gni or maybe third_party/boringssl include
> webrtc.gni? I'll defer to you all since I don't know what the convention is
> here.

The convention is totally made up :). I think this location is fine as it'll
be used for variables needed across multiple repos.

And, again, hopefully this all goes away soon and is replaced with something
less stupid (which I can say since it's my design ;).

[Dirk Pranke, 2016-04-26 21:46:15]

Of course, we still need a reference to build_with_chromium via
build_overrides/webrtc.gni , so all of these things are failing.

I'll post a CL with a fix, since I can't upload a new patch to this CL.

[Dirk Pranke, 2016-04-26 21:54:01]

On 2016/04/26 21:46:15, Dirk Pranke wrote:
> Of course, we still need a reference to build_with_chromium via
> build_overrides/webrtc.gni , so all of these things are failing.
> 
> I'll post a CL with a fix, since I can't upload a new patch to this CL.

Here's the fix (theoretically):

https://codereview.chromium.org/1923683002/#ps20001

[kjellander_chromium, 2016-04-27 06:42:45]

On 2016/04/26 21:46:15, Dirk Pranke wrote:
> Of course, we still need a reference to build_with_chromium via
> build_overrides/webrtc.gni , so all of these things are failing.
> 
> I'll post a CL with a fix, since I can't upload a new patch to this CL.

Ah, thanks for dealing with that.