bazel: bump Bazel repositories versions #3413

adambabik · 2023-07-21T05:55:47Z

References to other Issues or PRs

Hopefully fixes #3407

Brief description of what is fixed or changed

I had a problem to generate Bazel build files from main and noticed #3407. I bumped both rules_go and gazelle which resulted in getting rid of @go_googleapis references as expected.

One more problem I had after bumping rules_proto, which was also outdated, was that Bazel used older C++ standard by default. I enforced the C++14 with BAZEL_CXXOPTS="-std=c++14" bazel run :buildifier, however, there might be a better way to achieve this.

adambabik · 2023-07-21T19:58:39Z

I moved it to drafts as there are more problems to fix.

adambabik · 2023-07-23T06:52:03Z

BUILD.bazel

@@ -19,6 +19,18 @@ buildifier(
 # gazelle:go_proto_compilers //:go_apiv2
 # gazelle:go_grpc_compilers //:go_apiv2, //:go_grpc
 # gazelle:go_naming_convention import_alias
+# gazelle:resolve proto proto google/api/annotations.proto @googleapis//google/api:annotations_proto


I am not sure if this is the best place for these directives.

I will defer to @achew22 on this

I think this is the correct place for them

adambabik · 2023-07-23T06:54:45Z

CONTRIBUTING.md

@@ -19,7 +19,7 @@ docker run -itv $(pwd):/grpc-gateway -w /grpc-gateway --entrypoint /bin/bash --r
    ghcr.io/grpc-ecosystem/grpc-gateway/build-env:1.19 -c '\
        bazel run :gazelle -- update-repos -from_file=go.mod -to_macro=repositories.bzl%go_repositories && \
        bazel run :gazelle && \
-        bazel run :buildifier'
+        BAZEL_CXXOPTS="-std=c++14" bazel run :buildifier'


Alternatively, we could put .bazelrc in the project root. I found that in ci.yml we are setting exactly the same env.

We have various environment specific options like the CPU and memory limiters that shouldn't go in .bazelrc, but this option does seem like a reasonable thing to put in a repo wide .bazelrc.

I would probably drop it in the .bazelrc,

build --cxxopt='-std=c++14'

but I have a pathological aversion to environment variables.

johanbrandhorst

🙏🏻 thank you for this. I will give Andrew a chance to look at this before merging, but it LGTM.

achew22 · 2023-07-24T06:24:51Z

BUILD.bazel

@@ -19,6 +19,18 @@ buildifier(
 # gazelle:go_proto_compilers //:go_apiv2
 # gazelle:go_grpc_compilers //:go_apiv2, //:go_grpc
 # gazelle:go_naming_convention import_alias
+# gazelle:resolve proto proto google/api/annotations.proto @googleapis//google/api:annotations_proto


I think this is the correct place for them

achew22 · 2023-07-24T06:27:36Z

CONTRIBUTING.md

@@ -19,7 +19,7 @@ docker run -itv $(pwd):/grpc-gateway -w /grpc-gateway --entrypoint /bin/bash --r
    ghcr.io/grpc-ecosystem/grpc-gateway/build-env:1.19 -c '\
        bazel run :gazelle -- update-repos -from_file=go.mod -to_macro=repositories.bzl%go_repositories && \
        bazel run :gazelle && \
-        bazel run :buildifier'
+        BAZEL_CXXOPTS="-std=c++14" bazel run :buildifier'


I would probably drop it in the .bazelrc,

build --cxxopt='-std=c++14'

but I have a pathological aversion to environment variables.

achew22 · 2023-07-24T06:29:15Z

WORKSPACE

+load("@googleapis//:repository_rules.bzl", "switched_rules_by_language")
+
+switched_rules_by_language(
+    name = "com_google_googleapis_imports",


Don't you need to set go = true?

https://github.com/googleapis/googleapis/blob/master/repository_rules.bzl#L87

It seems we don't need this, because we use @googleapis only in proto_library. Go deps are resolved to @org_golang_google_genproto_googleapis_api.

I suppose that I could change:

# gazelle:resolve proto go google/api/annotations.proto @org_golang_google_genproto_googleapis_api//annotations

to

# gazelle:resolve proto go google/api/annotations.proto @googleapis//google/api:annotations_go_proto

then go = True would be necessary. But I am not sure if it gives us any benefits. @org_golang_google_genproto_googleapis_api was recommended in go_rules and gazelle.

adambabik requested a review from johanbrandhorst July 21, 2023 05:56

adambabik marked this pull request as draft July 21, 2023 19:57

adambabik marked this pull request as ready for review July 23, 2023 06:51

adambabik commented Jul 23, 2023

View reviewed changes

johanbrandhorst requested a review from achew22 July 23, 2023 23:45

johanbrandhorst approved these changes Jul 23, 2023

View reviewed changes

achew22 approved these changes Jul 24, 2023

View reviewed changes

adambabik added 3 commits July 24, 2023 21:52

bazel: bump Bazel repositories versions

8793731

bazel: clean up kept deps

9fce765

bazel: add .bazelrc

d650006

adambabik force-pushed the fix-bazel-go_googleapis branch from e162627 to d650006 Compare July 24, 2023 19:52

adambabik merged commit 78aa6e4 into main Jul 24, 2023
18 checks passed

adambabik deleted the fix-bazel-go_googleapis branch July 24, 2023 20:24

FrankSpitulski mentioned this pull request Aug 2, 2023

rules_go 0.41.0 upgrade breaking on missing go_googleapis #3407

Open

adambabik mentioned this pull request Aug 2, 2023

Fix renovate.yml overwriting .bazelrc #3451

Merged

alexbozhenko mentioned this pull request Sep 21, 2023

upd grpc-gateway rules-proto-grpc/rules_proto_grpc#283

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bazel: bump Bazel repositories versions #3413

bazel: bump Bazel repositories versions #3413

adambabik commented Jul 21, 2023

adambabik commented Jul 21, 2023

adambabik Jul 23, 2023

johanbrandhorst Jul 23, 2023

achew22 Jul 24, 2023

adambabik Jul 23, 2023

johanbrandhorst Jul 23, 2023

achew22 Jul 24, 2023

johanbrandhorst left a comment

achew22 Jul 24, 2023

achew22 Jul 24, 2023

achew22 Jul 24, 2023

adambabik Jul 24, 2023 •

edited

Loading

bazel: bump Bazel repositories versions #3413

bazel: bump Bazel repositories versions #3413

Conversation

adambabik commented Jul 21, 2023

References to other Issues or PRs

Brief description of what is fixed or changed

adambabik commented Jul 21, 2023

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

johanbrandhorst left a 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

adambabik Jul 24, 2023 • edited Loading

Choose a reason for hiding this comment

adambabik Jul 24, 2023 •

edited

Loading