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.

Sign up for GitHub

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

Jump to bottom

proto_lang_toolchain and blacklisted_protos with ProtoInfo not propagated correctly #10484

Closed
Tracked by #10005
keith opened this issue Dec 23, 2019 · 2 comments
Closed
Tracked by #10005

proto_lang_toolchain and blacklisted_protos with ProtoInfo not propagated correctly #10484

keith opened this issue Dec 23, 2019 · 2 comments
Labels
team-Rules-Server Issues for serverside rules included with Bazel untriaged

Comments

@keith
Copy link
Member

keith commented Dec 23, 2019

Description of the problem / feature request:

With protobuf's proto_lang_toolchain, after this change protocolbuffers/protobuf@7b28278 it seems the blacklisted_protos are no longer working correctly and removing that field has no effect on the build.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

See protocolbuffers/protobuf#7046 for a repro case

What operating system are you running Bazel on?

macOS

What's the output of bazel info release?

release 1.2.0 (I also tried with 2.0.0 with no change)

Have you found anything relevant by searching the web?

The protobuf commit linked above referenced this bazel change a5ee2c4

I'm also wondering if the issue is with protobuf's use of strip_import_prefix on the proto_library targets they define: protocolbuffers/protobuf@a03d332
This was referenced Dec 23, 2019
Closed
Closed
@jin
Copy link
Member

jin commented Dec 26, 2019

cc @cushon

@jin jin added team-Rules-Server Issues for serverside rules included with Bazel untriaged labels Dec 26, 2019
Yannic added a commit to Yannic/bazel that referenced this issue Dec 27, 2019
@Yannic
proto_lang_toolchain: Add original sources of ProtoInfo to blackliste…
6375e57 
…dProtos

Fixes bazelbuild#10484
@Yannic Yannic mentioned this issue Dec 27, 2019
Closed
@Yannic
Copy link
Contributor

Yannic commented Dec 27, 2019

Suspecting a combination of a5ee2c4 and 46c95dc (/cc @lberki).

Submitted #10493 to fix this for now.
Longer-term, we should make ProtoInfo mandatory for blacklisted_protos.

@Yannic Yannic mentioned this issue Dec 27, 2019
Closed
9 tasks
@rebello95 rebello95 mentioned this issue Dec 31, 2019
Merged
rebello95 added a commit to envoyproxy/envoy-mobile that referenced this issue Dec 31, 2019
@rebello95
core: apply protobuf patch to prevent duplicate symbols (#618)
3052a69 
Fixes #617, which resulted in duplicate symbols from protobuf being present in the final Envoy Mobile binary.

The [root cause](bazelbuild/bazel#10484) is being [patched in Bazel](bazelbuild/bazel#10493). In the meantime, we can revert these two commits in protobuf ([first](protocolbuffers/protobuf@7b28278), [second](protocolbuffers/protobuf@a03d332)) and apply the diff as a patch until we're able to update Bazel with the fix. More context on the protobuf issue is available [here](protocolbuffers/protobuf#7046).

Notes:
- This should be reverted after updating Bazel with the fix
- The version of protobuf specified in the `http_archive` should be in lockstep with upstream Envoy's version
- Upstream Envoy's `@envoy//bazel:protobuf.patch` patch is also included here

Signed-off-by: Michael Rebello <me@michaelrebello.com>
@gnossen gnossen mentioned this issue Jan 7, 2020
Merged
@Yannic Yannic mentioned this issue Jan 8, 2020
Merged
@Yannic Yannic mentioned this issue Jan 14, 2020
Closed
bazel-io pushed a commit that referenced this issue Jan 15, 2020
@meteorcloudy @copybara-github
Automated rollback of commit 4162cc5.
9db93bc 
*** Reason for rollback ***

Broken Bazel CI in Downstream pipeline
#10588

*** Original change description ***

proto_lang_toolchain: Add original sources of ProtoInfo to blacklistedProtos

Fixes #10484

Closes #10493.

PiperOrigin-RevId: 289828390
Yannic added a commit to Yannic/bazel that referenced this issue Jan 15, 2020
@Yannic
proto_lang_toolchain: Add original sources of ProtoInfo to blackliste…
66c6012 
…dProtos

Fixes bazelbuild#10484

Closes bazelbuild#10493.

PiperOrigin-RevId: 289411825
@Yannic Yannic mentioned this issue Jan 15, 2020
Closed
bazel-io pushed a commit that referenced this issue Jan 15, 2020
@Yannic @copybara-github
Rollforward of commit 4162cc5.
1821bd5 
proto_lang_toolchain: Add original sources of ProtoInfo to blacklistedProtos

This fixes the problem reported in #10588

Fixes #10484

Closes #10592.

PiperOrigin-RevId: 289857885
@bsravanin bsravanin mentioned this issue May 27, 2020
Closed
@Yannic Yannic mentioned this issue Aug 18, 2020
Merged
jpsim pushed a commit to envoyproxy/envoy that referenced this issue Nov 28, 2022
@rebello95 @jpsim
core: apply protobuf patch to prevent duplicate symbols (#618)
c708851 
Fixes envoyproxy/envoy-mobile#617, which resulted in duplicate symbols from protobuf being present in the final Envoy Mobile binary.

The [root cause](bazelbuild/bazel#10484) is being [patched in Bazel](bazelbuild/bazel#10493). In the meantime, we can revert these two commits in protobuf ([first](protocolbuffers/protobuf@7b28278), [second](protocolbuffers/protobuf@a03d332)) and apply the diff as a patch until we're able to update Bazel with the fix. More context on the protobuf issue is available [here](protocolbuffers/protobuf#7046).

Notes:
- This should be reverted after updating Bazel with the fix
- The version of protobuf specified in the `http_archive` should be in lockstep with upstream Envoy's version
- Upstream Envoy's `@envoy//bazel:protobuf.patch` patch is also included here

Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit to envoyproxy/envoy that referenced this issue Nov 29, 2022
@rebello95 @jpsim
core: apply protobuf patch to prevent duplicate symbols (#618)
4e54ca1 
Fixes envoyproxy/envoy-mobile#617, which resulted in duplicate symbols from protobuf being present in the final Envoy Mobile binary.

The [root cause](bazelbuild/bazel#10484) is being [patched in Bazel](bazelbuild/bazel#10493). In the meantime, we can revert these two commits in protobuf ([first](protocolbuffers/protobuf@7b28278), [second](protocolbuffers/protobuf@a03d332)) and apply the diff as a patch until we're able to update Bazel with the fix. More context on the protobuf issue is available [here](protocolbuffers/protobuf#7046).

Notes:
- This should be reverted after updating Bazel with the fix
- The version of protobuf specified in the `http_archive` should be in lockstep with upstream Envoy's version
- Upstream Envoy's `@envoy//bazel:protobuf.patch` patch is also included here

Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
team-Rules-Server Issues for serverside rules included with Bazel untriaged
Projects
None yet
Milestone
No milestone
3 participants
@keith @jin @Yannic