Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bazel] Fix blacklisted_protos in cc_toolchain and add test #7075

Merged
merged 1 commit into from
Jan 15, 2020

Conversation

Yannic
Copy link
Contributor

@Yannic Yannic commented Jan 10, 2020

No description provided.

@Yannic
Copy link
Contributor Author

Yannic commented Jan 10, 2020

We're running the test that verifies WKPs are excluded twice: One time as //:cc_proto_blacklist_test and one time as @com_google_protobuf//:cc_proto_blacklist_test. For some reason, it passes only one time. I have no idea why that makes a difference.

I see the same errors for Bazel 2.0.0 and Bazel 2.0.0 + bazelbuild/bazel#10493

$ /private/var/tmp/_bazel_yannic/1460a25073e849a4f43b8278b99f9b6d/execroot/io_bazel/bazel-out/darwin-fastbuild/bin/src/bazel test @com_google_protobuf//:cc_proto_blacklist_test //:cc_proto_blacklist_test
INFO: Invocation ID: 811f4b86-e0c0-4580-b7ff-b98f5533e30a
DEBUG: /private/var/tmp/_bazel_yannic/c28f18b452d490f3ae8554a2bc9b8bed/external/bazel_skylib/lib/unittest.bzl:351:5: In test _cc_proto_blacklist_test_impl from //:cc_proto_blacklist_test.bzl: Expected "0", but got "48"
INFO: Analyzed 2 targets (0 packages loaded, 0 targets configured).
INFO: Found 2 test targets...
FAIL: //:cc_proto_blacklist_test (see /private/var/tmp/_bazel_yannic/c28f18b452d490f3ae8554a2bc9b8bed/execroot/com_google_protobuf/bazel-out/darwin-fastbuild/testlogs/cc_proto_blacklist_test/test.log)
INFO: Elapsed time: 0.705s, Critical Path: 0.49s
INFO: 2 processes: 2 darwin-sandbox.
INFO: Build completed, 1 test FAILED, 2 total actions
@com_google_protobuf//:cc_proto_blacklist_test                  (cached) PASSED in 0.1s
//:cc_proto_blacklist_test                                               FAILED in 0.3s
  /private/var/tmp/_bazel_yannic/c28f18b452d490f3ae8554a2bc9b8bed/execroot/com_google_protobuf/bazel-out/darwin-fastbuild/testlogs/cc_proto_blacklist_test/test.log

Executed 1 out of 2 tests: 1 test passes and 1 fails locally.
INFO: Build completed, 1 test FAILED, 2 total actions

Currently bisecting if that ever worked...

@Yannic
Copy link
Contributor Author

Yannic commented Jan 10, 2020

I tried several Bazel versions between 0.20.0 and 2.0.0, but I always see the behavior from my previous comment.

@jtattermusch
Copy link
Contributor

I tried several Bazel versions between 0.20.0 and 2.0.0, but I always see the behavior from my previous comment.

Any progress on this?

@Yannic
Copy link
Contributor Author

Yannic commented Jan 15, 2020

Unfortunately not, but IIUC, the failing test only indicates that blacklisting doesn't work inside the protobuf repo, anyone else should be fine (see grpc/grpc#21590 (comment)).

However, running the test on 3.8 with the old blacklisting mechanism also fails with the same error, so I think this (and #7096) can be merged.

@Yannic
Copy link
Contributor Author

Yannic commented Jan 15, 2020

bazel test @com_google_protobuf//:cc_proto_blacklist_test //:cc_proto_blacklist_test does show a bug in how Bazel handles the blacklist (bazelbuild/bazel#10590).

When using the default toolchain for cc_proto_library (--proto_toolchain_for_cc=@com_google_protobuf//:cc_toolchain), @com_google_protobuf//:cc_proto_blacklist_test passes and //:cc_proto_blacklist_test fails.
When changing the toolchain to --proto_toolchain_for_cc=//:cc_toolchain, //:cc_proto_blacklist_test starts passing and @com_google_protobuf//:cc_proto_blacklist_test fails.

This is only an issue for the protobuf repo itself, everyone else is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants