build: update rules_go and data-plane-api #2617
Conversation
| @@ -14,5 +14,3 @@ load("@com_lyft_protoc_gen_validate//bazel:go_proto_library.bzl", "go_proto_repo | |||
| go_proto_repositories(shared=0) | |||
| go_rules_dependencies() | |||
| go_register_toolchains() | |||
| load("@io_bazel_rules_go//proto:def.bzl", "proto_register_toolchains") | |||
| proto_register_toolchains() | |||
There was a problem hiding this comment.
this is no longer needed in 0.9.0
|
Builds are failing due to what looks like a rules_go bug, investigating. |
|
The version of rules_go being upgraded from (0.8.1) appears to have bazel-contrib/rules_go@812c172 in it. See https://github.com/bazelbuild/rules_go/blob/0.8.1/go/private/go_repository.bzl#L52-L55 for the Was there a later commit that made improvements to proxy handling? I don't see anything recent in the rules_go commit logs. |
|
We're actually updating from 4374be38e9a75ff5957c3922adb155d32086fe14 (see https://github.com/envoyproxy/envoy/blob/master/bazel/repository_locations.bzl#L82), which does not contain the proxy fix: The sha we're currently at looks like just before 0.7.0? although it doesn't map directly to a tag: The proxy fix was added to 0.8.0 and back ported to 0.7.1 |
|
Ah, got it, I misread. Would it be possible to upgrade rules_go to only 0.7.1? Even though it's old, that's fine -- it's only being used to generate .go code from protobufs if I remember correctly. |
|
Wait, now I remember where I saw 0.8.1 -- that's the version Could this be fixed by bringing the rules_go dependency versions into sync? |
|
Sure I'll give that a go. |
|
Didn't seem like 0.8.1 did. I'll try 0.7.1 just in case. I don't think the version mismatch between api and envoy matters since the envoy version would take precedence in this case. |
snowp
force-pushed
the
update-rules-go
branch
from
5dc4db6 to
259d3aa
Compare
|
Looks like even 0.7.1 fails :( |
|
Looks like even 0.7.1 is failing. Digging more into the commits I found the sequence is:
which I guess explains why it's pinned to a non-tag commit. One possible solution is to take the Envoy commit, apply the proxy patch, and host it outside the main rules_go repo. @htuch has done something like this in the past with protobuf. Would you be OK with this approach? If so, I'll try to get it in place tomorrow. |
|
Yeah I'd be okay with forking rules_go to get this working. I was trying to avoid it, but it looks like that would be the easiest way to resolve this at this point. |
|
I'll rebase this and point back to 0.9.0. Should hopefully go green |
snowp
force-pushed
the
update-rules-go
branch
from
259d3aa to
c863b15
Compare
|
Updating data-plane-api is no longer necessary since it's been updated already on master |
|
Looks like we're all green. Gonna rebase to get the DCO right and then we should be good to go |
This updates rules_go to 0.9.0 in order to bring in bazel-contrib/rules_go@812c172 which is necessary in order to build envoy from behind a proxy. Signed-off-by: Snow Pettersen <snowp@squareup.com>
snowp
force-pushed
the
update-rules-go
branch
from
c863b15 to
c192ed4
Compare
|
all checks passed! @htuch we should be able to merge this |
Signed-off-by: Prabu Shyam <prabushyam41@gmail.com> Co-authored-by: Prabu Shyam <prabushyam@users.noreply.github.com>
title: build: update rules_go and data-plane-api
Description:
This updates rules_go to 0.9.0 in order to bring in
bazel-contrib/rules_go@812c172
which is necessary in order to build envoy from behind a proxy.
Updating data-plane-api is necessary in order to to update the version
of protoc-gen-validate to a version that includes bufbuild/protoc-gen-validate@3204975
Risk Level: Low
This only modifies build dependencies. Only risk I can imagine is incompatible protos being brought in by data-plane-api, but I'd imagine code review in that repo would have prevented that.
Testing:
Ran the tests on OS X. I also successfully built
//source/exe:envoy-staticon Centos 7 behind a proxy (after specifying HTTP(S)_PROXY) in order to verify that this fixes #2578.Docs Changes:
n/a
Release Notes:
n/a
[Optional Fixes #Issue]
This should address #2578