reflect/protoregistry: restore conflicting file names check
There are inconsistencies between implementations on what happens when
a single program contains generated code for multiple files with the
same source path. At least one canonical implementation (C++) will fail
at link time. Others print warnings. Some silently resolve the registry
conflict in favor of one file or the other. The protobuf maintainers
agree, however, that the desired behavior is for this condition to be an
error.
Updates golang/protobuf#1122
Change-Id: I716708f16ef90210bdfceb0888691e47783df172
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/322729
Trust: Damien Neil <dneil@google.com>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
diff --git a/reflect/protoregistry/registry.go b/reflect/protoregistry/registry.go
index 3c7ec70..59f024c 100644
--- a/reflect/protoregistry/registry.go
+++ b/reflect/protoregistry/registry.go
@@ -121,8 +121,13 @@
r.filesByPath = make(map[string][]protoreflect.FileDescriptor)
}
path := file.Path()
- if len(r.filesByPath[path]) > 0 {
+ if prev := r.filesByPath[path]; len(prev) > 0 {
r.checkGenProtoConflict(path)
+ err := errors.New("file %q is already registered", file.Path())
+ err = amendErrorWithCaller(err, prev[0], file)
+ if !(r == GlobalFiles && ignoreConflict(file, err)) {
+ return err
+ }
}
for name := file.Package(); name != ""; name = name.Parent() {
diff --git a/reflect/protoregistry/registry_test.go b/reflect/protoregistry/registry_test.go
index bc8019c..d058c19 100644
--- a/reflect/protoregistry/registry_test.go
+++ b/reflect/protoregistry/registry_test.go
@@ -69,7 +69,7 @@
files: []testFile{
{inFile: mustMakeFile(`syntax:"proto2" name:"test1.proto" package:"foo.bar"`)},
{inFile: mustMakeFile(`syntax:"proto2" name:"foo/bar/test.proto" package:"my.test"`)},
- {inFile: mustMakeFile(`syntax:"proto2" name:"foo/bar/test.proto" package:"foo.bar.baz"`)},
+ {inFile: mustMakeFile(`syntax:"proto2" name:"foo/bar/test.proto" package:"foo.bar.baz"`), wantErr: "already registered"},
{inFile: mustMakeFile(`syntax:"proto2" name:"test2.proto" package:"my.test.package"`)},
{inFile: mustMakeFile(`syntax:"proto2" name:"weird" package:"foo.bar"`)},
{inFile: mustMakeFile(`syntax:"proto2" name:"foo/bar/baz/../test.proto" package:"my.test"`)},
@@ -112,8 +112,10 @@
{"weird", "foo.bar"},
},
}, {
- inPath: "foo/bar/test.proto",
- wantErr: `multiple files named "foo/bar/test.proto"`,
+ inPath: "foo/bar/test.proto",
+ wantFiles: []file{
+ {"foo/bar/test.proto", "my.test"},
+ },
}},
}, {
// Test when new enum conflicts with existing package.
