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

Cherry-pick proto_common module #15706

Closed
wants to merge 14 commits into from

Conversation

comius
Copy link
Contributor

@comius comius commented Jun 20, 2022

No description provided.

@comius comius self-assigned this Jun 20, 2022
comius added 14 commits June 20, 2022 08:40
Renamed StrictProtoDepsViolationMessage to ProtoConstants and added protoc label there.

PiperOrigin-RevId: 409142099
…lify ProtoCompileActionBuilder.

`$(OUT)` placeholder is replaced with `%s` so it can be directly used in addFormatted. This simplifies construction of proto compile action and makes it possible for Starlark version to have the same performance.

github shows no uses of the placeholder: https://github.com/search?q=%22%24%28PLUGIN_OUT%29%22&type=Repositories

PiperOrigin-RevId: 412286743
This makes it possible to pair it with command_line attribute which could contain dependent data,
for example:

```
proto_lang_toolchain(
    name = "j2objc_proto_toolchain",
    blacklisted_protos = [],
    command_line = "--PLUGIN_j2objc_out=file_dir_mapping,generate_class_mappings:$(OUT)",
+   plugin_format_flag = "--plugin=protoc-gen-PLUGIN_j2objc=%s",
    plugin = "//third_party/java/j2objc:proto_plugin",
    runtime = "//third_party/java/j2objc:proto_runtime",
    visibility = ["//visibility:public"],
)
```

It also retains reference to the flag on proto_lang_toolchain rule and so doesn't cause a memory regression when proto_lang_libraries are Starlarkyfied.

PiperOrigin-RevId: 412287195
This necessitated changing ProtoLangToolchainProvider from native to Starlark type.

PiperOrigin-RevId: 437751442
This is needed for the proto_common.generate_code function.

This change doesn't modify any of proto_lang_toolchain public attributes.

PiperOrigin-RevId: 437713619
…ttributes.

This is needed for proto_common.generate_sources function.

Two public attributes are added to proto_lang_toolchain rule. Both have defaults, so no immediate migration of targets is needed.

PiperOrigin-RevId: 437714094
The semantics of empty descriptor set is changed, as it doesn't depend on other descriptor sets any longer. This kind of dependencies would be hard to observe by integrations and testing showed there are no such integrations.

PiperOrigin-RevId: 410501409
This will make migration to the new call easier (because we don't have progress_message set yet on the proto_lang_toolchain rules).

Design doc: https://docs.google.com/document/d/1dY_jfRvnH8SjRXGIfg8av-vquyWsvIZydXJOywvaR1A/edit

PiperOrigin-RevId: 440098602
PiperOrigin-RevId: 445149402
Adding it will make it possible to migrate the uses from proto_library_target to proto_info. When the migration is done proto_library_target will be removed.

The cost of changing this now is still low, because it's not yet released/used in Bazel.

PiperOrigin-RevId: 455616355
Change-Id: Ieb0f03b0600e1f90b72a61f90420675075c79a9e
@meteorcloudy
Copy link
Member

@comius Do we still need to cherry pick those change in to 5.3? Can you resolve all the conflicts and make sure tests pass?

@comius
Copy link
Contributor Author

comius commented Jul 25, 2022

@comius Do we still need to cherry pick those change in to 5.3? Can you resolve all the conflicts and make sure tests pass?

This is stale PR. It was my first attempt.

I believe proto_common was already cherry picked here #15854

@comius comius closed this Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants