Arwalk / zig-protobuf

a protobuf 3 implementation for zig.
MIT License
188 stars 20 forks source link

Are transitive imports supported? #50

Closed inge4pres closed 1 week ago

inge4pres commented 1 week ago

hello πŸ‘‹πŸΌ

I am trying to generate code from the OpenTelemetry protobuf definitions.

When configuring a subset of protobuf files to be processed, I encounter a "File not found" error from protoc. I guess this is because there is no way to specify the "--proto-path=" option in the protoc command run on generation?

That option seems to be needed to resolve imports.

For completeness, my build config is below. I have a submodule in the proto-src/ directory, and with/without absolute paths, the import in https://github.com/open-telemetry/opentelemetry-proto/blob/main/opentelemetry/proto/resource/v1/resource.proto#L19 is not resolved.

    const protoc_step = protobuf.RunProtocStep.create(b, protobuf_dep.builder, target, .{
        // Output directory for the generated zig files
        .destination_directory = b.path("src/model"),
        .source_files = &.{
            // Add more protobuf definitions as the API grows
            b.pathJoin(&.{ wd, "proto-src/opentelemetry/proto/common/v1/common.proto" }),
            b.pathJoin(&.{ wd, "proto-src/opentelemetry/proto/resource/v1/resource.proto" }),
            b.pathJoin(&.{ wd, "proto-src/opentelemetry/proto/metrics/v1/metrics.proto" }),
        },
        .include_directories = &.{
            // Don't forget to add the relevant folder here as well
            b.pathJoin(&.{ wd, "proto-src/opentelemetry/proto/common/v1" }),
            b.pathJoin(&.{ wd, "proto-src/opentelemetry/proto/resource/v1" }),
            b.pathJoin(&.{ wd, "proto-src/opentelemetry/proto/metrics/v1" }),
        },
    });

WDYT? Thanks πŸ™πŸΌ

inge4pres commented 1 week ago

Maybe I'm also confused, apparently -I is equivalent to --proto_path, but when running protoc externally with the added flag it fails anyway 🀷🏼

Arwalk commented 1 week ago

Hello @inge4pres, thank you for your issue.

Using another generator with protoc, do you manage to generate the thing you'd like to to have using zig-protobuf ? Which flags were necessary for it?

We have an include_directory option in RunProtocStep, that should use the -I flag in the background.

If you have small project strapped of anything unnecessary that reproduces the problem, it'd give us more way to help you.

Also, with the verbose flag, you should be able to see the exact protoc command used by RunProtocStep

Thanks again.

inge4pres commented 1 week ago

Also, with the verbose flag, you should be able to see the exact protoc command used by RunProtocStep

Hey @Arwalk thanks for the hint, the verbose is not adding any more useful information to the already printed output. Though I guess since it's failing to build even when calling protoc directly, it's not an issue in this library but a problem in how protoc expects includes...

I will close this issue and post back once I find the solution to the protoc error πŸ‘πŸΌ

inge4pres commented 6 days ago

I confirm the problem is not in zig-protobuf, rather in how include_directories should be used when imports are declared in protobuf files. protoc requires that imports are expressed as relative paths to --proto_path (or -I).

For reference, the solution for me was to use include_directories as follows:

        .include_directories = &.{
            "proto-src/",
        },

In the end, relative paths are working just fine, and there's no need to use absolute paths by adding the build working directory.

Arwalk commented 5 days ago

Thank you for your analysis and your time. I really appreciate it.