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

Unexpected . in generated file names results in buf warning #148

Open
nahk-ivanov opened this issue Oct 28, 2022 · 2 comments
Open

Unexpected . in generated file names results in buf warning #148

nahk-ivanov opened this issue Oct 28, 2022 · 2 comments
Labels
good first issue Good for newcomers P3 triaged Issue has been triaged

Comments

@nahk-ivanov
Copy link

When using the plugin v3.21.2 through buf on Linux (Ubuntu), I'm getting the following warning:

Warning: Generated file name "./some/path/to/file/my_message_pb.js" does not conform to the Protobuf generation specification. Note that the file name must be relative, use "/" instead of "", and not use "." or ".." as part of the file name. Buf will continue without error here, but please raise an issue with the maintainer of the plugin and reference https://github.com/protocolbuffers/protobuf/blob/95e6c5b4746dd7474d540ce4fb375e3f79a086f8/src/google/protobuf/compiler/plugin.proto#L122

...which is rather self-explanatory. I suspect this might be because of the . at the beginning.

The buf command I was using is the following:

buf generate /path/to/protos --template buf.gen.yaml -o /output/location

(i.e. I used full path for both - input and output paths)

buf generation template is

version: v1
plugins:
  - name: js
    out: .
    opt: import_style=commonjs

I tried changing out to not be . to no avail.

There are no warnings when plugin is used through protoc directly, but I think the buf is right that returned file path should not have . components.

@lukesandberg
Copy link
Contributor

Interesting, looks like a simple issue to address in our code generator. maybe just remove the ./?

We do not actually test with the buf toolchain so i am not surprised there are issues there.

@lukesandberg lukesandberg added good first issue Good for newcomers P3 triaged Issue has been triaged labels Oct 31, 2022
@nahk-ivanov
Copy link
Author

I was thinking that it's an easy fix, yeah. I don't have dev environment setup, but just from static code analysis, it seems like in all places where path is generated, it is using options.output_dir, so I tried changing out in buf generation template (which supposedly translates to --js_out for protoc), but it didn't change a thing. Weird.

Ultimately you are just using provided base class for the code generator, which relies on the GeneratorContext. Every time you context->Open a file, it will add it to the response, so we just need to make sure options.output_dir does not have . or .. in it in places like this. or maybe just validate it when options are being parsed here initially.

It would be nice though, if the base class (GeneratorResponseContext) would clean up the path to make it conform to the response contract, instead of each generator dealing with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers P3 triaged Issue has been triaged
Projects
None yet
Development

No branches or pull requests

2 participants