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

protoc-gen-go: why use import aliases? #1051

Closed
meling opened this issue Mar 5, 2020 · 4 comments
Closed

protoc-gen-go: why use import aliases? #1051

meling opened this issue Mar 5, 2020 · 4 comments

Comments

@meling
Copy link

meling commented Mar 5, 2020

The generated code uses import aliases when unnecessary, e.g.

	protoreflect "google.golang.org/protobuf/reflect/protoreflect"
	protoimpl "google.golang.org/protobuf/runtime/protoimpl"
	reflect "reflect"
	sync "sync"

So far, I've not seen any aliases with a different name than the package name itself and so these aliases seem useless.

I would not normally care about this problem, but I tried to bundle generated code with non-generated code that don't use such aliases, and the bundle tool (golang.org/x/tools/cmd/bundle) didn't recognize that the aliases were the same import as non-aliased ones, and this resulted in a list of imports with duplicates (a compile error), e.g.:

	"sync"
	sync "sync"
	"golang.org/x/net/trace"
	trace "golang.org/x/net/trace"
	"google.golang.org/grpc"
	grpc "google.golang.org/grpc"

Maybe this is the responsibility of the bundle tool, but I thought I'd ask here first: Why use import aliases when this is not necessary?

@puellanivis
Copy link
Collaborator

I’m pretty sure it is because the code is generated, we want to be absolutely sure that the package name is what we expect it to be.

I think in general, you probably don’t want to bundle generated code with non-generated code in a single file. Having your non-generated code in another file also ensures that it can be properly linted, etc.

@neild
Copy link
Contributor

neild commented Mar 5, 2020

As @puellanivis says, you can't necessarily tell what the package name is from the import path. In addition, the code generator sometimes needs to rename packages even where the package name is consistent with the import path--if you're generating a file that imports "sync" and another generated proto package named "example.com/m/sync", one of them will need to be renamed to avoid a conflict.

We could make the generator try to know when it doesn't need to explicitly name an import, but it doesn't seem worth it.

@dsnet
Copy link
Member

dsnet commented Mar 5, 2020

This seems like a bug in bundle.

The general issue for this is golang/go#29036.

@dsnet dsnet changed the title Why use import aliases? protoc-gen-go: why use import aliases? Mar 5, 2020
@dsnet
Copy link
Member

dsnet commented Mar 9, 2020

I don't think there is much to do here. We could avoid the renaming for common things (like standard library packages), but that seems like a minor improvement over the status quo. Fundamentally, the code generator needs to be a firm control over the imported package names and it can't trust what may be the target package.

@dsnet dsnet closed this as completed Mar 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants