do not allow files that cross package boundaries

[jhump]

protoc-gen-go tries to disallow multiple files if they cross package boundaries -- it wants to be invoked for a single package at a time, and with all files for that package at the same time.

But it doesn't do a very good job checking for this condition. It simply verifies that the package name for all files is the same, but it does not check that all files emit code to the same package path (which is what Go really considers to be the identity of a package).

That means I can supply different files that have the same package name but live in different folders, and the generated code is simply wrong. Because it assumes they are the same package (including same package path), references from one to the other will result in code that uses naked symbol references (no package qualifier/alias) and also not produce an import statement.

So this change will cause the plugin to issue an error message if the files it produces would be in different Go packages.

[jhump]

Ping?

[neild]

This is reasonable, but I'm in the middle of some significant reworking of this part of the code. (I'd really like to make it possible to just generate output for multiple packages at once, but no promises.) Mind holding off for a bit?

[jhump]

@neild, I am happy to never merge this. I think protoc-gen-go's behavior that requires it to compile only one package and a complete package (e.g. can't produce output for single proto file at a time, if the target package results from more than one source file) is actually terrible. It is very different from how other language output works, where protoc happily transforms any number of files, regardless of package.

This patch was just to make that behavior actually work, since we ran into a situation where files had different packages, but the same name last path element. Instead of spitting out an error, the plugin just generated non-working code.

When I worked at Square, we (and by we I mean @zellyn) tried to submit issues/patches to fix that IIRC, but they weren't accepted. It required us to maintain a fork of this repo since we had build scripts that would generate code for lots of proto files (crossing packages) all at once, and generate output for multiple languages, not just Go (only Go has this quirk).

[zellyn]

We use https://github.com/square/goprotowrap. It doesn't let you generate one file at a time, but it does help you generate all the files you need at once without having to think about it.

Back when I was working on Go at Square (I got reassigned to Java-land 😢), it seemed that a few fairly simple changes to how go protobufs generate code would fairly easily clean up the file-at-a-time restriction. In particular, uniquifying symbols better within each file would probably do it. (While you're at it, you might want to make package import names sane too…)

However, getting even simple patches (or indeed even ideas for changes: I'm fine if the maintainers don't like my implementation) against the Go protobuf code base accepted proved completely futile, and we gave up.

[awalterschulze]

@zellyn Please bring your ideas. This is a new team maintaining golang/protobuf. They listen to ideas and are really trying to fix things.

[jhump]

@awalterschulze, I remember a couple of them. One of them is written up in #141.

The other was embedding the proto source file name into the fileDescriptor* names, instead of just a numeric suffix that always started at 0. That would allow compiling two source files for the same package in separate invocations of protoc and still getting deterministic output that compiles. (Currently, that produces code that won't compile because each output file will have a package-level var with the same name, like fileDescriptor0.)

[awalterschulze]

Oh wow I remember that the fileDescriptor one.
I think gogoprotobuf ended up going for the filename instead of the number.
That was quite a while ago.

[zellyn]

Heh. I forgot about #141. That is a pretty good writeup. Good work, past self! :-)

[neild]

FYI, #549 will change protoc-gen-go to enforce that all generated files are under the same import path.

(I'm also making progress towards removing the need for that requirement entirely, but it's not there yet.)