proto: should not print to fmt or log

[dsnet]

$ rgrep --exclude "*_test.go" "log.Print"
equal.go:226:	log.Printf("proto: don't know how to compare %v", v1)
equal.go:279:			log.Printf("proto: don't know how to compare extension %d of %v", extNum, base)
equal.go:291:			log.Printf("proto: badly encoded extension %d of %v: %v", extNum, base, err)
properties.go:482:		log.Printf("proto: duplicate proto type registered: %s", name)
properties.go:503:		log.Printf("proto: duplicate proto type registered: %s", name)
clone.go:234:		log.Printf("proto: don't know how to copy %v", in)
lib.go:671:			log.Printf("proto: can't set default for field %v (sf.kind=%v)", f, sf.kind)
lib.go:741:			log.Print(err)
text.go:156:		log.Print("proto: textWriter unindented too far")

It is generally considered a code smell when a library prints to log. We should investigate whether these are actually necessary and remove them or turn them into errors.

[kuba--]

Can we suppress these warnings (at least this one:
properties.go:503: log.Printf("proto: duplicate proto type registered: %s", name)
)
Or maybe we can generate ifs in init function to check if type is already registered (instead of printing this warning).
In my case, I use 2 packages (client and server) in one app. and they share/register the same types, so when packages initialise it prints these messages on start of my app.

[dsnet]

What was the rationale for having the same .proto file generated into two different Go packages, as opposed to just being generated into a single Go package that is depended upon by both the client and server?

The .proto registration system is built upon the assumption that a single message is linked into the Go binary once.

[kuba--]

Our app. uses two 3rd-party packages. One is a http client and the second one is a server. Both register the same types. In our app. we vendor both packages, because some users may want to talk over http, where others may prefer having embedded server.

[kuba--]

In other words, I'm not against warnings, I'd like to have a option to suppress them by setting log level or by passing own logger.

[dsnet]

What you are describing seems to be a case of the logging statement correctly complaining about a real issue. In order for some parts of the protobuf ecosystem to work correctly (e.g., jsonpb or the Any type), registration needs to be conflict free.

Several things to note:

• What you are hitting is a known failure mode of vendoring, where the same Go package can end up getting linked in the same binary multiple times. With the upcoming module support, vendoring will be gradually deprecated, and hopefully registration conflicts as a result of vendoring will decline.
• Rather than skirt around the issue of conflicting registration by ignoring the error, we may want to provide an option in the .proto file to disable registration, however, that is beyond the scope of this issue.

I'd like to have a option to suppress them by setting log level or by passing own logger.

That is one possibility, but I'm hesitant to add API for something that is actually a problem.

[puellanivis]

:/ With go modules, we will also however begin allowing separate major versions of proto packages. Will the full module+package name be used or the package name alone? If the later, then modules will not resolve this issue, but could even exacerbate the issue?

[dsnet]

@puellanivis, I'm not sure I follow the concern. Is the issue regarding registration and major version change of generated protos? Can you mention it on #268, and we can discuss further there. Let's keep this issue about the log.Print statements.

[dsnet]

This is fixed in the upcoming v1.4.0 release.