-
Notifications
You must be signed in to change notification settings - Fork 70
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
rules_cc dependency marked as dev causes rules_cc module visibility issues #203
Comments
cc @alexeagle @thesayyn, please triage/fix |
Known EventGroup metadata types are types expected to be known by all callers. These need not be included in the EventGroupMetadataDescriptor FileDescriptorSet, and are instead loaded separately. This also enforces uniqueness for EventGroupMetadataDescriptor message types. Note that this only affects the creation of new resources. Existing resources may exist that already violate this constraint. Includes a workaround for bazelbuild/rules_proto#203. Closes #1511
Yup, that tool is written in C++ which is naughty since we only declare that as a devDep: Line 16 in f889a1b
and many users have broken C++ toolchains. The program itself is very small https://github.com/bazelbuild/rules_proto/blob/f889a1b532fdca5f5051691f023a6a9f37ce494f/tools/file_concat/main.cc I think we should avoid having tools live in rules_proto. |
It looks like despite being reported against v6.0.0-rc2, this bug continued into both v6.0.0-rc3 and the final v6.0.0 release. Can we get a fix either in rules_proto itself (v6.0.1) or in BCR (6.0.0.bcr.1)? |
What do you propose as the fix? I suppose our only choice per semver is to make rules_cc a non-dev dependency, rather than remove this tool or ship pre-built binaries for it. |
Yes, exactly this for the short term as right now the module definition is just incorrect/broken. Anything like what was proposed in #203 (comment) would need to come later. |
Known EventGroup metadata types are types expected to be known by all callers. These need not be included in the EventGroupMetadataDescriptor FileDescriptorSet, and are instead loaded separately. This also enforces uniqueness for EventGroupMetadataDescriptor message types. Note that this only affects the creation of new resources. Existing resources may exist that already violate this constraint. Includes a workaround for bazelbuild/rules_proto#203. Closes #1511
When trying to use the following code:
This causes the following error:
The text was updated successfully, but these errors were encountered: