Skip to content

Use Bazel native build rules for BoringSSL.#2652

Merged
htuch merged 3 commits intoenvoyproxy:masterfrom
jmillikin-stripe:bazel-native-boringssl
Feb 22, 2018
Merged

Use Bazel native build rules for BoringSSL.#2652
htuch merged 3 commits intoenvoyproxy:masterfrom
jmillikin-stripe:bazel-native-boringssl

Conversation

@jmillikin-stripe
Copy link
Contributor

Description:
Now that we've fixed the CC vs CXX compiler situation, we can safely build BoringSSL using its native Bazel rules.

The commit ID change is because BoringSSL's BUILD rules are maintained on a separate branch, which gets merges from "chromium-stable". The commit 426db8 is the merge of a20bb7.

Fixes #1549

Risk Level: Low

Signed-off-by: John Millikin jmillikin@stripe.com

Testing:
I built and tested this locally on a MacOS machine with Clang. Will depend on CI to test with other configurations.

Fixes envoyproxy#1549

Signed-off-by: John Millikin <jmillikin@stripe.com>
remote = "https://github.com/abseil/abseil-cpp",
),
com_googlesource_boringssl = dict(
commit = "426db8db7d1cbd17573e295b52d7aab7a97ba1ff", # chromium-64.0.3282.119
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you also add chromium-stable-with-bazel as the tag for future reference

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Signed-off-by: John Millikin <jmillikin@stripe.com>
)

def _com_googlesource_boringssl():
_repository_impl("com_googlesource_boringssl")
Copy link
Contributor

@PiotrSikora PiotrSikora Feb 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong. While you're trying to come up with "semi-standard" name, you're ignoring the boringssl name provided in BoringSSL's WORKSPACE, which leads to a build time warning:

WARNING: /build/tmp/_bazel_bazel/436badd4919a15958fa3800a4e21074a/external/com_googlesource_boringssl/WORKSPACE:1: Workspace name in /build/tmp/_bazel_bazel/436badd4919a15958fa3800a4e21074a/external/com_googlesource_boringssl/WORKSPACE (@boringssl) does not match the name given in the repository's definition (@com_googlesource_boringssl); this will cause a build error in future versions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I'd been looking at the chromium-stable branch and forgot WORKSPACE was populated in chromium-stable-with-bazel. Fixed.

Signed-off-by: John Millikin <jmillikin@stripe.com>
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@htuch htuch merged commit e88442b into envoyproxy:master Feb 22, 2018
@jmillikin-stripe jmillikin-stripe deleted the bazel-native-boringssl branch February 22, 2018 17:46
jpsim pushed a commit that referenced this pull request Nov 28, 2022
Risk Level: n/a
Testing: n/a
Docs Changes: n/a
Release Notes: n/a
(hopefully) fixes #2652

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit that referenced this pull request Nov 29, 2022
Risk Level: n/a
Testing: n/a
Docs Changes: n/a
Release Notes: n/a
(hopefully) fixes #2652

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: JP Simard <jp@jpsim.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bazel build should not need external go install

4 participants