include boringssl headers from third_party explicitly

include boringssl headers from third_party explicitly

This also allow us to participate in DEPS checking, which will help
catch instances of directories including BoringSSL without adding to
build targets.

This patch was partly generated by the following command lines:

$ g grep -l -e '^#[[:blank:]]*include <\(openssl[^>]*\)>' | xargs sed -i
'/^#[[:blank:]]*include/s/<\(openssl[^>]*\)>/"\1"/'
$ g grep -l "#include \"openssl/" | xargs sed -i -e 's/\(#.*
\)"\(openssl\/.*\)"/\1"third_party\/boringssl\/src\/include\/\2"/'

The regex were taken from http://stackoverflow.com/a/25378698 and
https://svn.boost.org/trac/boost/ticket/12057, and adapted to suit our
needs.

Then the includes were put in their right places with some manual editing and
the help of tools/sort-headers.py.

BUG=446558
R=davidben@chromium.org,thestig@chromium.org,jochen@chromium.org,slan@chromium.org

Committed: https://crrev.com/29a3a174ef1227551d596b260d57506bbf493cc0
Cr-Commit-Position: refs/heads/master@{#428442}

[tfarina, 2016-10-27 20:44:32]

Description was changed from

==========
include boringssl headers from third_party explicitly

This prevents the system OpenSSL headers from being silently included
instead of
our headers.

This also allow us to participate in DEPS checking, which will help
catch instances of directories including BoringSSL without adding to
build targets.

This patch was partly generated by the following command lines:

$ g grep -l -e '^#[[:blank:]]*include <\(openssl[^>]*\)>' | xargs sed -i
'/^#[[:blank:]]*include/s/<\(openssl[^>]*\)>/"\1"/'
$ g grep -l "#include \"openssl/" | xargs sed -i -e 's/\(#.*
\)"\(openssl\/.*\)"/\1"third_party\/boringssl\/src\/include\/\2"/'

The regex were taken from http://stackoverflow.com/a/25378698 and
https://svn.boost.org/trac/boost/ticket/12057, and adapted to suit our
needs.

Then the includes were put in their right places with some manual editing and
the help of tools/sort-headers.py.

BUG=446558
R=davidben@chromium.org
==========

to

==========
include boringssl headers from third_party explicitly

This prevents the system OpenSSL headers from being silently included
instead of
our headers.

This also allow us to participate in DEPS checking, which will help
catch instances of directories including BoringSSL without adding to
build targets.

This patch was partly generated by the following command lines:

$ g grep -l -e '^#[[:blank:]]*include <\(openssl[^>]*\)>' | xargs sed -i
'/^#[[:blank:]]*include/s/<\(openssl[^>]*\)>/"\1"/'
$ g grep -l "#include \"openssl/" | xargs sed -i -e 's/\(#.*
\)"\(openssl\/.*\)"/\1"third_party\/boringssl\/src\/include\/\2"/'

The regex were taken from http://stackoverflow.com/a/25378698 and
https://svn.boost.org/trac/boost/ticket/12057, and adapted to suit our
needs.

Then the includes were put in their right places with some manual editing and
the help of tools/sort-headers.py.

BUG=446558
R=davidben@chromium.org
==========

[tfarina, 2016-10-27 20:44:32]

tfarina@chromium.org changed reviewers:
+ jochen@chromium.org, slan@chromium.org, thestig@chromium.org

[tfarina, 2016-10-27 20:48:39]

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

[commit-bot: I haz the power, 2016-10-27 20:49:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tfarina, 2016-10-27 20:51:56]

David, everything and owner for crypto + net.
Lei -> chrome
Jochen -> components, content
Slan -> chromecast

[davidben, 2016-10-27 20:54:01]

I haven't looked at this yet, but real quick before I forget, this is not true:

This prevents the system OpenSSL headers from being silently included
instead of our headers.

BoringSSL headers, being a standalone project, reference each other with
#include <openssl/rsa.h>, so things still won't work. We are still reliant on
the -I flag getting passed in. Using relative paths in BoringSSL headers would
avoid that, but even that won't avoid the -I flags. We pull in third-party code
which expects BoringSSL/OpenSSL be found at <openssl/rsa.h>.

[slan, 2016-10-27 21:43:15]

On 2016/10/27 20:54:01, davidben wrote:
> I haven't looked at this yet, but real quick before I forget, this is not
true:
> 
> This prevents the system OpenSSL headers from being silently included
> instead of our headers.
> 
> BoringSSL headers, being a standalone project, reference each other with
> #include <openssl/rsa.h>, so things still won't work.

+ryangchung@ for FYI

I'd like to see this comment resolved before giving l-g-t-m. All Cast platforms
use boringssl, so from our perspective, I don't anticipate any problems with
making this explicit.

[commit-bot: I haz the power, 2016-10-27 21:46:06]

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

[commit-bot: I haz the power, 2016-10-27 21:46:07]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[davidben, 2016-10-27 21:57:23]

On 2016/10/27 21:43:15, slan wrote:
> On 2016/10/27 20:54:01, davidben wrote:
> > I haven't looked at this yet, but real quick before I forget, this is not
> true:
> > 
> > This prevents the system OpenSSL headers from being silently included
> > instead of our headers.
> > 
> > BoringSSL headers, being a standalone project, reference each other with
> > #include <openssl/rsa.h>, so things still won't work.
> 
> +ryangchung@ for FYI
> 
> I'd like to see this comment resolved before giving l-g-t-m. All Cast
platforms
> use boringssl, so from our perspective, I don't anticipate any problems with
> making this explicit.

To clarify, my comment was just asking for a sentence to be removed from the
description. Most things in //third_party work this way. They internally include
things as a system thing (since naturally a standalone project doesn't know it
lives in a "third_party" directory), so -I build flags are needed, but our code
uses the fully-qualified path.

In other words, this is desirable in so far as it makes things more consistent
(although the new #include lines are atrocious :-/) and gives us gn check. It
just doesn't give us anything more than that.

[davidben, 2016-10-27 22:30:36]

https://codereview.chromium.org/2449873005/diff/140001/chrome/common/DEPS
File chrome/common/DEPS (right):

https://codereview.chromium.org/2449873005/diff/140001/chrome/common/DEPS#new...
chrome/common/DEPS:39: 
Stray newline?

https://codereview.chromium.org/2449873005/diff/140001/components/webcrypto/a...
File components/webcrypto/algorithms/ec.cc (right):

https://codereview.chromium.org/2449873005/diff/140001/components/webcrypto/a...
components/webcrypto/algorithms/ec.cc:4: #include "base/logging.h"
Er, it seems this file got rather messed up.

[tfarina, 2016-10-27 23:00:05]

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

[commit-bot: I haz the power, 2016-10-27 23:00:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tfarina, 2016-10-27 23:01:26]

Description was changed from

==========
include boringssl headers from third_party explicitly

This prevents the system OpenSSL headers from being silently included
instead of
our headers.

This also allow us to participate in DEPS checking, which will help
catch instances of directories including BoringSSL without adding to
build targets.

This patch was partly generated by the following command lines:

$ g grep -l -e '^#[[:blank:]]*include <\(openssl[^>]*\)>' | xargs sed -i
'/^#[[:blank:]]*include/s/<\(openssl[^>]*\)>/"\1"/'
$ g grep -l "#include \"openssl/" | xargs sed -i -e 's/\(#.*
\)"\(openssl\/.*\)"/\1"third_party\/boringssl\/src\/include\/\2"/'

The regex were taken from http://stackoverflow.com/a/25378698 and
https://svn.boost.org/trac/boost/ticket/12057, and adapted to suit our
needs.

Then the includes were put in their right places with some manual editing and
the help of tools/sort-headers.py.

BUG=446558
R=davidben@chromium.org
==========

to

==========
include boringssl headers from third_party explicitly

This also allow us to participate in DEPS checking, which will help
catch instances of directories including BoringSSL without adding to
build targets.

This patch was partly generated by the following command lines:

$ g grep -l -e '^#[[:blank:]]*include <\(openssl[^>]*\)>' | xargs sed -i
'/^#[[:blank:]]*include/s/<\(openssl[^>]*\)>/"\1"/'
$ g grep -l "#include \"openssl/" | xargs sed -i -e 's/\(#.*
\)"\(openssl\/.*\)"/\1"third_party\/boringssl\/src\/include\/\2"/'

The regex were taken from http://stackoverflow.com/a/25378698 and
https://svn.boost.org/trac/boost/ticket/12057, and adapted to suit our
needs.

Then the includes were put in their right places with some manual editing and
the help of tools/sort-headers.py.

BUG=446558
R=davidben@chromium.org
==========

[tfarina, 2016-10-27 23:02:01]

Description was changed from

==========
include boringssl headers from third_party explicitly

This also allow us to participate in DEPS checking, which will help
catch instances of directories including BoringSSL without adding to
build targets.

This patch was partly generated by the following command lines:

$ g grep -l -e '^#[[:blank:]]*include <\(openssl[^>]*\)>' | xargs sed -i
'/^#[[:blank:]]*include/s/<\(openssl[^>]*\)>/"\1"/'
$ g grep -l "#include \"openssl/" | xargs sed -i -e 's/\(#.*
\)"\(openssl\/.*\)"/\1"third_party\/boringssl\/src\/include\/\2"/'

The regex were taken from http://stackoverflow.com/a/25378698 and
https://svn.boost.org/trac/boost/ticket/12057, and adapted to suit our
needs.

Then the includes were put in their right places with some manual editing and
the help of tools/sort-headers.py.

BUG=446558
R=davidben@chromium.org
==========

to

==========
include boringssl headers from third_party explicitly

This also allow us to participate in DEPS checking, which will help
catch instances of directories including BoringSSL without adding to
build targets.

This patch was partly generated by the following command lines:

$ g grep -l -e '^#[[:blank:]]*include <\(openssl[^>]*\)>' | xargs sed -i
'/^#[[:blank:]]*include/s/<\(openssl[^>]*\)>/"\1"/'
$ g grep -l "#include \"openssl/" | xargs sed -i -e 's/\(#.*
\)"\(openssl\/.*\)"/\1"third_party\/boringssl\/src\/include\/\2"/'

The regex were taken from http://stackoverflow.com/a/25378698 and
https://svn.boost.org/trac/boost/ticket/12057, and adapted to suit our
needs.

Then the includes were put in their right places with some manual editing and
the help of tools/sort-headers.py.

BUG=446558
R=davidben@chromium.org,thestig@chromium.org,jochen@chromium.org,slan@chromiu...
==========

[tfarina, 2016-10-27 23:02:34]

Sentence removed.

https://codereview.chromium.org/2449873005/diff/140001/chrome/common/DEPS
File chrome/common/DEPS (right):

https://codereview.chromium.org/2449873005/diff/140001/chrome/common/DEPS#new...
chrome/common/DEPS:39: 
On 2016/10/27 22:30:36, davidben wrote:
> Stray newline?

Done.

https://codereview.chromium.org/2449873005/diff/140001/components/webcrypto/a...
File components/webcrypto/algorithms/ec.cc (right):

https://codereview.chromium.org/2449873005/diff/140001/components/webcrypto/a...
components/webcrypto/algorithms/ec.cc:4: #include "base/logging.h"
On 2016/10/27 22:30:36, davidben wrote:
> Er, it seems this file got rather messed up.

Er, still developing my emacs skills. Clearly something went wrong. :/

[commit-bot: I haz the power, 2016-10-28 00:26:17]

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

[commit-bot: I haz the power, 2016-10-28 00:26:18]

Dry run: This issue passed the CQ dry run.

[davidben, 2016-10-28 00:31:05]

lgtm

[slan, 2016-10-28 00:34:03]

cast lgtm, thanks!

[jochen (gone - plz use gerrit), 2016-10-28 13:40:14]

lgtm

[Lei Zhang, 2016-10-28 17:19:25]

chrome/ lgtm

https://codereview.chromium.org/2449873005/diff/160001/chrome/common/DEPS
File chrome/common/DEPS (right):

https://codereview.chromium.org/2449873005/diff/160001/chrome/common/DEPS#new...
chrome/common/DEPS:36: "+third_party/boringssl/src/include",
BTW, I think
chrome/common/extensions/api/networking_private/networking_private_crypto.cc can
be moved into chrome/browser, and then this wouldn't be necessary anymore. Maybe
in a follow up CL?

[tfarina, 2016-10-28 18:39:50]

https://codereview.chromium.org/2449873005/diff/160001/chrome/common/DEPS
File chrome/common/DEPS (right):

https://codereview.chromium.org/2449873005/diff/160001/chrome/common/DEPS#new...
chrome/common/DEPS:36: "+third_party/boringssl/src/include",
On 2016/10/28 17:19:25, Lei Zhang wrote:
> BTW, I think
> chrome/common/extensions/api/networking_private/networking_private_crypto.cc
can
> be moved into chrome/browser, and then this wouldn't be necessary anymore.
Maybe
> in a follow up CL?

I will look in a follow up.

[tfarina, 2016-10-28 18:39:59]

The CQ bit was checked by tfarina@chromium.org

[commit-bot: I haz the power, 2016-10-28 18:40:28]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-28 18:48:03]

Description was changed from

==========
include boringssl headers from third_party explicitly

This also allow us to participate in DEPS checking, which will help
catch instances of directories including BoringSSL without adding to
build targets.

This patch was partly generated by the following command lines:

$ g grep -l -e '^#[[:blank:]]*include <\(openssl[^>]*\)>' | xargs sed -i
'/^#[[:blank:]]*include/s/<\(openssl[^>]*\)>/"\1"/'
$ g grep -l "#include \"openssl/" | xargs sed -i -e 's/\(#.*
\)"\(openssl\/.*\)"/\1"third_party\/boringssl\/src\/include\/\2"/'

The regex were taken from http://stackoverflow.com/a/25378698 and
https://svn.boost.org/trac/boost/ticket/12057, and adapted to suit our
needs.

Then the includes were put in their right places with some manual editing and
the help of tools/sort-headers.py.

BUG=446558
R=davidben@chromium.org,thestig@chromium.org,jochen@chromium.org,slan@chromiu...
==========

to

==========
include boringssl headers from third_party explicitly

This also allow us to participate in DEPS checking, which will help
catch instances of directories including BoringSSL without adding to
build targets.

This patch was partly generated by the following command lines:

$ g grep -l -e '^#[[:blank:]]*include <\(openssl[^>]*\)>' | xargs sed -i
'/^#[[:blank:]]*include/s/<\(openssl[^>]*\)>/"\1"/'
$ g grep -l "#include \"openssl/" | xargs sed -i -e 's/\(#.*
\)"\(openssl\/.*\)"/\1"third_party\/boringssl\/src\/include\/\2"/'

The regex were taken from http://stackoverflow.com/a/25378698 and
https://svn.boost.org/trac/boost/ticket/12057, and adapted to suit our
needs.

Then the includes were put in their right places with some manual editing and
the help of tools/sort-headers.py.

BUG=446558
R=davidben@chromium.org,thestig@chromium.org,jochen@chromium.org,slan@chromiu...
==========

[commit-bot: I haz the power, 2016-10-28 18:48:04]

Committed patchset #9 (id:160001)

[Lei Zhang, 2016-10-28 18:51:16]

On 2016/10/28 18:39:50, tfarina wrote:
> https://codereview.chromium.org/2449873005/diff/160001/chrome/common/DEPS
> File chrome/common/DEPS (right):
> 
>
https://codereview.chromium.org/2449873005/diff/160001/chrome/common/DEPS#new...
> chrome/common/DEPS:36: "+third_party/boringssl/src/include",
> On 2016/10/28 17:19:25, Lei Zhang wrote:
> > BTW, I think
> > chrome/common/extensions/api/networking_private/networking_private_crypto.cc
> can
> > be moved into chrome/browser, and then this wouldn't be necessary anymore.
> Maybe
> > in a follow up CL?
> 
> I will look in a follow up.

Please give it a try. I think there is a slight benefit because then the code on
Windows won't have to be duplicated between chrome.dll and chrome_child.dll. I
haven't checked to make sure it can be moved to chrome/browser though.

[commit-bot: I haz the power, 2016-10-28 19:09:10]

Description was changed from

==========
include boringssl headers from third_party explicitly

This also allow us to participate in DEPS checking, which will help
catch instances of directories including BoringSSL without adding to
build targets.

This patch was partly generated by the following command lines:

$ g grep -l -e '^#[[:blank:]]*include <\(openssl[^>]*\)>' | xargs sed -i
'/^#[[:blank:]]*include/s/<\(openssl[^>]*\)>/"\1"/'
$ g grep -l "#include \"openssl/" | xargs sed -i -e 's/\(#.*
\)"\(openssl\/.*\)"/\1"third_party\/boringssl\/src\/include\/\2"/'

The regex were taken from http://stackoverflow.com/a/25378698 and
https://svn.boost.org/trac/boost/ticket/12057, and adapted to suit our
needs.

Then the includes were put in their right places with some manual editing and
the help of tools/sort-headers.py.

BUG=446558
R=davidben@chromium.org,thestig@chromium.org,jochen@chromium.org,slan@chromiu...
==========

to

==========
include boringssl headers from third_party explicitly

This also allow us to participate in DEPS checking, which will help
catch instances of directories including BoringSSL without adding to
build targets.

This patch was partly generated by the following command lines:

$ g grep -l -e '^#[[:blank:]]*include <\(openssl[^>]*\)>' | xargs sed -i
'/^#[[:blank:]]*include/s/<\(openssl[^>]*\)>/"\1"/'
$ g grep -l "#include \"openssl/" | xargs sed -i -e 's/\(#.*
\)"\(openssl\/.*\)"/\1"third_party\/boringssl\/src\/include\/\2"/'

The regex were taken from http://stackoverflow.com/a/25378698 and
https://svn.boost.org/trac/boost/ticket/12057, and adapted to suit our
needs.

Then the includes were put in their right places with some manual editing and
the help of tools/sort-headers.py.

BUG=446558
R=davidben@chromium.org,thestig@chromium.org,jochen@chromium.org,slan@chromiu...

Committed: https://crrev.com/29a3a174ef1227551d596b260d57506bbf493cc0
Cr-Commit-Position: refs/heads/master@{#428442}
==========

[commit-bot: I haz the power, 2016-10-28 19:09:11]

Patchset 9 (id:??) landed as
https://crrev.com/29a3a174ef1227551d596b260d57506bbf493cc0
Cr-Commit-Position: refs/heads/master@{#428442}

[tfarina, 2016-10-30 00:48:05]

On Friday, October 28, 2016, <thestig@chromium.org> wrote:

> On 2016/10/28 18:39:50, tfarina wrote:
> > https://codereview.chromium.org/2449873005/diff/160001/
> chrome/common/DEPS
> > File chrome/common/DEPS (right):
> >
> >
> https://codereview.chromium.org/2449873005/diff/160001/
> chrome/common/DEPS#newcode36
> > chrome/common/DEPS:36: "+third_party/boringssl/src/include",
> > On 2016/10/28 17:19:25, Lei Zhang wrote:
> > > BTW, I think
> > > chrome/common/extensions/api/networking_private/networking_
> private_crypto.cc
> > can
> > > be moved into chrome/browser, and then this wouldn't be necessary
> anymore.
> > Maybe
> > > in a follow up CL?
> >
> > I will look in a follow up.
>
> Please give it a try. I think there is a slight benefit because then the
> code on
> Windows won't have to be duplicated between chrome.dll and
> chrome_child.dll. I
> haven't checked to make sure it can be moved to chrome/browser though.
>
> Done.  https://codereview.chromium.org/2464663002/





-- 
Thiago Farina

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.