Build with latest Bazel(upgrade Bazel from 0.5.4 to 0.21.0) #529
Conversation
doc/build-esp-on-ubuntu-16-04.md
Outdated
| @@ -57,4 +60,7 @@ libio-socket-ssl-perl is needed to run ESP tests: | |||
|
|
|||
| To run ESP tests, run the following command in the terminal. | |||
|
|
|||
| bazel test //src/... //third_party:all | |||
| bazel test --incompatible_remove_native_git_repository=false \ | |||
There was a problem hiding this comment.
Under the ROOT folder, same folder as WORKSPACE, you can have .bazelrc file, similar to
https://github.com/cloudendpoints/esp/blob/master/tools/bazel.rc
But the new bazel obsoleted it,
I believe over there, you can specify these flags. so that it will apply to all
I don't know the detail, could you study it?
| @@ -51,7 +47,7 @@ go_library( | |||
| srcs = glob( | |||
| ["google/protobuf/*.pb.go"], | |||
| ), | |||
| go_prefix = "//tools/src:go_prefix", | |||
| importpath = "github.com/cloudendpoints/esp", | |||
There was a problem hiding this comment.
with this change, can we at least build it by
bazel build //tools/src/...
Otherwise, not need to change it
There was a problem hiding this comment.
Though with this change "bazel build //tools/src/..." still fails, I would like keep it because go_prefix is no longer valid syntax in new version of Bazel and we need to use importpath instead. Also this moves one more step toward making "bazel build //tools/src/..." successful.
|
Good job. all presubmit tests are passed now. I just started a e2e test for this https://endpoints-jenkins.appspot.com/job/jenkins-testing/job/pr-e2e/ |
|
Please rebase your changes |
jiachenwang
force-pushed
the
update_bazel
branch
from
e63c68d to
fe261e9
Compare
|
Upgrading Bazel would be a very welcome change. |
|
@jiachenwang could you rebase this PR? |
| 'bazel-bin/external/org_golang_google_grpc/stress/metrics_client/metrics_client', | ||
| 'bazel-bin/external/org_golang_google_grpc/interop/server/server', | ||
| 'bazel-bin/external/org_golang_google_grpc/stress/client/client', | ||
| 'bazel-bin/test/grpc/linux_amd64_stripped/interop-client', |
There was a problem hiding this comment.
if --config=asan or debug,the file name will without linux_amd64_stripped. do we need to stash or save them?
There was a problem hiding this comment.
Jenkins stores Release version binary to GCS bucket for e2e testing, so we do not neet stash asan or debug one.
WORKSPACE
Outdated
| @@ -168,7 +171,7 @@ bind( | |||
| # | |||
| git_repository( | |||
| name = "io_bazel_rules_pex", | |||
| commit = "6af30588d4f11cafcb744c50935cc37f029e6e7f", | |||
| commit = "0c5773db01ab8aeb3ae749b2fc570749b93af41f", | |||
There was a problem hiding this comment.
please add comment on to state the date of this SHA
WORKSPACE
Outdated
| http_archive( | ||
| name = "bazel_gazelle", | ||
| urls = ["https://github.com/bazelbuild/bazel-gazelle/releases/download/0.16.0/bazel-gazelle-0.16.0.tar.gz"], | ||
| sha256 = "7949fc6cc17b5b191103e97481cf8889217263acf52e00b560683413af204fcb", |
There was a problem hiding this comment.
please state the date of the SHA
repositories.bzl
Outdated
| """ | ||
| native.new_git_repository( | ||
| name = "org_golang_google_grpc_git", | ||
| commit = "9bf8ea0a8282ebecd1aa474c926e3028f5c22a4c", |
There was a problem hiding this comment.
please state the date for this SHA
| build_file_content = BUILD, | ||
| ) | ||
|
|
||
| if bind: |
There was a problem hiding this comment.
It seems this bind is not used. when it is binded, instead of depends on "@org_golang_google_grpc_git//:test_proto", you can depend on "//external:test_proto"
I saw the BUILD file use @org_golang_google_grpc_git//:test_proto directly. so bind is not needed.
There was a problem hiding this comment.
Ack. Other parts in this file uses bind = True. I would like to leave it to be consistent.
jiachenwang
force-pushed
the
update_bazel
branch
3 times, most recently
from
b53af9d to
eb76fe2
Compare
|
Could you rebase it and run pre-e2e again? Thanks |
jiachenwang
force-pushed
the
update_bazel
branch
from
eb76fe2 to
6e68051
Compare
|
Thanks. @jiachenwang Great job to get this in. |
No description provided.