protobuf icon indicating copy to clipboard operation
protobuf copied to clipboard

--descriptor_set_out without --include_imports or code generation should not fail with missing import files

Open anuraaga opened this issue 2 years ago • 6 comments

What version of protobuf and what language are you using? Version: 3.21.2 Language: None

What operating system (Linux, Windows, ...) and version?

Mac OS X

What runtime / compiler are you using (e.g., python version or gcc version)

What did you do?

Proto file with imports, e.g.

import "google/api/annotations.proto";
import "google/api/field_behavior.proto";
import "google/protobuf/empty.proto";
import "google/protobuf/timestamp.proto";
import "validate/validate.proto";

protoc -odescriptors.pb hello.proto

What did you expect to see

Build succeeds to generate the descriptor for the input proto

What did you see instead?

Build fails with missing includes

import "google/api/annotations.proto";
import "google/api/field_behavior.proto";
import "google/protobuf/empty.proto";
import "google/protobuf/timestamp.proto";
import "validate/validate.proto";

I am trying to use protoc to generate machine parsable descriptors from arbitrary proto files where imports are not available. For context, I want to process the proto files in Go code. Because I am not specifying --include_imports, there doesn't seem to be a reason that this would fail as the descriptor should just be a transformation of the source file with no added semantics (e.g. loading imports).

anuraaga avatar Jul 25 '22 04:07 anuraaga

I think this could potentially be a useful feature to have, but we do need at least one level of imports to be able to interpret the source file. Given an identifier X, we often need to process the imports to determine whether X is an enum or a message.

acozzette avatar Jul 25 '22 16:07 acozzette

Agree that this would be an incredibly useful feature. Thank you for the explanation @acozzette but I am curious if a "best-available-info" descriptor could be generated?

So any objects reliant on imports would just have an unknown type?

I think there is great value in this functionality if it seems feasible.

kylepapili avatar Aug 19 '22 20:08 kylepapili

This functionality already exists to a certain extent through DescriptorPool::AllowUnknownDependencies. Currently there is no flag that will make protoc call that method, but it would be easy to tweak protoc to do that just for the purpose of experimentation. I have thought about doing this in the past because it could make our build system more efficient. We could potentially speed up builds a bit if we didn't have to rebuild a proto's descriptors every time a transitive .proto dependency changes.

acozzette avatar Aug 19 '22 20:08 acozzette

I wasn't aware of DescriptorPool::AllowUnknownDependencies, that definitely should be a fairly straight forward modification to protoc. We have a few workflows that would benefit greatly from this capability as well and I imagine many others do too.

kylepapili avatar Aug 19 '22 21:08 kylepapili

No updates on this? @acozzette

kylepapili avatar Sep 08 '22 19:09 kylepapili

No, we are not actively working on this but would be willing to review pull requests.

acozzette avatar Sep 14 '22 18:09 acozzette

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago.

github-actions[bot] avatar Jan 07 '24 10:01 github-actions[bot]

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please reopen it.

This issue was closed and archived because there has been no new activity in the 14 days since the inactive label was added.

github-actions[bot] avatar Jan 22 '24 10:01 github-actions[bot]