Bazel incompatible changes

[keith]

This issue tracks incompatible bazel changes that cause upstream bazel CI test failures.

Starting today we have upstream bazel CI building and testing envoy so bazel can catch issues sooner. As part of this we want to try and stay ahead of upcoming breaking bazel changes (more discussion here).

Now that this is enabled I want to take a first pass on trying to get us to incompatible change 0, so future changes are easier to keep up with.

Here's the most recent build showing us which specific changes we aren't compatible with https://buildkite.com/bazel/bazelisk-plus-incompatible-flags/builds/110#72eb6717-c478-44de-833d-b7da832ee58e

From a quick test locally most of these incompatibilities are coming from upstream dependencies. I will submit a series of PRs to update these were applicable and link them to this issue.

cc @htuch @laurentlb

[keith]

I've submitted a PR against protoc-gen-validate with some changes bufbuild/protoc-gen-validate#200

[keith]

We're being hit by bazelbuild/bazel#7849 which I'm not sure what the migration plan is for, but it appears to be affecting protobuf directly as well.

[laurentlb]

Two flags I'd like to flip soon are:

• --incompatible_depset_is_not_iterable (broken due to https://github.com/envoyproxy/protoc-gen-validate)
• --incompatible_disable_deprecated_attr_params (broken due to grpc, but I think grpc has been fixed)

[htuch]

CC @lizan @moderation @fcfort. Do we have any Bazel ninjas who can give the fixes for this a shot? Also @rodaine for PGV aspects.

[keith]

I'm currently working on some of these. I'll post an update shortly.

…

-- Keith Smiley

On May 20, 2019, at 14:07, htuch ***@***.***> wrote: CC @lizan @moderation @fcfort. Do we have any Bazel ninjas who can give the fixes for this a shot? Also @rodaine for PGV aspects. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[keith]

Ok so when building on macOS, using this branch of protoc-gen-validate, master from rules_foreign_cc, and ignoring this flag bazelbuild/bazel#7260 (comment)

I'm at migration 0. I don't see the issue with --incompatible_disable_deprecated_attr_params, @laurentlb any idea why? If we did have to update grpc, we'd need to wait until their current pre-release was actually released.

[laurentlb]

Repro:

$ bazel build --nobuild --incompatible_disable_deprecated_attr_params test/...
...
	File "/usr/local/google/home/laurentlb/.cache/bazel/_bazel_laurentlb/71d59151158aafb7db935cf8065f6d0c/external/com_lyft_protoc_gen_validate/bazel/protobuf.bzl", line 155, in rule
		attr.label(cfg = "host", default = Label("@co..."), <2 more arguments>)
'single_file' is no longer supported. use allow_single_file instead. You can use --incompatible_disable_deprecated_attr_params=false to temporarily disable this check.

-> Need to update https://github.com/envoyproxy/protoc-gen-validate/blob/master/bazel/protobuf.bzl (running buildifier --lint=fix might be enough)

Repro:

$ bazel build --nobuild --incompatible_depset_is_not_iterable test/...
...
	File "/usr/local/google/home/laurentlb/.cache/bazel/_bazel_laurentlb/71d59151158aafb7db935cf8065f6d0c/external/com_github_grpc_grpc/bazel/generate_cc.bzl", line 10, in generate_cc_impl
		[f for src in ctx.attr.srcs for f in src.proto.transitive_imports]
type 'depset' is not iterable. Use the `to_list()` method to get a list. Use --incompatible_depset_is_not_iterable=false to temporarily disable this check.

This comes from grpc (I haven't checked if/when it was fixed in grpc).

[fcfort]

Looks like @keith has fixes for both issues in bufbuild/protoc-gen-validate#198, which are blocked on bufbuild/protoc-gen-validate#200 going in as well.

[keith]

I've submitted the grpc change upstream grpc/grpc#19097 I didn't see that because I was testing against //source/... not //test/...

[hlopko]

Hi all, I'm updating all Bazel incompatible flags related issues - the Bazel team is in increased pressure because 0.27 will start a 3 month window of no incompatible changes). We will have to potentially temporarily disable envoy on our downstream pipeline if we cannot get it fixed this week.

[hlopko]

To consolidate efforts, these issues are also related to Bazel incompatible flags:

• Update rules_foreign_cc version #7058 - migrates for incompatible_enable_cc_toolchain_resolution: Turn on toolchain resolution for cc rules bazelbuild/bazel#7260 and incompatible_require_ctx_in_configure_features: Require ctx argument in cc_common.configure_features bazelbuild/bazel#7793
• libgcc and libstdc++ not linking statically with incompatible Bazel flag #6968 - required for incompatible_do_not_split_linking_cmdline: Order of C++ link flags affected bazelbuild/bazel#7687.
• ci: update build image #6866 - (seemingly stalled?) update of Bazel to 0.25.

[keith]

@hlopko let me know if I can help with any of these!

[hlopko]

Both #6968 and #6866 could use somebody with Envoy domain knowledge to push them forward :)

#7058 seems unblocked and will be submitted hopefully soon.

[stale]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

[hlopko]

Seems this issue could be closed?

[mattklein123]

Closing, thank you @keith @hlopko for driving this forward!