import package alias not generated by protoc-gen-fastmarshal

[rohsaini]

Title

alias not generated in pb.fm.go file by protoc-gen-fastmarshal

Version

v0.24.3

Repro steps

I have two proto3 files =>

api/xyz/grpc/xyz.proto: option go_package = "x/y/z/grpc;xyzgrpc";

api/abc/grpc/abc.proto: import "api/xyz/grpc/xyz.proto";

Generated abc.pb.fm.go file has import statement as

import ( "x/y/z/grpc" )

Due to this, I am getting compilation error as - "x/y/z/grpc" imported as xyzgrpc and not used"

Expectation is to have import ( grpc "x/y/z/grpc" )

pb.go file generates correctly. Problem is only with fm.pb.go file.

[dylan-bourque]

@rohsaini Thanks for the bug report. Can you try it with the fix I pushed to the respect-go-package-aliases-in-addtl-imports branch? You should be able to install it with go get github.com/CrowdStrike/csproto/cmd/protoc-gen-fastmarshal@respect-go-package-aliases-in-addtl-imports.

Let me know if that works for you.

[rohsaini]

Thanks @dylan-bourque . This didn't work as expected. My abc.pb.fm.go now looks like this, which is incorrect:

package abcgrpc

import ( abcgrpc "x/y/z/grpc" )

[dylan-bourque]

Can you link to your actual code? I tried that locally and it correctly used the xyzgrpc import alias from the .proto file.

[rohsaini]

grpc_example_code.zip

Attaching the example code. xyzgrpc alias is also not correct, gives compilation error. Alias should be derived from package name itself. Please refer naming convention used in attached abc.pb.go i.e.

import ( protoreflect "google.golang.org/protobuf/reflect/protoreflect" protoimpl "google.golang.org/protobuf/runtime/protoimpl" reflect "reflect" sync "sync" grpc "xyz/grpc" )

---

My protoc version: protoc-24.0

Protoc commands: protoc --go_out=. --fastmarshal_out=apiversion=v2,paths=source_relative:. --go_opt=paths=source_relative --go-grpc_out=. --go-grpc_opt=paths=source_relative xyz/xyz.proto

protoc --go_out=. --fastmarshal_out=apiversion=v2,paths=source_relative:. --go_opt=paths=source_relative --go-grpc_out=. --go-grpc_opt=paths=source_relative abc/abc.proto

[dylan-bourque]

One thing I noticed is that your xyz.proto doesn't have a package declaration, which I thought might be what was going on. I "fixed" that, though, and the generated code is still wrong. After debugging I was able to figure out that the Protobuf file associated with your AddrInfo field is abc.proto, not xyz.proto, which is why the import alias in abc.fm.pb.go is abcgrpc.

Unfortunately, I don't have any more bandwidth to look into this now. I'll circle back to it as soon as I can.

[rohsaini]

Sure @dylan-bourque . Thanks.

[biosvs]

@dylan-bourque thanks for your quick response! Although I'm afraid you went to the wrong direction: package names already parsed and passed to plugin, so plugin only has to use this information.

I created a PR #123, feel free to merge it or to discard and use as a reference.

[dylan-bourque]

@biosvs I ended up adapting your code a bit to fit into what I had already done. My "miss" was using fld.Desc (the file descriptor associated with the field declaration) instead of fld.Message.Desc and fld.Enum.Desc (the file descriptor associated with the field's type) to locate the parent file. I definitely appreciate the input and extra eyes.

@rohsaini I tried this update against your example locally and it did the right thing as far as I can tell. Let me know if this works for you.

[rohsaini]

Thanks @dylan-bourque and @biosvs . This is working fine now. I just want to bring to your notice - o/p of pb.fm.go import is not same as pb.go file. It is upto you if you want to keep similar output or not. Things are compiling and fine otherwise.

pb.fm.go: import ( xyzgrpc "xyz/grpc" ) ... ... xyzgrpc.SomeMethod()

pb.go: import ( grpc "xyz/grpc" ) ... ... grpc.SomeMethod()

[rohsaini]

@dylan-bourque Any update please?

[dylan-bourque]

I was head-down on a project at work all of last week and didn't have time to circle back on this. I'm inclined to merge this as is since it addresses the underlying problem and exactly matching the code generated by protoc-gen-go is not one of my goals.

An argument could even be made that this code is more correct since it's using the explicit Go package name from the .proto file instead of a technically unnecessary alias that uses the last token of the import path. 😉

[rohsaini]

Sure. Please go ahead and merge.

[dylan-bourque]

@rohsaini I just merged #126. Haven't tagged a new release yet but you can grab the new build right away if you go install github.com/CrowdStrike/csproto/cmd/protoc-gen-fastmarshal@main.

[rohsaini]

@dylan-bourque Thanka Dylan. Request you to tag it, as main will keep moving forward. I would like to align to some fixed tag.

[dylan-bourque]

To avoid version churn I tag new versions on (or just after) the first of each month if there are changes to be released.

[rohsaini]

Okay. Thanks. I'll wait for next tag (whenever available) then.

[dylan-bourque]

v0.25.0 was tagged this morning and includes this change. Let me know if you find any other issues.

Thank you for the contribution.