Git Product home page Git Product logo

Comments (28)

dsymonds avatar dsymonds commented on July 18, 2024 57

The standard way for Go packages to be distinguished is to be in separate directories. The package declaration in a Go file is not the way that package boundaries are defined; the go tool demands the package names to match, but it otherwise doesn't use it to determine what constitutes a package. The other languages that ship with protoc don't care about package boundaries in the same way as Go (the mapping to C++ is simply a namespace), and so Go requires you to pass a package-worth of .proto files to protoc at a time. That is sometimes trivial (if your .proto files are already grouped into the relevant directories), but will sometimes require a bit of manual work.

from protobuf.

tamird avatar tamird commented on July 18, 2024 35

from protobuf.

neild avatar neild commented on July 18, 2024 33

New year's update:

  1. The v2 proto implementation (currently on track to release later this year) will completely fix this issue. It includes a rewritten protoc-gen-go which has no restrictions on what .proto source files can be simultaneously compiled. (You can find this version of protoc-gen-go on the api-v1 and api-v2 branches of this repository. Note that these branches are unreleased and subject to change without notice.)
  2. While the currently released protoc-gen-go doesn't let you do protoc --go_out=. **/*.proto, you can get the (probable) desired effect with for x in **/*.proto; do protoc --go_out=paths=source_relative:. $x; done. If you have cases where this doesn't produce the desired results, I'd very much like to hear about them.
  3. While not specifically about this issue, as a general rule I strongly recommend always passing the paths=source_relative option to the compiler (e.g., protoc --go_out=paths=source_relative:. foo.proto) and setting option go_package = "full/import/path/of/package"; in every .proto source file. Following these rules resolve most problems with generation that I've seen.

from protobuf.

dsymonds avatar dsymonds commented on July 18, 2024 25

Your last command seems to have been cut off.

I'm not saying that .proto files need to be grouped into directories, one package per dir. I'm saying that a single invocation of protoc should be given a single package's worth of .proto files when generating Go code. That might involve multiple invocations or some trivial shell scripting, but that's easy. I am not keen on adding more complexity to protoc-gen-go to support that use case.

from protobuf.

tamird avatar tamird commented on July 18, 2024 24

The standard way for Go packages to be distinguished is to be in separate directories. The package declaration in a Go file is not the way that package boundaries are defined; the go tool demands the package names to match, but it otherwise doesn't use it to determine what constitutes a package. The other languages that ship with protoc don't care about package boundaries in the same way as Go (the mapping to C++ is simply a namespace),

With you so far.

and so Go requires you to pass a package-worth of .proto files to protoc at a time.

This is where you lost me. That Go demands that a package is made up of files in the same directory is not the same as compiling a single package at a time. Rather, it seems that there's a requirement that each collection of files that belongs to a single package must be located in the same directory.

For these files which each belong to a different package:

        ├── extension_base
        │   └── extension_base.proto
        ├── extension_extra
        │   └── extension_extra.proto
        ├── extension_user
        │   └── extension_user.proto

It should be legal to invoke protoc --go_out=. **/*.proto and have the generated Go files mirror the layout of the proto files (both in packages and on disk).

from protobuf.

brobits avatar brobits commented on July 18, 2024 22

Google doesn't want to fix this:

I am not keen on adding more complexity to protoc-gen-go to support that use case.

the most common golang protobuf usability issue is too complex apparently. thanks to Square for providing a practical solution to a real issue. shame on golang protobuf maintainers.

from protobuf.

dmarkhas avatar dmarkhas commented on July 18, 2024 15

Any updates on this?

from protobuf.

zellyn avatar zellyn commented on July 18, 2024 11

We use https://github.com/square/goprotowrap - a wrapper that collects things into packages. Pull requests welcome :-)

The sad part is that it would be pretty straightforward to make Go protos work like other languages :-(

from protobuf.

tamird avatar tamird commented on July 18, 2024 10

I understand what you are saying, but you are just describing the status quo. Why is this not a valid use case for protoc-gen-go to support? The exact same use case is supported by the Java code generator.

from protobuf.

kingkf avatar kingkf commented on July 18, 2024 10

so, how to solve the question? now every time I generate code for golang, I need do lots of manual work to fix generated code.

from protobuf.

tigrus avatar tigrus commented on July 18, 2024 9

I wanted to play with golang and implement one of grpc services in it. Got this error, saw this thread.. This is the fastest turn around that language can provide.

from protobuf.

gardell avatar gardell commented on July 18, 2024 7

What's the status of this bug? Why is this and #67 both closed?

I have a number of protos which all depend on one proto declaring a few custom options declared in its own package. If I compile all the protos separately, I run into #67 and running them together I run into this issue. It seems like packaging protos in go is broken then?

from protobuf.

neild avatar neild commented on July 18, 2024 6

How do I add the plugins=grpc to protoc --go_out=paths=source_relative:. foo.proto

Everything before the : is a comma-separated list of options, so:

--go_out=plugins=grpc,paths=source_relative:.

from protobuf.

tandr avatar tandr commented on July 18, 2024 3

Because of this bug, Google's own Gradle's plugin for protobuf https://github.com/google/protobuf-gradle-plugin makes it impossible to build Go sources unless they are all in the same folder/package, forcing us to have all services and unrelated 'messages' under same package
Invocation of protoc from this plugin is ONE HUGE invocation with all the files, include dirs, and all the language plugins. So far C# and Java are producing something nice and usable (packages etc), but protoc-gen-go can only build one package from one directory.

from protobuf.

zellyn avatar zellyn commented on July 18, 2024 1

Yes, you're describing the problem exactly, and it appears source_relative works as I understood it from the name. We use go_package precisely to avoid this problem.

As for modifications… we forked both protoc-gen-go fairly early on, which led to people assuming a freedom to make changes that has made unforking difficult. I'm almost the whole way there. Ignoring ill-thought changes, there are two big/unavoidable ones:

  • placement of packages. This might have been before the go_package option existed, or maybe after it existed but before it could affect the output location of generated code. We modified our protoc-gen-go to place files based on the protobuf package. I only recently managed to get rid of this (I've been on a Java-only team for a couple of years in the interim) by writing a small Go tool that writes explicit go_package directives into all the .proto files before generating. Effectively, the sed solution :-)
  • The other big thing is redaction of sensitive fields. Once I finish my current stream of unforking work, we'll be on master of github.com/golang/protobuf, with only one additional small patch. I have a proposal to upstream sensitive fields, and everyone seems open, but I've been told it's likely to be too intrusive to land. I intend (at some point) so write clean patches against the main runtimes (protoc, C++, Java, Go) implementing redaction, to prove it's not intrusive, but that hasn't been a high priority so far.

from protobuf.

robpike avatar robpike commented on July 18, 2024

See my comment in the pull request.

from protobuf.

lnshi avatar lnshi commented on July 18, 2024

@tamird So, has this been fixed or not? I cannot get it work, I have some structures like this:

module-proto/
  |
  |--lib/
  |    |--bool_res.protp
  |    |--empty.proto
  |    |--mobile_phone.proto
  |    |
  |--microservices/
  |    |--auth/
  |    |    |--v1/
  |    |    |   |--auth.proto
  |    |    |--apis.proto
  |    |--sync
  |    |    |--v1/
  |    |    |   |--sync.proto
  |    |    |--apis.proto

And under the path module-proto/, tried to run protoc -I=. --go_out=plugins=grpc:. **/*.proto, keep getting error:

protoc-gen-go: error:inconsistent package names: microservice lib

Where am I doing wrong?

from protobuf.

dsnet avatar dsnet commented on July 18, 2024

I'm going through the issue tracker and figuring what needs to be done. Re-opening this so this doesn't get forgotten. This does not necessarily imply it will be implemented as every feature needs to be carefully thought through.

from protobuf.

dsnet avatar dsnet commented on July 18, 2024

Pinging this thread for subscribers to see #515.

from protobuf.

anjmao avatar anjmao commented on July 18, 2024

As a workaround I made simple interation other each proto file and called protoc compiler on that file

for filename in ${PROTO_DIR}/*.proto; do
    filename=${filename##*/} # remove path
    filename=${filename%.*} # remove extension

    rm -rf ${GO_OUTDIR}/${filename}
    mkdir ${GO_OUTDIR}/${filename}
    
    ${PROTOC} \
    --go_out=plugins=grpc:${GO_OUTDIR}/${filename} \
    -I ${PROTO_DIR} ${filename}.proto
done

from protobuf.

zellyn avatar zellyn commented on July 18, 2024

I haven't tried it, but if it does what it sounds like, I doubt using source-relative paths would work for us. We explicitly set the go_package in places to work around the fact that (only) in Go, protobuf circular dependencies break at the package level, while in every other protobuf language, they break at the file level.

from protobuf.

dsnet avatar dsnet commented on July 18, 2024

@zellyn

protobuf circular dependencies break at the package level

Can you clarify what this means? In protobuf, cyclic dependencies are only possible within a proto file. Since each proto file is generated to exactly one Go source file, that should make it impossible to have cyclicly dependent Go packages result from that.

from protobuf.

zellyn avatar zellyn commented on July 18, 2024

If a/red.proto references b/red.proto and b/green.proto references a/green.proto, that's okay for protos, C++, Java, etc. In Go, package a and package b are not allowed to reference each other that way. This is a frequent cause of problems at Square, and was one of the original reasons we forked the go protoc plugin so severely back in the day: there used to be no way to change the package into which Go code gets generated.

You would not have this problem at Google, because you use blaze for building everything, and it can create multiple distinct Go packages in the same directory, based on your BUILD rules.

from protobuf.

dsnet avatar dsnet commented on July 18, 2024

If I understand you correctly, you're describing a file tree that looks like:

a
├── red.proto   # imports b/red.proto
└── green.proto
b
├── red.proto
└── green.proto # imports a/green.proto 

However, proto-file layout does not necessary correspond with go package layout since the go_package option dictates the final Go package, and may have no relationship with the proto file layout (nor the proto package itself). I recommend that they stay related, though.

If:

  • a/red.proto and a/green.proto are generated into the same Go package, and
  • b/red.proto and b/green.proto are generated into the same Go package
    then I agree this results that this is problematic and that source_relative as a default doesn't help.

With the go_package option (which I suggest every proto file specify), this could be avoided by having the following Go package organization:

  • Each .proto file generates into its own Go package

or

  • a/red.proto and b/green.proto belong to one Go package
  • a/green.proto and b/red.proto belong to a different Go package

then this works. However, both Go package layouts would not work with source_relative, since the resulting .pb.go files need to be generating into a directory that is different from the source .proto files.

@zellyn, I'm curious. At a high-level, what modifications to protoc-gen-go did you do?

from protobuf.

zellyn avatar zellyn commented on July 18, 2024

One last random thought: that second bullet point (my current arc of unforking work) has been slow because I've been fixing every single place people use reflect.DeepEqual in tests. When they do it directly on protos, it's a simple fix, but the maps of maps of structs that have proto fields are less fun. Adding a proto-aware version of reflect.DeepEqual somewhere (in the proto package?) would be fantastic. If it did the right thing with time.Time too, even better.

Edit: it looks like github.com/google/go-cmp/cmp or a wrapper over that that adds a protobuf comparer might be what I'm looking for.

from protobuf.

rhass99 avatar rhass99 commented on July 18, 2024

New year's update:

  1. The v2 proto implementation (currently on track to release later this year) will completely fix this issue. It includes a rewritten protoc-gen-go which has no restrictions on what .proto source files can be simultaneously compiled. (You can find this version of protoc-gen-go on the api-v1 and api-v2 branches of this repository. Note that these branches are unreleased and subject to change without notice.)
  2. While the currently released protoc-gen-go doesn't let you do protoc --go_out=. **/*.proto, you can get the (probable) desired effect with for x in **/*.proto; do protoc --go_out=paths=source_relative:. $x; done. If you have cases where this doesn't produce the desired results, I'd very much like to hear about them.
  3. While not specifically about this issue, as a general rule I strongly recommend always passing the paths=source_relative option to the compiler (e.g., protoc --go_out=paths=source_relative:. foo.proto) and setting option go_package = "full/import/path/of/package"; in every .proto source file. Following these rules resolve most problems with generation that I've seen.

How do I add the plugins=grpc to protoc --go_out=paths=source_relative:. foo.proto

from protobuf.

rhass99 avatar rhass99 commented on July 18, 2024

How do I add the plugins=grpc to protoc --go_out=paths=source_relative:. foo.proto

Everything before the : is a comma-separated list of options, so:

--go_out=plugins=grpc,paths=source_relative:.

Thanks alot, works like a charm

from protobuf.

dsnet avatar dsnet commented on July 18, 2024

This should be fixed in the v1.20.0 release.

from protobuf.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.