Add Bazel support #416

lizan · 2017-02-03T08:52:24Z

Porting from https://github.com/istio/proxy/blob/53fd02610571f8c05af2e50fea911f89f935e678/src/envoy/repositories.bzl .
For #415.

cc @PiotrSikora @fengli79

PiotrSikora · 2017-02-03T12:07:23Z

repositories.bzl

+    tools = [
+        "configure",
+    ],
+    cmd = ' '.join([


Buildifier: s/' '/" "/g.

Also, I personally prefer concatenating strings, instead of joining them:

cmd = "$(location configure) --enable-lib-only --enable-shared=no" + " && cp config.h $@",

instead of:

cmd = ' '.join([ "$(location configure) --enable-lib-only --enable-shared=no", "&& cp config.h $@", ]),

PiotrSikora · 2017-02-03T12:08:25Z

repositories.bzl

+    outs = [
+        "config.h",
+    ],
+    tools = [


Buildifier: tools should be listed after cmd.

PiotrSikora · 2017-02-03T12:09:03Z

repositories.bzl

+        "http_parser.h",
+    ],
+    visibility = ["//visibility:public"],
+)"""


Buildifier: add newline at the end.

PiotrSikora · 2017-02-03T12:09:21Z

repositories.bzl

+        "include",
+    ],
+    visibility = ["//visibility:public"],
+)"""


Buildifier: add newline at the end.

PiotrSikora · 2017-02-03T12:10:12Z

repositories.bzl

+    outs = [
+        "config.h",
+    ],
+    tools = [


Buildifier: tools should be listed after cmd.

PiotrSikora · 2017-02-03T12:56:46Z

repositories.bzl

+    native.new_git_repository(
+        name = "http_parser_git",
+        remote = "https://github.com/nodejs/http-parser.git",
+        commit = "9b0d5b33ebdaacff1dadd06bad4e198b11ff880e",


Please add comment with version or commit date.

PiotrSikora · 2017-02-03T12:57:28Z

repositories.bzl

+    native.new_git_repository(
+        name = "rapidjson_git",
+        remote = "https://github.com/miloyip/rapidjson.git",
+        commit = "f54b0e47a08782a6131cc3d60f94d038fa6e0a51", # v1.1.0


Style: there should be 2 spaces before the comment.

PiotrSikora · 2017-02-03T12:58:20Z

repositories.bzl

+
+    native.new_http_archive(
+        name = "nghttp2_tar",
+        url = "https://github.com/nghttp2/nghttp2/releases/download/v1.14.1/nghttp2-1.14.1.tar.gz",


Any reason we're using .tar.gz and not git clone?

PiotrSikora · 2017-02-03T13:00:23Z

repositories.bzl

+            actual = "@boringssl//:ssl",
+        )
+
+


Buildifier: remove extra empty line.

PiotrSikora · 2017-02-03T13:00:35Z

repositories.bzl

+            actual = "@libevent_git//:event_pthreads",
+        )
+
+


Buildifier: remove extra empty line.

rshriram · 2017-02-03T13:51:06Z

Hi, having two entirely different build systems in one repository is a pain, not to mention Bazel itself, from the experiences we have had in istio/manager.

CMake needs to remain as the canonical build system. So, what's the strategy for keeping the BUILD and WORKSPACE files in sync, when people add or remove files from envoy or add dependencies? These files will quickly go out of sync and no one will bother to update the bazel files with "gazelle" or whatever additional tool is needed to track the dependent libraries, etc.

Asking folks to run a script to sync bazel build files before they submit an Envoy PR is futile as they cannot verify the correctness of the build files unless they are forced to install the bazel build system, and validate.

PiotrSikora · 2017-02-03T14:22:44Z

@rshriram while using two different build systems is indeed far from perfect, the current one, which builds Envoy inside "random" Docker image isn't the smoothes experience, either.

mattklein123 · 2017-02-03T16:08:05Z

It's true that the docker image that is used to build is currently "random" in that it is "latest." We can easily version that to fix the "random" problem.

I was under the impression that bazel was just a wrapper around dealing with all of the third party dependencies. I agree with @rshriram that trying to keep both build systems up to date is going to be futile, so I'm not sure how much this makes sense, unless we can switch to mostly manage third party as convenience but still use cmake for the main envoy build.

mattklein123 · 2017-02-03T16:09:38Z

docs/install/building.rst

+
+  bazel test //:envoy-test
+
+Note not all tests pass with Bazel yet.


Why don't they pass?

bazel test run tests in different paths than do_ci.sh does, results some fixtures cannot be found. I will do a follow up PR after this to fix them.

rshriram · 2017-02-03T16:25:49Z

It certainly is much better than the bazel build for two reasons.. I can develop on any machine without any issues related to case insensitive file systems, etc. and just use plain docker image to build. The source code can be on my native OS (e.g., OSX), and I can run whatever IDE I want. Second, all the dependencies are cleanly packaged into one docker container, that I do not have to bother installing on my machine.

With bazel in the picture, you would still need to run the entire bazel build in a docker container, if you want to provide an easy way to build/develop, or force people to run a big fat VM to hack on Envoy. However, this requires more workarounds, and is slower as per this blog post https://bazel.build/blog/2016/01/27/continuous-integration.html

Ultimately, in the end, the so called "random" docker container is still in the picture unless you want to force everyone to build inside a VM or linux only.

mattklein123 · 2017-02-03T16:38:45Z

Let's try to focus this conversation a bit. We are not getting rid of the docker build system. We can make the docker build system better by explicitly versioning the build image and using that version vs. latest in do_ci.sh

If people want to add a parallel bazel build system for convenience, it either needs to be best effort and not fully supported, or there needs to be a way to make sure it is kept up to date.

lizan · 2017-02-03T19:11:35Z

@rshriram I didn't mean to replace cmake/docker build system, the issues related to case insensitive file systems does not apply here, that is the issue with bazel go support, which is not primary. C++ experience is much better with bazel.

The BUILD file I wrote here is fairly tolerant to source code structure changes. I didn't change the BUILD since its first version in last November.

The bazel BUILD here is intended to be used locally, not with docker. There is no need to provide a docker image to build/develop easily. Just use the bazel command in the docs, installing docker vs. bazel is comparable.

mattklein123 · 2017-02-03T19:18:45Z

@lizan I'm OK with this for now as long as:

We leave the experimental tag in the docs (which you have now)
We agree that the bazel stuff is best effort. We aren't going to test it in CI (for now) and it might break. As such bazel users might have to fix stuff post fact.

If we are OK with that I'm fine getting this in. LMK once @PiotrSikora and others are happy and we can merge.

lizan · 2017-02-03T22:59:30Z

@mattklein123 I'm OK with that, we will maintain the BUILD files anyway for istio/proxy. @rshriram let me know if you still have concerns and feel free to ping me on gitter or istio-dev slack.

rshriram · 2017-02-04T00:05:50Z

As long as its not in the CI, i am fine with it as well. And I will take your word on the C++ experience. Will try out sometime on local machine. :)

htuch · 2017-02-06T18:25:11Z

I don't want to derail the conversation, but thought I'd bring up another project that manages multiple build systems: https://github.com/grpc/grpc. Here, the canonical build rules and dependencies are expressed via a build.yaml file in the root directory, and then the project build/makefiles for the various systems (bazel, cmake, Visual Studio, etc.) are generated prior to the actual build.

See https://github.com/grpc/grpc/tree/master/templates and https://github.com/grpc/grpc/tree/master/tools/buildgen.

It might be worth chatting with some of the folks behind the grpc build work to understand how well this has worked and how reusable their templating solution is.

lizan · 2017-02-06T21:16:12Z

@htuch Thanks for bringing that up. I'm open to it, but I think bringing that here is too much effort, at least for now. GRPC has much more languages and build systems to support, and grpc needs to be available via multiple building systems as dependency.

Bazel support here is just in experimental stage and best effort. If eventually envoy team decided to keep both building system as primary, then we should consider generating build files.

mattklein123

@lizan other than the small doc nit, I have no real ability to review this change. Please get a +1 from @PiotrSikora

mattklein123 · 2017-02-08T01:04:26Z

docs/install/building.rst

@@ -27,3 +27,24 @@ available to turn on debug builds, address sanitizer, etc.).
 * :repo:`CMakeLists.txt`
 * :repo:`common.cmake`
 * :repo:`thirdparty.cmake`
+
+(Experimental) Building with Bazel_


nit: can you actually put this under a section heading.

I would suggest moving this content to the readme that @htuch has.

Yes I think it's a good idea to move this to the dev docs for now once that change gets merged.

Sounds good, I will move once that change merged.

lizan · 2017-02-09T17:10:16Z

I will also wait until c-ares changes are merged so I can get c-ares into the dependencies as well.

mattklein123 · 2017-02-15T21:45:25Z

@lizan can we get this merged or close for now?

lizan · 2017-02-15T21:56:05Z

OK, let's close this for now, I will be OOO tomorrow until next Friday.

This patch introduces the minimal Bazel infrastructure required to build and execute utility_test, as a step towards conversion of the cmake build system to Bazel (envoyproxy#415). The main contributions are: * Machinery for declaring Envoy C++ library and test targets (envoy_cc_library, envoy_cc_test). * External dependency management that works both inside the Docker CI environment (using prebuilts) and with developer-local dependency fetch/build (based on envoyproxy#416). * Handling of the implicit dependencies that are today sourced via prebuilts. * Simple example of building and running a unit test with only its dependencies built. With the cmake system, we would need to build all of Envoy and all tests to run utility_test. E.g. blaze test //test/common/common:utility_test This is not intended to be used by anyone other than those working with the Bazel conversion at this point. The plan is to add "ci/do_ci.sh bazel.debug" as a Travis target to ensure we don't bit rot.

* Minimal Bazel infrastructure (#415). This patch introduces the minimal Bazel infrastructure required to build and execute utility_test, as a step towards conversion of the cmake build system to Bazel (#415). The main contributions are: * Machinery for declaring Envoy C++ library and test targets (envoy_cc_library, envoy_cc_test). * External dependency management that works both inside the Docker CI environment (using prebuilts) and with developer-local dependency fetch/build (based on #416). * Handling of the implicit dependencies that are today sourced via prebuilts. * Simple example of building and running a unit test with only its dependencies built. With the cmake system, we would need to build all of Envoy and all tests to run utility_test. E.g. blaze test //test/common/common:utility_test This is not intended to be used by anyone other than those working with the Bazel conversion at this point. The plan is to add "ci/do_ci.sh bazel.debug" as a Travis target to ensure we don't bit rot.

* update BUILD.api * remove many mixer go dependencies. * use mock mixer test. * use official pubref repo. * use mixer test repositories.bzl

Automatic merge from submit-queue. [DO NOT MERGE] Auto PR to update dependencies of mixerclient This PR will be merged automatically once checks are successful. ```release-note none ```

Merge remote envoy

…roxy#416)

Implements the [gRPC protocol](https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md) on top of the existing Envoy Mobile interfaces, allowing for easily sending gRPC requests over Envoy. Note: The `GRPCResponseHandler` isn't kept in memory by the library (similarly to `ResponseHandler`) - only its closures are passed down to the core. Thus, no state is stored on the handler, and the necessary data is kept in memory by the closures capturing it. Throughout the buffering function, we're currently copying some data by doing `append` calls. This may be optimized further in the future. This PR also includes a set of tests for this new functionality. Example of sending a gRPC request: ```swift let handler = GRPCResponseHandler(queue: .main) .onHeaders { headers, grpcStatus, _ in print("gRPC status: \(grpcStatus), headers: \(headers)") } .onMessage { messageData, _ in print("Got message over gRPC: \(messageData)") } .onError { error in print("gRPC failed with error: \(error)") } let requestBuilder = GRPCRequestBuilder(path: "/pb.api.v1.foo.Bar/Baz", authority: "api.foo.com") let emitter = GRPCClient(httpClient: envoy).send(requestBuilder.build(), handler: handler) emitter.sendMessage(someProtoMessage.serializedData()) ... ``` Risk Level: Low (new feature) Testing: Unit tests, tested locally Signed-off-by: Michael Rebello <me@michaelrebello.com> Signed-off-by: JP Simard <jp@jpsim.com>

lizan added 2 commits February 2, 2017 17:47

Add bazel BUILD file

8135afc

Add building instruction

2d0f96c

PiotrSikora suggested changes Feb 3, 2017

View reviewed changes

mattklein123 reviewed Feb 3, 2017

View reviewed changes

mattklein123 mentioned this pull request Feb 3, 2017

build: update docker image on every master commit that passes tests #155

Closed

lizan added 2 commits February 3, 2017 13:47

Apply bulidifier

a444344

No bind

975131e

lizan added 2 commits February 6, 2017 17:02

Address review comments

f3f7cbc

Merge remote-tracking branch 'upstream/master' into bazel_build

a49b28b

mattklein123 reviewed Feb 8, 2017

View reviewed changes

lizan closed this Feb 15, 2017

htuch mentioned this pull request Mar 16, 2017

Minimal Bazel infrastructure (#415). #577

Merged

rshriram pushed a commit to rshriram/envoy that referenced this pull request Oct 30, 2018

Use mock mixer (envoyproxy#416)

37b6e80

* update BUILD.api * remove many mixer go dependencies. * use mock mixer test. * use official pubref repo. * use mixer test repositories.bzl

PiotrSikora added a commit to istio/envoy that referenced this pull request Feb 24, 2020

Merge pull request envoyproxy#416 from gargnupur/nup_update3

cacdada

Merge remote envoy

wolfguoliang pushed a commit to wolfguoliang/envoy that referenced this pull request Jan 23, 2021

zh-translation:docs/root/configuration/listeners/overview.rst (envoyp…

d160efa

…roxy#416)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Bazel support #416

Add Bazel support #416

lizan commented Feb 3, 2017

PiotrSikora Feb 3, 2017

PiotrSikora Feb 3, 2017

PiotrSikora Feb 3, 2017

lizan Feb 7, 2017

PiotrSikora Feb 3, 2017

lizan Feb 7, 2017

PiotrSikora Feb 3, 2017

PiotrSikora Feb 3, 2017

PiotrSikora Feb 3, 2017

PiotrSikora Feb 3, 2017

PiotrSikora Feb 3, 2017

PiotrSikora Feb 3, 2017

rshriram commented Feb 3, 2017

PiotrSikora commented Feb 3, 2017

mattklein123 commented Feb 3, 2017

mattklein123 Feb 3, 2017

lizan Feb 3, 2017

rshriram commented Feb 3, 2017

mattklein123 commented Feb 3, 2017

lizan commented Feb 3, 2017 •

edited

Loading

mattklein123 commented Feb 3, 2017

lizan commented Feb 3, 2017

rshriram commented Feb 4, 2017

htuch commented Feb 6, 2017

lizan commented Feb 6, 2017

mattklein123 left a comment

mattklein123 Feb 8, 2017

rshriram Feb 8, 2017 •

edited

Loading

mattklein123 Feb 8, 2017

lizan Feb 9, 2017

lizan commented Feb 9, 2017

mattklein123 commented Feb 15, 2017

lizan commented Feb 15, 2017


		bazel test //:envoy-test

		Note not all tests pass with Bazel yet.

Add Bazel support #416

Add Bazel support #416

Conversation

lizan commented Feb 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rshriram commented Feb 3, 2017

PiotrSikora commented Feb 3, 2017

mattklein123 commented Feb 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rshriram commented Feb 3, 2017

mattklein123 commented Feb 3, 2017

lizan commented Feb 3, 2017 • edited Loading

mattklein123 commented Feb 3, 2017

lizan commented Feb 3, 2017

rshriram commented Feb 4, 2017

htuch commented Feb 6, 2017

lizan commented Feb 6, 2017

mattklein123 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rshriram Feb 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lizan commented Feb 9, 2017

mattklein123 commented Feb 15, 2017

lizan commented Feb 15, 2017

lizan commented Feb 3, 2017 •

edited

Loading

rshriram Feb 8, 2017 •

edited

Loading