protoc-gen-go: remove generation of magic import comment

[dsnet]

This is a reversion of #267.

The Go tooling ecosystem in moving towards modules, where this import comment is being deprecated in favor of a go.mod file. The magic // import "path/to/package" comment may be used to initialize a go.mod file, but the use of go.mod supersedes the magic comment afterwards.

We may remove this now, or wait until module support is even more solidified.

[tandr]

Or at least make it configurable somehow

[dsymonds]

This move seems premature. While the package import comments might be deprecated, they will still be effective (and not harmful!) for likely quite some time. I'd wait at least a year on this.

[neild]

Are they particularly important to generate? We only started adding them relatively recently, and anyone who does want an import comment can include a handwritten stub .go file with just a package statement and the comment.

[dsymonds]

They're useful in the ways that import comments in non-generated packages are useful: they ensure a package is only imported in a specific way (and this is doubly important for proto packages to avoid duplicate registrations!).

I could write a stub .go file, but I've already written the package name in an option go_package directive in the .proto file.

Note that they existed for a long time internal to Google. I can't remember why it wasn't generated externally for so long. But my point still stands: they're useful (that's why you added them earlier this year!), and will remain useful for quite some time. Modules aren't going to be widely adopted in the short term; it'll take some time.

[neild]

I don't think the import comments ever existed internal to Google, did they? We build everything with Blaze, so option go_package never sets the import path--everything is explicitly set on the command-line and there isn't really a concept of importing something with the wrong path.

[dsymonds]

Maybe I'm misremembering, or don't have the old emails any more.

[dsymonds]

Anyway, I don't feel particularly strongly. I think they'd be useful to keep around for the next 6-12mo, and I don't see any harm in them, but it's your call.

[puellanivis]

As I remember it, internally at Google the option go_package was never a full import statement, and was always just the final name of the package alone, i.e. it went straight into the package statement: package ${GO_PACKAGE} without any munging.

The notion of having it as a full import path just kind of weirds me out, as it locks/locked you into building out of the GOPATH, which I was never a fan of.

[neild]

Having the full import path in option go_package is important for generating accurate import statements. That is, a.proto imports b.proto, you need to know what (if any) import statement to put in a.pb.go. The best way to do that is to explicitly put that information in b.proto; the alternative is to specify all the import paths on the command line, which is error-prone and difficult to do by hand.

Internally to Google, we do pass everything on the command-line because the build system (Blaze) knows the import paths and can easily construct complex protoc invocations. Other systems build systems like Bazel presumably do the same thing. So the only thing option go_package gets used for inside Google is to set the package name (and it probably shouldn't do even that, because Blaze knows that too).

But for users who just want to run protoc --go_out=. foo.proto on the command line or from a Makefile, it's important to make things work well without needing complex invocations like protoc --go_out=Ma.proto=import/path/of/a,Mb.proto=import/path/of/b,... foo.proto. Import paths in the go_package option let us do that.

And once we know the import path, it seems reasonable to put it in a // import comment, because why not?

Anyway, I don't feel strongly about this either way. On one hand the import comment seems harmless to me; on the other hand it's trivial to drop in a stub .go file with the desired import comment and it's less simple to remove it from the generated code if you don't want it for some reason. I'll make the next comment a poll on what we should do.

[neild]

POLL: Should we keep generating the // import "/path/to/package" comment?

Thumbs up for yes, thumbs down for no, heart for throw it all out and go back to XML. :)

[puellanivis]

I know the purpose of specifying the full import, but like I said, it locked you into doing everything in the GOPATH. I was also mostly just looking to add more Google input.

Of course also in Google the option go_package was necessary when I was there, because internal Go could handle multiple packages in the same directory. (We all know why that didn’t make it out to the wild though, because importing path/to/your/package_name/package_name was super annoying. ;) )

[dsnet]

Removal made in #701 from #678 (comment) it seems that the general opinion is to remove them.