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

reflect/protoregistry: conflicts with same filename #1122

Closed
guyguy333 opened this issue May 10, 2020 · 67 comments
Closed

reflect/protoregistry: conflicts with same filename #1122

guyguy333 opened this issue May 10, 2020 · 67 comments
Milestone

Comments

@guyguy333
Copy link

Hello,

Sharing a same .proto filename with different proto package names results in these errors:

2020/05/10 15:03:12 WARNING: proto: file "common.proto" is already registered previously from: "git.example.com/proto/common" currently from: "git.example.com/proto/a" A future release will panic on registration conflicts. See: https://developers.google.com/protocol-buffers/docs/reference/go/faq#namespace-conflict

2020/05/10 15:03:12 WARNING: proto: file "common.proto" is already registered previously from: "git.example.com/proto/common" currently from: "git.example.com/proto/b" A future release will panic on registration conflicts. See: https://developers.google.com/protocol-buffers/docs/reference/go/faq#namespace-conflict

Here is my layout:

proto/
  common/
    common.proto (package common)
  a/
    common.proto (package a and importing common/common.proto)
    foo.proto (importing common.proto)
    ...
  b/
    common.proto (package b and importing common/common.proto)
    bar.proto  (importing common.proto)
    ...

I don't think my case is similar to both quoted in documentation https://developers.google.com/protocol-buffers/docs/reference/go/faq#namespace-conflict. I don't generate multiple times a same proto file (here I generate one time common/common.proto and I import from others files) and I'm using different package names. It would be better if I rename package common to something else, but I don't think it's my issue here.

What's the best practice to solve this issue ? Is it normal to get this warning in my case ? I mean, it doesn't look uncommon to have the same filename in different projects.

Thanks

@neild
Copy link
Contributor

neild commented May 11, 2020

I don't see any good answers here, so we're probably stuck picking from bad ones. There is a naming conflict which will prevent the two different common.proto files from appearing in the same dependency tree, but it is plausible that they will never do so.

The options I see are to continue to mandate that filenames must be globally unique (do any other implementations require this?), or to permit a conflict. If we permit a conflict, we need to figure out what (*protoreflect.Files).FindDescriptorByName returns for a conflicting name. Probably an error.

@dsnet
Copy link
Member

dsnet commented May 12, 2020

Is there any reason why a/common.proto can't simply be compiled as a/common.proto and b/common.proto simply be compiled as b/common.proto? That seems like it would avoid the problem altogether.

@neild
Copy link
Contributor

neild commented May 12, 2020

It seems plausible that someone might want to link packages outside their control that both contain a common.proto, though.

@dsnet
Copy link
Member

dsnet commented May 12, 2020

It seems plausible that someone might want to link packages outside their control that both contain a common.proto, though.

Sure, but that doesn't seem to be problem in this issue though.

In my opinion, the protobuf developer site is lacking guidance in general in this area, and the protoc tool itself is not particularly friendly towards users who do not work in a mono-repo like Google does (where every .proto source file is relative to the root of the mono-repo). The open-source world is fundamentally a collection of disjoint repositories. There needs to be better guidance on what is the appropriate import path for a .proto file and how to reference a .proto file that crosses repo boundaries.

@guyguy333
Copy link
Author

Is there any reason why a/common.proto can't simply be compiled as a/common.proto and b/common.proto simply be compiled as b/common.proto? That seems like it would avoid the problem altogether.

We can do that and yes it will avoid the problem. To be more accurate, A and B are two micro services and we can see common as an util library. We considered it was ok to include a directory directly in include path when we generate A proto Go files. As we can see common dir as an "external lib", we decided to include only proto dir (to avoid "import" conflicts) to have a prefix (common in our case).

I think we can compare this issue to a project (whatever the language) doing internal import not prefixed with full path and external import mainly using full path.

@dsnet dsnet changed the title Registration conflicts: same filename reflect/protoregistry: conflicts with same filename Jun 1, 2020
@trinitum
Copy link

trinitum commented Jul 8, 2020

Is there any reason why a/common.proto can't simply be compiled as a/common.proto and b/common.proto simply be compiled as b/common.proto? That seems like it would avoid the problem altogether.

@dsnet may I ask how would one compile a/common.proto as a/common.proto? I've run into a similar problem and when I run protoc --go_out=paths=source_relative,plugins=grpc:a a/common.proto it still warns about the conflict with common.proto from another package.

@strowk
Copy link

strowk commented Jul 15, 2020

I also had similar problem and what I don't understand is how it does not take "package" into account. In my case I have two events.proto files in two different folders, they generate events.pb.go , also in two different folders AND both of them have different "package" and go_package. Why do I get proto: file "events.proto" is already registered? Is this a bug in protobuf or I don't understand that documentation - https://developers.google.com/protocol-buffers/docs/reference/go/faq#namespace-conflict ?
Particulary that

Every protobuf declaration (e.g., enums, enum values, or messages) has an absolute name, which is the concatenation of the package name with the relative name of the declaration in the .proto source file

sounds like I should not have this problem, yet warning is printed

@neild
Copy link
Contributor

neild commented Jul 15, 2020

The problem is a conflict in the source file name.

The protoregistry package permits looking up a .proto source file by name. If there are two files with same name and path, it has no way to pick between the them.

This can also be a problem when running protoc; if a .proto file includes

import "events.proto";

and there are two different "events.proto" files with no additional path prefix to distinguish them, the import is ambiguous.

I suspect that looking up .proto files by name is sufficiently uncommon that we should probably relax this check and return an error from (*protoreflect.Files).FindFileByPath when there is ambiguity.

@luisguillenc
Copy link

I have solved the problem including all the path (relative to gopath) in the "import" and compiling with protoc using the absolute path to the proto file.

After this, in the pb files generated, in the grpc.ServiceDesc structure, Metadata field contains the full path (relative to gopath) and works without warnings.

@rcgoodfellow
Copy link

So we have the situation @dsnet describes. We have Go program that depends on two external libraries both of which have a spec.proto and we get this warning when the program runs

2020/08/11 11:20:11 WARNING: proto: file "spec.proto" is already registered
A future release will panic on registration conflicts. See:
https://developers.google.com/protocol-buffers/docs/reference/go/faq#namespace-conflict

I agree with @strowk, at the very least there should be some second level logic based on package name to disambiguate source file name collisions. The workaround of changing how the protobuf is compiled does not work when an external library is managing how the protobuf is compiled.

Along the current logical arc, the implication is that all protobufs in all repositories globally, need to have unique filenames - which is not reasonable. I really hope the threat of panic in future releases does not come to fruition.

@johnsiilver
Copy link

I receive these warnings now on and older project I just came back to after a year. This project had been having no issues but, since I've come back I've made the following updates:

  • using go.mod instead of gopath
  • using a new protoc
  • using a new grpc protoc lib
  • using the old proto package that is now using the new proto package underneath
  • added go_package to my protos that I did not use before

This is happening in a layout model I was using that had components setup such as:

componentA/
io/
io.proto
componentB/
io/
io.proto

So now I'm getting the: WARNING: proto: file "io.proto" is already registered, A future release will panic on registration conflicts

I also moved to using go_package as well with dsymonds overloading setup, for example:

package marmot.example.scripts.juniper_password_change.ssh;

option go_package = "github.com/bearlytools/bearmetal/framework/examples/scripts/juniper_password_change/cogs/ssh/io;io";

I'm sure I haven't thought of a million different ways this could break or is undesirable, but could we avoid this conflict by registering using the go_package full path with a flag passed, like --register_using_go_package? I'm sure this has some undesirable behavior, but I'm 100% sure neild is right, that there is probably just no great solution.

@jabersmith
Copy link

Has the protobuf team made any further decisions on this issue?

Much like @rcgoodfellow, I have a Go program that depends on two third-party libraries, each of which defines a telemetry.proto file at the top level of their project. With current google.golang.org/protobuf, the program panics unless I set the GOLANG_PROTOBUF_REGISTRATION_CONFLICT environment variable.

This can't be that uncommon a situation. At this point, I'm not really clear how the protobuf team expect the file-by-path registry to work outside of a monorepo setting.

@masterclock
Copy link

I have the same issue too.
just like @jabersmith, I create a repo A with two external dependencies B and C, both B and C provides source code compiled from file named as principal.proto, which causes the panic threat.
note:

  1. the two principal.proto are completely different, with different package and go_package
  2. I don't have the .proto files, only the compiled go code

It seems unacceptable to got this panic threat from protobuf.

  1. as protobuf is namespaced, everything should work namespaced.
  2. It's a totally different situation as get conflict when compiling .proto files with protoc, otherwise I can simply change file name.

Is there any solution?

bretmckee added a commit to bretmckee/microservice that referenced this issue Feb 1, 2021
It seems like a proto library problem that we were getting these
warnings, but we were.

See golang/protobuf#1122 for more details. At
the time of this commit that issue is still open, but since I'm renaming
the files it should fix our problem.
@dsnet dsnet added this to the unplanned milestone Mar 29, 2021
@masterclock
Copy link

finally, I got the conflict panic!
And l'd like to paste some doc from protobuf here:
doc from panic

All protocol buffers declarations linked into a Go binary are inserted into a global registry.

Every protobuf declaration (e.g., enums, enum values, or messages) has an absolute name, which is the concatenation of the 

package name with the relative name of the declaration in the .proto source file (e.g., 

my.proto.package.MyMessage.NestedMessage). The protobuf language assumes that all declarations are universally unique.

as the doc states: The protobuf language assumes that all declarations are universally unique.
note: all declarations not file names.
Please consider fix it.

@davidandradeduarte
Copy link

davidandradeduarte commented Apr 22, 2021

This is really becoming an issue.
I also don't get why would it look/care for the proto filename.
I'm using different package names for the protobuf and still have the issue complaining about proto file being registered twice.

My protos structure:
service-a/entities.proto
service-b/entities.proto

different option go_package = "something"; in both of them.
If I include both packages I get: panic: proto: file "entities.proto" is already registered

I know I can solve this with absolute paths but it's definitely not ideal.

@gunjanpatidar
Copy link

Agree with previous comments. Not everyone uses a monorepo, and enforcing unique file path across different services is unnecessary and troublesome.

@rajmaniar
Copy link

rajmaniar commented May 5, 2021

We're coming up this issues' first birthday 🍰
Any resolution in sight?

@jskcnsl
Copy link

jskcnsl commented May 7, 2021

I tried to update protobuf and golang yesterday, and I met this issue, on its birthday...
I can understand the limitation about the namespace, but give people a chance to choose, please.

Just an option for namespace in proto file?

@ForsakenHarmony
Copy link

Ran into this problem as well, tried adding a unique namespace to the two, but that didn't help either

How can you generate the go files with a unique inlined path?

@sneko
Copy link

sneko commented May 12, 2021

Different package YYYY and option go_package = XXX in multiple .proto on my end... but same as above:

WARNING: proto: file "service.proto" is already registered

I don't understand why the file name is a problem if content is scoped to a package...

Does any of you have a reasonable solution? Should I modify all files (but it's not a proper solution), or cross fingers so their panic warnings will never be implemented?

Thank you,

@Warashi
Copy link

Warashi commented May 18, 2021

I met this issue.

(*google.golang.org/protobuf/reflect/protoregistry.Files).RegisterFile has this comment, but it is wrong?

It is permitted for multiple files to have the same file path.

https://github.com/protocolbuffers/protobuf-go/blob/0e358a402f994eaf96a258b7c5c5b3317d4575aa/reflect/protoregistry/registry.go#L110

@haswalt
Copy link

haswalt commented May 18, 2021

I've hit hit issue recently. First I followed what documentation I could find about namespacing:

https://developers.google.com/protocol-buffers/docs/reference/go/faq#namespace-conflict

I followed the guide, specifically the bit Missing or generic proto package names which implies that having a unique package name should be sufficient and is in fact recommended. This does not actually say that the files must also be uniqely named.

We're running mutliple services each with a service.proto but scoped to their own package name. This should work as far as I can see from the docs but it emits the mentioned warning.

If this becomes a panic later I can see this breaking a huge number of projects around the open source community due to assumption (based on official guides) that a unique package name is sufficient to namespace a service.

@guyguy333
Copy link
Author

I've hit hit issue recently. First I followed what documentation I could find about namespacing:

https://developers.google.com/protocol-buffers/docs/reference/go/faq#namespace-conflict

I followed the guide, specifically the bit Missing or generic proto package names which implies that having a unique package name should be sufficient and is in fact recommended. This does not actually say that the files must also be uniqely named.

We're running mutliple services each with a service.proto but scoped to their own package name. This should work as far as I can see from the docs but it emits the mentioned warning.

If this becomes a panic later I can see this breaking a huge number of projects around the open source community due to assumption (based on official guides) that a unique package name is sufficient to namespace a service.

Warning is already a panic with latest release 😕

It has been introduced in release v1.26.0 : https://github.com/protocolbuffers/protobuf-go/releases/tag/v1.26.0 (Fatal namespace conflicts part)

@johnsiilver
Copy link

@dsnet @neild It seems that the latest release went ahead and added enforcement as the default behavior. This of course broke code and we will have to go back and mitigate via env flags or build flags.

There could be a million reasons Go team has decided to do this, so this isn't a criticism and like everything else, people aren't going to be happy with the decision. I'm hoping you could address this issue here to let us know that this is just the way it is and close the issue or that there is some other way you plan to fix this in the future (by adding enforcement, this seems like the issue really should just be closed).

I would just like to move this issue towards resolution so we are all on the same page.

For anyone reading, your mitigations are:

go build -ldflags "-X google.golang.org/protobuf/reflect/protoregistry.conflictPolicy=warn"

Env var:
GOLANG_PROTOBUF_REGISTRATION_CONFLICT=warn ./main

@Warashi
Copy link

Warashi commented May 19, 2021

Instead of using only the file path, use the concatenation of the package name with the file path can partially resolve this issue?
I think this way resolves many of the case, but protoregistry's exported interface needs breaking change.

@hdevalence
Copy link

Along the current logical arc, the implication is that all protobufs in all repositories globally, need to have unique filenames - which is not reasonable.

This remains just as bizarre and unreasonable today as it was two years ago.

bretmckee added a commit to bretmckee/microservice that referenced this issue Apr 11, 2023
It seems like a proto library problem that we were getting these
warnings, but we were.

See golang/protobuf#1122 for more details. At
the time of this commit that issue is still open, but since I'm renaming
the files it should fix our problem.
@nikandfor
Copy link

Hi, I've experienced this problem too. I have two Go modules with two *.proto files in different locations inside the modules. The only common they have is the basename.
Both are generated by go:generate from *.go file in the same folder as protocol.proto file.
So I have

module_A/path_a/protocol.proto
module_A/path_a/generate.go

module_B/path_b/protocol.proto
module_B/path_b/generate.go

Generate commands are the same and looks like

protoc --go_opt=module=module_A_or_B --go_out=. protocol.proto

And when I run the command which imports both of this packages it panics.

panic: proto: file "protocol.proto" is already registered
	previously from: "module_A/path_a"
	currently from:  "module_B/path_b"
# and so on

So it knows the Go path of both packages, it knows they are different, but when it registers files it checks only relative path from protoc command executions.

I've made a little change and it seems solved the problem.

air@22:40:00:protobuf-go$ head -n 1 go.mod
module google.golang.org/protobuf
air@22:40:06:protobuf-go$ git diff
diff --git a/internal/filedesc/desc.go b/internal/filedesc/desc.go
index 7c3689b..6b8b8a3 100644
--- a/internal/filedesc/desc.go
+++ b/internal/filedesc/desc.go
@@ -72,7 +72,7 @@ func (fd *File) Options() protoreflect.ProtoMessage {
        }
        return descopts.File
 }
-func (fd *File) Path() string                                  { return fd.L1.Path }
+func (fd *File) Path() string                                  { return fd.fileRaw.builder.GoPackagePath }
 func (fd *File) Package() protoreflect.FullName                { return fd.L1.Package }
 func (fd *File) Imports() protoreflect.FileImports             { return &fd.lazyInit().Imports }
 func (fd *File) Enums() protoreflect.EnumDescriptors           { return &fd.L1.Enums }

Is this change reasonable?

@kom0055
Copy link

kom0055 commented Jun 15, 2023

Hi, I've experienced this problem too. I have two Go modules with two *.proto files in different locations inside the modules. The only common they have is the basename. Both are generated by go:generate from *.go file in the same folder as protocol.proto file. So I have

module_A/path_a/protocol.proto
module_A/path_a/generate.go

module_B/path_b/protocol.proto
module_B/path_b/generate.go

Generate commands are the same and looks like

protoc --go_opt=module=module_A_or_B --go_out=. protocol.proto

And when I run the command which imports both of this packages it panics.

panic: proto: file "protocol.proto" is already registered
	previously from: "module_A/path_a"
	currently from:  "module_B/path_b"
# and so on

So it knows the Go path of both packages, it knows they are different, but when it registers files it checks only relative path from protoc command executions.

I've made a little change and it seems solved the problem.

air@22:40:00:protobuf-go$ head -n 1 go.mod
module google.golang.org/protobuf
air@22:40:06:protobuf-go$ git diff
diff --git a/internal/filedesc/desc.go b/internal/filedesc/desc.go
index 7c3689b..6b8b8a3 100644
--- a/internal/filedesc/desc.go
+++ b/internal/filedesc/desc.go
@@ -72,7 +72,7 @@ func (fd *File) Options() protoreflect.ProtoMessage {
        }
        return descopts.File
 }
-func (fd *File) Path() string                                  { return fd.L1.Path }
+func (fd *File) Path() string                                  { return fd.fileRaw.builder.GoPackagePath }
 func (fd *File) Package() protoreflect.FullName                { return fd.L1.Package }
 func (fd *File) Imports() protoreflect.FileImports             { return &fd.lazyInit().Imports }
 func (fd *File) Enums() protoreflect.EnumDescriptors           { return &fd.L1.Enums }

Is this change reasonable?

In your protos, two files should declare in two namespace or package, like this

package module_A.path_a;
package module_B.path_b;

Hope it might help

@kom0055
Copy link

kom0055 commented Jun 15, 2023

Hi, I've experienced this problem too. I have two Go modules with two *.proto files in different locations inside the modules. The only common they have is the basename. Both are generated by go:generate from *.go file in the same folder as protocol.proto file. So I have

module_A/path_a/protocol.proto
module_A/path_a/generate.go

module_B/path_b/protocol.proto
module_B/path_b/generate.go

Generate commands are the same and looks like

protoc --go_opt=module=module_A_or_B --go_out=. protocol.proto

And when I run the command which imports both of this packages it panics.

panic: proto: file "protocol.proto" is already registered
	previously from: "module_A/path_a"
	currently from:  "module_B/path_b"
# and so on

So it knows the Go path of both packages, it knows they are different, but when it registers files it checks only relative path from protoc command executions.

I've made a little change and it seems solved the problem.

air@22:40:00:protobuf-go$ head -n 1 go.mod
module google.golang.org/protobuf
air@22:40:06:protobuf-go$ git diff
diff --git a/internal/filedesc/desc.go b/internal/filedesc/desc.go
index 7c3689b..6b8b8a3 100644
--- a/internal/filedesc/desc.go
+++ b/internal/filedesc/desc.go
@@ -72,7 +72,7 @@ func (fd *File) Options() protoreflect.ProtoMessage {
        }
        return descopts.File
 }
-func (fd *File) Path() string                                  { return fd.L1.Path }
+func (fd *File) Path() string                                  { return fd.fileRaw.builder.GoPackagePath }
 func (fd *File) Package() protoreflect.FullName                { return fd.L1.Package }
 func (fd *File) Imports() protoreflect.FileImports             { return &fd.lazyInit().Imports }
 func (fd *File) Enums() protoreflect.EnumDescriptors           { return &fd.L1.Enums }

Is this change reasonable?

What's you pwd when you generate proto.go files?
For example, if you have repo/a/api.proto and repo/b/api.proto, you should execute generator in repo path. It means that you could not execute in repo/a or repo/b .
I met this problem before, but solve it now.
You could take a look of my makefile:

commonv1alpha1:
	for file in $(COMMON_V1ALPHA1_DIR)/*.proto; do \
  		protoc \
  		--proto_path="${PROTO_ROOT_PATH}" \
		--go_out=${PROTO_ROOT_PATH} \
  		--go-grpc_out=${PROTO_ROOT_PATH} \
  		--validate_out="lang=go:${PROTO_ROOT_PATH}" \
  		$${file} \
  		; \
  	done

@nikandfor
Copy link

package module_A.path_a;
package module_B.path_b;

package directives are already different and that doesn't help.

What's you pwd when you generate proto.go files?

pwd is local directory. But this is not the point. I found a way to avoid the panic, but it's not right.

The same relative file names should not be a problem. Shouldn't we find the reason and fix it once for ever and for everybody?

@neild
Copy link
Contributor

neild commented Jun 15, 2023

The right place to bring this up is either the general protobuf issue tracker (https://github.com/protocolbuffers/protobuf/issues) or mailing list (https://groups.google.com/g/protobuf).

The question of conflicting .proto source file names is not unique to the Go protobuf implementation, and the Go protobuf implementers don't have any ability to change the current behavior independently of the general protobuf project. Any change will need to come from the general protobuf maintainers.

@edocevol
Copy link

The right place to bring this up is either the general protobuf issue tracker (protocolbuffers/protobuf/issues) or mailing list (groups.google.com/g/protobuf).

The question of conflicting .proto source file names is not unique to the Go protobuf implementation, and the Go protobuf implementers don't have any ability to change the current behavior independently of the general protobuf project. Any change will need to come from the general protobuf maintainers.

protocolbuffers/protobuf#15122

google/protobuf refers us to the maintainers of protocolbuffers, but then they refer us back to you.
Isn't registry the implementation for Golang? If it's allowed to modify variables through build tag, why must there be a panic? Does it have a significant relationship with the implementations in other languages? It seems more like a poorly implemented registry.

@neild
Copy link
Contributor

neild commented Feb 23, 2024

I added a comment on protocolbuffers/protobuf#15122.

The rejection of naming conflicts is a general requirement for protobuf implementations. Not every runtime rejects conflicts, but I believe the protobuf owners consider the cases which do not to be buggy.

@hdevalence
Copy link

Prost (the Rust protobuf implementation) handles this just fine.

As I mentioned upthread a year ago, the requirement that all source filenames are globally unique across all software libraries is simply not a reasonable assumption.

Perhaps this works for Google, but out in the real world, not everything is inside one monorepo under one company's control.

@dsnet
Copy link
Member

dsnet commented Feb 23, 2024

As mentioned by @neild, this is an issue that needs to be addressed at the https://github.com/protocolbuffers/protobuf level. Repeatedly bringing up this issue in this repo won't do much good.

@hdevalence
Copy link

As noted on that comment, there are in fact other options available:

Not every protobuf runtime enforces this requirement. Some ignore conflicts, some print a warning at startup.

Nothing other than the choices of this library's maintainers prevents them from fixing this problem. They should be honest about that choice, rather than punting responsibility off to another team.

@dsnet
Copy link
Member

dsnet commented Feb 23, 2024

Keep in mind that the Rust implementation is not maintained by Google, so it doesn't adhere to the level of expectations that the protobuf project has for languages with first-class support. It also doesn't help that protobuf doesn't have a formal specification for implementations (whether formally maintained by Google or not) to agree on what is "correct semantics".

I can say this transparently as an outsider (since I don't work for Google anymore) that the Go protobuf maintainers are ultimately under the authority of the wider protobuf project in terms of what semantics and features they an provide. Complaining about the problem here is not going to change anything as they don't have the authority to make this change. Protobuf has gone through some leadership changes since I was involved with the Go protobuf implementation, so they may feel differently today, but the stance several years ago is that the rejection of duplicates is correct behavior. Again, this needs to be brought up at the https://github.com/protocolbuffers/protobuf level.

@hawkw

This comment was marked as abuse.

@johnsiilver
Copy link

johnsiilver commented Feb 24, 2024 via email

@xuxiangyang
Copy link

Any resolution after 4 years?

In real world, many companies may has some projects, like this

project_a/protos/error.proto has package project_a.error and the go code generate under the project_a mod
project_b/protos/error.proto has package project_a.error and the go code generate under the project_b mod

why we must ensure these proto must has uniq name since these protos has complete different package and go mod ?

@puellanivis
Copy link
Collaborator

So, recently, I’ve come into some work with these specifics. It can often happen that because of -Ispecific/path/to/proto foo.proto creates a situation where the only identifier left to the libraries is foo.proto not the fully specified path.

I don’t have answers, and I don’t have solutions here either, but in some cases we’re losing specificities of filepath in how we run the protoc commandline.

@neild
Copy link
Contributor

neild commented Apr 9, 2024

I will attempt to summarize.


Every .proto source file can be represented as a FileDescriptorProto, as defined in google/protobuf/descriptor.proto. The protobuf compiler, protoc, converts .proto files into FileDescriptorProtos.

The FileDescriptorProto contains the file name of the source .proto file that created it. This name is assigned by protoc.

The FileDescriptorProto is part of a protobuf object definition's metadata, and can be acquired at runtime. In Go, this is a protoreflect.FileDescriptor.

It is possible to look up a file descriptor by file name. In Go, this is protoregistry.FindFileByPath. FindFileByPath does not allow for multiple files with the same name; it assumes that every file name is unique.

While protoregistry.FindFileByPath is a Go API, there are equivalents in C++, Java, and other languages. None of them, so far as I know, allow for multiple files with the same name.

The Go protobuf runtime will panic at init time if there are multiple file descriptors in the global registry with the same file name.


Why does the Go runtime panic on filename conflicts?

A .proto file's name (as understood by protoc) is a primary key for its file descriptor. If multiple files have the same name, then looking up files by name breaks. The protobuf maintainers consider this to be sufficient reason to forbid naming conflicts.

I disagree with that reasoning. How do I get you to change your mind?

This decision is from the overall protobuf maintainers, not the maintainers of the Go implementation. If you would like to argue for a change, the place to do so is with https://github.com/protocolbuffers/protobuf. Nobody working on the Go implementation has any ability to change this.

I have project1/foo.proto and project2/foo.proto. Why is there still a conflict when the names are different?

You're probably invoking protoc with something like this:

protoc -Iproject1 project1/foo.proto
protoc -Iproject2 project2/foo.proto

When you pass protoc a -I or --proto_path option, it considers the canonical name for any files under that directory to be relative to the directory. So protoc -Iproject1 project1/foo.proto is going to assign the name foo.proto to the file project1/foo.proto, and the above commands generate two files with the same file name in their metadata.

If you instead invoke proto without the -I, you will avoid the conflict:

protoc project1/foo.proto
protoc project2/foo.proto

I have two completely unrelated packages that each contain a common.proto. How do I avoid a conflict?

The requirement that file names in the metadata be unique means that you should generally aim to give files in public packages names that are unlikely to be used by other packages. For example, all the .proto files in the main protobuf project have a google/protobuf prefix.

I don't know of any canonical guidance on how to choose a non-conflicting name. You could use your project name if it is sufficiently unique, or use a Java-style URL-based naming scheme such as com/mydomain/common.proto.

Does this mean I need to put my .proto files in a directory like myproject/common.proto?

That's probably simplest. (This is what is done for the google/protobuf/*.proto files.)

However, you could also copy your files into an appropriate hierarchy in a temporary directory at code generation time.

Is there anything else I can do to avoid this panic at init time?

You can set the environment variable GOLANG_PROTOBUF_REGISTRATION_CONFLICT=warn to downgrade the panic to a warning, or GOLANG_PROTOBUF_REGISTRATION_CONFLICT=ignore to ignore it entirely.

You can also set the conflict policy at build time with a linker-initialized variable:

go build -ldflags "-X google.golang.org/protobuf/reflect/protoregistry.conflictPolicy=warn"

@nikandfor
Copy link

It seems putting proto files under com/org/project/ directory is the most compatible and stable solution so far. And maybe I'll stick to it anyway.

Yet it would be nice to be able to keep your proto files in arbitrary directory and still have them registered under uniq names.
So maybe we can introduce go_opt option, something like path_prefix, which doesn't change where files are searched or saved, but changes registry filenames as they were generated with fully qualified names?

Here is an example project structure and protoc command usage.
https://github.com/nikandfor/protobuf-test/blob/path_prefix/Makefile#L12

And here are few changes to protoc-gen-go to append path prefix to registry filenames. The key part is change in genFileDescriptor function in cmd/protoc-gen-go/internal_gengo/reflect.go file.
https://github.com/golang/protobuf/compare/master...nikandfor:protobuf:conflicting-filenames?expand=1

@janpfeifer
Copy link

Just hit the same issue. common.proto in two different projects, different package, different go_package.

And this is a 4 year old bug, and it seems so simple to fix in protoc, or just add proper documentation/information in the error message and site 😞

Making the error to a warning is also in many cases (my in particular) is not acceptable: I'm doing command line programs that shouldn't be spitting useless (from the end-user point of view) warnings.

Creating a cumbersome unique directory structure (com/github/my/project/...) misaligns with Go project structure (at least mine), so I'm avoiding this.

Now, after reading this very long issue I see that GOLANG_PROTOBUF_REGISTRATION_CONFLICT can be set to ignore. It would be helpful to have it documented in the ProtoBuf Go FAQ -- and more importantly, on the error message itself. It feels wrong though to hijack this global setting -- maybe someone else is depending on this being something else.

But finally, for anyone else out there who may be interested, I adopted a strategy of creating a temporary soft link named after the Go import path. It is presumably unique, but one could add a random id if needed. It soft links the current directory, and feeding this self-link to the protoc provides a unique and semantically meaningful key for the file (with the go import path), without having to change anything else.

#!/bin/bash
set -e

#!/bin/bash
set -e

# Find Go import path: presumably unique.
import_path="$(go list -f '{{.ImportPath}}')"

# Extract the domain and the rest of the path
domain=$(echo "$import_path" | cut -d '/' -f 1)
rest_of_path=$(echo "$import_path" | cut -d '/' -f 2-)

# Reverse the domain part (split by '.')
reversed_domain=$(echo "$domain" | awk -F '.' '{ for (i=NF; i>1; i--) printf "%s.", $i; print $1 }' | sed 's/\.$//')

# Combine the reversed domain with the rest of the path
tmp_link="$reversed_domain/$rest_of_path"
tmp_link=$(echo "$tmp_link" | tr '/.' '__')

# Create temporary self-link, run protoc and clean link.
rm -f "${tmp_link}"
ln -s . "${tmp_link}"
protoc --go_out=. --go_opt=paths=source_relative "./${tmp_link}/_your_proto_file_.proto"
rm -f "${tmp_link}"

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