New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 606944 link

Starred by 1 user
Status: WontFix
Owner:
█ kjellander@chromium.org
Last visit > 30 days ago
Closed: Mar 2017
Cc:
davidben@chromium.org
dpranke@chromium.org
█ brettw@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

BoringSSL GN files hard to re-use by other projects.

Project Member Reported by kjellander@chromium.org, Apr 26 2016 
Projects like WebRTC relies heavily on BoringSSL and needs to stay up to date with a high frequency with the security fixes etc. Because of that WebRTC has an automated rolling system that pulls in the version Chrome currently has in its DEPS file.
WebRTC also uses the same GYP and GN files as the ones maintained in Chrome's src/third_party/boringssl directory (the BoringSSL source code itself is DEPSed in to src/third_party/boringssl/src).

Without the approach above, WebRTC would have to manually maintain a separate set of GYP and GN files, which doesn't make sense (and would have a high maintenance cost).
See https://bugs.chromium.org/p/webrtc/issues/detail?id=5829 for more details.

In Chromium, a boringssl_unittest target exists, which wraps the large number of lower-level tests that exists for BoringSSL. It's currently not being build or run from Chromium checkouts, but it might be useful to keep around for simplicity in test execution (although since the tests already run in BoringSSL's continuous integration, it's not really necessary to have that luxury).

This bug tracks how to deal with this problem and how to work around it from WebRTC's perspective.

Comment 1 by davidben@chromium.org, Apr 26 2016 

As discussed out-of-band, I think this is ultimately working as intended, but not ideal for WebRTC (so we should cut things up further). Chromium's third_party/boringssl (rather than third_party/boringssl/src) is supposed to be Chromium-specific. That was the reason for the third_party/boringssl vs third_party/boringssl/src split. boringssl_unittests isn't the only Chromium-specific target. We also have fuzzers, and there's some is_nacl check in there too. It just happened to be the first that didn't work on accident.

In the short-term, WebRTC's circular dependency on Chromium src.git means we need to add some hacks so src.git's third_party/boringssl/ can serve two masters. Long-term, I think we need to fix WebRTC. Since asking WebRTC to run generate_build_files.py is obnoxious, I think this blocks on the BoringSSL "with batteries" branch idea we'd been throwing around recently. That way WebRTC can maintain a minimal BoringSSL BUILD.gn file that ~never changes, or perhaps none at all. (Maybe the batteries will include a basic one, I dunno.)

> (although since the tests already run in BoringSSL's continuous integration, it's not really necessary to have that luxury).

BoringSSL has a lot of configuration-specific bits and fiddly things like assembly. I think it is important that we have consumer embeddings of the tests to make sure the BoringSSL that Chromium builds is functional.
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 28 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/da2f89b031149de3966bda976cb5ac7ed3b5d4ac

commit da2f89b031149de3966bda976cb5ac7ed3b5d4ac
Author: kjellander <kjellander@chromium.org>
Date: Thu Apr 28 06:55:45 2016

GN: Move boringssl_tests into build_with_chromium condition

BUG= 606944 

Review-Url: https://codereview.chromium.org/1917223004
Cr-Commit-Position: refs/heads/master@{#390314}

[modify] https://crrev.com/da2f89b031149de3966bda976cb5ac7ed3b5d4ac/third_party/boringssl/BUILD.gn

Comment 3 by kjellander@chromium.org, Mar 27 2017

Status: WontFix (was: Available)
I haven't had any issues with this since when this bug was filed so I'm leaning towards doing nothing for now. We really want to avoid maintaining our own BUILD.gn files whenever possible. Especially for such complex ones as https://cs.chromium.org/chromium/src/third_party/boringssl/BUILD.gn

Comment 4 by davidben@chromium.org, Mar 27 2017 

The offending target is ultimately going to be removed and replaced with the native upstream GTest ones anyway, when we finish converting everything.
►

Sign in to add a comment