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

Feature Request: Allow generation of ProtoInfo objects without the checkSourceFilesAreInSamePackage check occurring #10744

Closed
christianladam opened this issue Feb 10, 2020 · 2 comments
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-CPP Issues for C++ rules type: feature request

Comments

@christianladam
Copy link

christianladam commented Feb 10, 2020

Description of the problem / feature request:

Presently there is no way to bypass the checkSourceFilesAreInSamePackage check during ProtoInfo creation. This differs from other rules that utilize the checkSrcsSamePackage method in RuleContext where one can configure the error to only be a warning.

Feature request: what underlying problem are you trying to solve with this feature?

In a poly-repo environment, Protos generally get created to live with the services that utilize them. To avoid proto package chains, we create a package that contains all the proto files and utilize that to build all packages. This ensures that any illegal chains are avoided, and allows easy access to cross repo protos in a single location. Each repo gets configured to point to a version of the package containing all the protos, allowing complete access of protos across repos easily.

With Bazel this system becomes an issue due to the checkSourceFilesAreInSamePackage check. If we are in repo A and make a change to a proto file (proto 1) that is a dependency of a proto (proto 2) in another repo we need to ensure that that proto 2 is built utilizing the changed proto 1. This isn’t allowed presently without checking in proto 2 to the repo with proto 1 in it. We’d like to avoid that check specifically for this scenario.

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

Create 3 protos. Proto 3 depends on a message in Proto 2 which depends on a message in Proto 1. Ensure that Proto 2 is in a separate package from Proto 3 and Proto 1 (Proto 3 and 1 should be in the same package).

What operating system are you running Bazel on?

Mac OS X

What's the output of bazel info release?

release 2.1.0-homebrew

Have you found anything relevant by searching the web?

Discussion around not exposing ProtoInfo in Starlark:
protocolbuffers/protobuf#6163 (comment)

Any other information, logs, or outputs that you want to share?

Initial ideas for options:

  1. Add an option to allow checkSourceFilesAreInSamePackage to only warn or to bypass the check entirely.
  2. Move checkSourceFilesAreInSamePackage into BazelProtoLibrary and expose ProtoCommon.createProtoInfo to Starlark. This doesn’t break existing callers and allows unchecked creation of ProtoInfo objects in Starlark.
  3. Allow ProtoInfo to be created in Starlark natively. There are previous discussions around not allowing this (for example see Update for new bazel proto API protocolbuffers/protobuf#6163 (comment)).

If there are other recommendations on how to handle this, we'd love to hear them.

@oquenchil oquenchil added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels Feb 18, 2020
@oquenchil oquenchil added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) and removed P3 We're not considering working on this, but happy to review a PR. (No assignee) labels Nov 19, 2020
@github-actions
Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 2+ years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Apr 12, 2023
@github-actions
Copy link

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please reach out to the triage team (@bazelbuild/triage). Thanks!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-CPP Issues for C++ rules type: feature request
Projects
None yet
Development

No branches or pull requests

3 participants