The EnumName, UnmarshalJSONEnum, and CompressGZIP helpers currently live
in v1 protoapi, which would cause all generated messages to depend on v1.
In an effort to break the dependency of v2 on v1, we move these helper
functions to v2 (and re-written to take advantage of protobuf reflection).
These helpers are unfortunate, but we cannot eliminate the functionality
that they implement since they are exposed in the publicly generated API.
Since EnumName does not rely on the enum maps, it removes another dependency
on those variables. Eventually, we can get to the point where these variables
(though declared) are not linked into the binary if the user does not use them.
Also, we rely on the v1 proto package for registration instead of v1 protoapi.
This may re-introduce a cyclic dependency on descriptor proto again in the
future, but the better approach is to just start registering with v2.
Change-Id: Id755585a7a1df14e4a6a2dfa650df221a3c153fb
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/167921
Reviewed-by: Damien Neil <dneil@google.com>
Added temporary work-around for now by always using deterninistic
marshaling so that protoV1.Equals knows how to do comparison.
Change-Id: Idec412647269d59f86ed3e485a1893c070355b3d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/167917
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Substitute protoV1.Marshal calls to use proto v2.
A few tests still need to use v1 due to missing required field checks in
v2.
Change-Id: I97ee208b81afc85c81142e70c1d7bdc47db76ce4
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/167774
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Unmarshaling of scalar, messages, repeated, and maps.
Need to further improve on error messages for consistency, some error
messages contain the position info while some currently do not. There
are cases where position info is wrong as well when a value is decoded
in another pass, e.g. numbers in string value, or map keys.
Change-Id: I6f9e903c499b5e87fb258dbdada7434389fc7522
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/166338
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
A blank line between the docstring and the package expression prevents
godoc from recognizing the comment as the package's docstring.
Change-Id: Id3c46ec6dbadb85a9b393c34fe1f0e46ed41a447
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/167766
Reviewed-by: Herbie Ong <herbie@google.com>
Unfortunately a good amount of code uses pointer comparisons on the
v1 ExtensionDesc to determine exactly which extension field is set,
rather than checking whether the extension descriptor semantically
describes the field that they are interested in.
To preserve this behavior in v1, we need a 1:1 mapping between
a v2 ExtensionType and a specific v1 ExtensionDesc.
Change-Id: I852b3cefb4585bd656e48e5adad6cc28795d02df
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/167759
Reviewed-by: Damien Neil <dneil@google.com>
Don't rely on legacyWrapper.ExtensionTypeFromDesc when desc.Type
is populated. Avoids panics when working with extensions when
internal/legacy has not been imported.
Change-Id: Id41ffa3c60b6e5503d0a3e29ef48e4f523fad588
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/167465
Reviewed-by: Joe Tsai <joetsai@google.com>
The prototype package was initially used by generated reflection support,
but has now been replaced by internal/fileinit.
Eventually, this functionality should be deleted and re-written in terms
of other components in the repo.
Usages that prototype currently provides (but should be moved) are:
* Constructing standalone messages and enums, which is behavior we should
provide in reflect/protodesc. The google.protobuf.{Enum,Type} are well-known
proto messages designed for this purpose.
* Constructing placeholder files, enums, and messages.
* Consructing protoreflect.{Message,Enum,Extension}Types, which are protobuf
descriptors with associated Go type information.
Change-Id: Id7dbefff952682781b439aa555508c59b2629f9e
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/167383
Reviewed-by: Damien Neil <dneil@google.com>
The test had a "golden" tag, which meant that it wasn't being tested.
Changes made:
* Alter the test to avoid shelling out to protoc
* Modify TestFiles to TestNoGoPackage, where the testdata files seem
concerned about making sure the lack of a go_package statement works
* Upgrade to latest cmp, which provides diffing for multiline strings
* Delete protogen/testdata directory
Change-Id: Ic576167bfce1046f45c8c614d0ab76af8c5ba723
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/167279
Reviewed-by: Damien Neil <dneil@google.com>
Collapse Value.Float32 and Value.Float64 into single API to keep it
consistent with Value.{Int,Uint}.
Change-Id: I07737e72715fe3cc3f6bcad579cf5d6cfe3757d5
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/167317
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Now that binary bloat concerns have been addressed, remove the flag
to control whether to generate support for reflection.
Change-Id: Ia0d11183707572caaf91d2f01dfa77e3aac0a417
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/167140
Reviewed-by: Damien Neil <dneil@google.com>
Rather than manually keeping track of which directories may be used
by other branches, use a time-based approach to determine whether
to purge a sub-directory or not.
Change-Id: I648dcca075e351e4e5624c48e73792782fc349ca
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/167042
Reviewed-by: Herbie Ong <herbie@google.com>
Previous decoder decodes a JSON number into a float64, which lacks
64-bit integer precision.
I attempted to retrofit it with storing the raw bytes and parsed out
number parts, see golang.org/cl/164377. While that is possible, the
encoding logic for Value is not symmetrical with the decoding logic and
can be confusing since both utilizes the same Value struct.
Joe and I decided that it would be better to rewrite the JSON encoder
and decoder to be token-based instead, removing the need for sharing a
model type plus making it more efficient.
Change-Id: Ic0601428a824be4e20141623409ab4d92b6167c7
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/165677
Reviewed-by: Damien Neil <dneil@google.com>
The previous implementation of messageWrapper implemented both
proto.Message and protoreflect.Message. This made it possible for users
to accidentally rely on this fact when it was actually an internal
implementation detail.
To avoid this, we split the wrapper type into two, ensuring that each
only implements one or the other. Doing so also revealed bugs in our
own code where we accidentally relied on this fact.
Change-Id: I0ff521b5c806b7dcb0b86942bd97e8319d8e8657
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/166938
Reviewed-by: Damien Neil <dneil@google.com>
The go.sum is not particularly important for the testdata directory,
so ignore it.
Change-Id: I069613deda2af4944d09aed34008fec201df6bb4
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/166879
Reviewed-by: Herbie Ong <herbie@google.com>
Running "go build ./..." does not descend into testdata directories.
However, the testdata in this repository is source code that is
intended to build properly. We could rename the directory, but that does
not test whether the generated packages can initialize properly.
Thus, we generate a trivial test that simply blank imports all packages.
Doing this reveals that some of the generated files have incorrect imports,
leading to registration conflicts.
To avoid introducing a dependency on gRPC from our go.mod file, we put
the testdata directories in their own module. Also, we avoid running
internal/testprotos through the grpc plugin because the servie and method
definitions in that directory are more for testing proto file initialization
rather than testing grpc generation.
Change-Id: Iaa6a06449787a085200e31bc7606e3ac904d3180
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/164917
Reviewed-by: Damien Neil <dneil@google.com>
To keep the dependency tree of Go protobufs as small as possible,
avoid depending on astutil. Most of the complexity of astutil.AddNamedImport
was for identifying an existing import block to insert imports into,
which is not relevant for our use-case.
Assuming that we always create a new import block after the package
statement, the logic for doing the AST manipulation is relatively simple.
This re-write properly handles an inline comment after the
package statement, which astutil.AddNamedImport (see golang.org/issue/30724)
currently fails to do.
Change-Id: I894e733aa82a241719b6f0c23de8d2fbfb67b778
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/166522
Reviewed-by: Damien Neil <dneil@google.com>
Ensure that the init funcs for files within a Go package run in the
dependency order of the source .proto files. That is, if a.proto and b.proto
are part of the same Go package, and a.proto imports b.proto, then b.pb.go's
init funcs must run before a.pb.go's.
Change-Id: I0e86ff22e5c4cab9df7a73fe4805390fadd34b0d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/166419
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Herbie Ong <herbie@google.com>
Changes made:
* Call autogen.sh before building protobuf.
It's unclear how this worked before, but the README instructions for
protobuf does state to run autogen.sh prior to building and installing.
* Fix downloadArchive to take in a prefix path rather than the number of
prefix directories to skip. The reason for this change is due to a bug
in the Go build system where unexpected directories were being packed.
See https://golang.org/issue/29906
* Explicitly set Travis dist to be "xenial", which comes with Go1.11.
We require Go1.11 for two reasons:
* The use of t.Helper in integration_test.go
* Proper understanding of the -mod=vendor flag
(even if all that flag does is disable modules).
* Add a hack to integration_test.go to periodically output the timestamp
to work around a restriction in Travis where it auto-kills the test
after 10 minutes of no stdout activity.
Change-Id: I114fe2855faeed091c34d79df3d97068be7eccd8
Reviewed-on: https://go-review.googlesource.com/c/164919
Reviewed-by: Herbie Ong <herbie@google.com>
Generate a list of descriptor fields from the descriptor.proto itself
as an internal package. Use these fields for the internal implementation.
Change-Id: Ib1ab0c5c6deb332ba6c8018ef55136b7e5974944
Reviewed-on: https://go-review.googlesource.com/c/164864
Reviewed-by: Herbie Ong <herbie@google.com>
We're rewriting internal/encoding/json. So, make a copy of it first in
order not to break encoding/jsonpb package.
Change-Id: I8b63c468d3f432102d2af4db22a7549998ce3876
Reviewed-on: https://go-review.googlesource.com/c/164642
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
This CL switches the integration test to be written in Go instead of bash.
The benefits are:
* The logic for setting up the dependencies is more robust and
handles better a situation where the dependency failed to initialize
(due to network trouble, FS permission problems, etc).
* The logic does a better job at cleaning up stale dependencies.
For example, my .cache folder has go1.9.4, go1.9.5, go1.10.5, and go1.10.6
folders even though we don't use them anymore.
* Being able to run only a subset of the integration test since you can
pass "-run" to the script.
* A signifcant amount of complexity in the test.bash script was running
the tests in parallel. This is trivial to do in Go.
The major detriment is that Go is more verbose as a "scripting" language.
Change-Id: Id7e5b303fb305fdbc0368bdf809dbf29fca1d983
Reviewed-on: https://go-review.googlesource.com/c/164861
Reviewed-by: Herbie Ong <herbie@google.com>
Using arrays in the generated reflection information adds unnecessary
eq and hash functions being added to the package. Change to slices
to reduce bloat.
Change-Id: I1a4f6d59021644d93dd6c24679b9233141e89a75
Reviewed-on: https://go-review.googlesource.com/c/164640
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Create a single binary for handling generation of protos.
This replaces previous logic spread throughout the repo in:
* regenerate.bash
* cmd/protoc-gen-go/golden_test.go
* cmd/protoc-gen-go-grpc/golden_test.go
* (indirectly) internal/protogen/goldentest
One of the problems with the former approaches is that they relied on
a version of protoc that was specific to a developer's workstation.
This meant that the result of generation was not hermetic.
To address this, we rely on the hard-coded version of protobuf specified
in the test.bash script.
A summary of changes in this CL are:
* The internal_gengo.GenerateFile and internal_gengogrpc.GenerateFile
functions are unified to have consistent signatures. It seems that the
former accepted a *protogen.GeneratedFile to support v1 where gRPC code
was generated into the same file as the base .pb.go file. However, the
same functionality can be achieved by having the function return
the generated file object.
* The test.bash script patches the protobuf toolchain to have properly
specified go_package options in each proto source file.
* The test.bash script accepts a "-regenerate" argument.
* Add generation for the well-known types. Contrary to how these were
laid out in the v1 repo, all the well-known types are placed in the
same Go package.
* Add generation for the conformance proto.
* Remove regenerate.bash
* Remove internal/protogen
* Remove cmd/protoc-gen-go/golden_test.go
* Remove cmd/protoc-gen-go-grpc/golden_test.go
* Add cmd/protoc-gen-go/annotation_test.go
Change-Id: I4a1a97ae6f66e2fabcf4e4d292c95ab2a2db0248
Reviewed-on: https://go-review.googlesource.com/c/164477
Reviewed-by: Damien Neil <dneil@google.com>
Use the full path (including the extension) for the generation of
the per-file variable name. Several reasons for this:
* The current logic is buggy in the case where pathType == pathTypeImport
since the prefix variable will be mangled with the Go import path.
* The extension is technically part of the path.
Thus, "path/to/foo.proto" and "path/to/foo.protodevel" are two
distinctly different imports.
* Style-wise, it subjectively looks better. Rather than being a mixture
of camelCase and snake_case, it is all snake_case for the common case:
before: ProtoFile_google_protobuf_any
after: File_google_protobuf_any_proto
* Since the extension is almost always ".proto", this results in a
suffix of "_proto", which provides an additional layer of protection
against possible name conflicts. The previous approach could possibly
have a conflict between "Foo.proto" and a message named ProtoFile
with a sub-message called Foo.
Also, use the per-file variable name for the raw descriptor variables
instead of the hashed version.
Change-Id: Ic91e326b7593e5985cee6ececc60539c27fe32fe
Reviewed-on: https://go-review.googlesource.com/c/164379
Reviewed-by: Damien Neil <dneil@google.com>
Calling a helper function directly should reduce binary bloat slightly.
Change-Id: I6068dc4cd00c8d90d2e6e6d99633b81388bc8781
Reviewed-on: https://go-review.googlesource.com/c/164679
Reviewed-by: Damien Neil <dneil@google.com>
For a file "foo/bar.proto", put the FileDescriptor in "ProtoFile_foo_bar"
rather than "Bar_fileDescriptor".
Avoid name clashes when a package contains "a/foo.proto" and "b/foo.proto".
Don't camelcase the filename: These vars weren't fully camelcased to begin
with, and leaving the filename relatively unchanged is clearer and more
predictable.
Move "ProtoFile" from the end of the var name to the start, so that vars
will sort better in packages with multiple descriptors.
These changes do add a chance of name collision when the input filename
begins with an uppercase letter: Foo.proto becomes "ProtoFile_Foo", which
could be the result of camelcasing "proto_file.foo". The readability
benefits seem worth it.
Change-Id: If27d3a0d7b5bf3535aa1607a8579eb057c74d2dc
Reviewed-on: https://go-review.googlesource.com/c/163199
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Herbie Ong <herbie@google.com>
This does not handle extensions, messagesets, and well-known types yet.
Change-Id: I2786c429f490fe8c57f3f85cd25058d936b58bf7
Reviewed-on: https://go-review.googlesource.com/c/162637
Reviewed-by: Damien Neil <dneil@google.com>
These test proto definitions will be reused for encoding/jsonpb package
and hence move these one directory up.
Also, add and simplify some tests.
Change-Id: I5297546fd9b853a7fd3e72dfab2fdc7237332c9c
Reviewed-on: https://go-review.googlesource.com/c/162537
Reviewed-by: Damien Neil <dneil@google.com>
This test examines the result of converting math.NaN() to a fixed byte
string. Change it to use a specific NaN value instead, since the value
returned by math.NaN is specified only as being a NaN, not a specific
one.
Use specific float32 and float64 NaN values, since the result of
converting a float64 NaN to a float32 can and does vary.
Fixes test failure on ARM.
Change-Id: Ia1517fdba768cdd88e5ee5f5af37f0b481e651b4
Reviewed-on: https://go-review.googlesource.com/c/162117
Reviewed-by: Herbie Ong <herbie@google.com>
Updates golang/protobuf#798 by adding testcases to show the intention.
Also, slightly move code blocks in encode.go w/o affecting logic to make
it cleaner.
Change-Id: I14575f6e7139a0908483bd318b599339c2daf8ad
Reviewed-on: https://go-review.googlesource.com/c/161717
Reviewed-by: Damien Neil <dneil@google.com>
This only handles compliant MessageSet extension fields where the field
name has to be message_set_extension.
Current C++ lib allows for different message field names, which is a
possible bug as it makes marshal output possibly contain duplicate names
when more than one field extends the same MessageSet, and makes
unmarshaling confusing as to which field to populate.
Change-Id: Ifda828ba794fe7e058ee6004f03001b1031f6d1e
Reviewed-on: https://go-review.googlesource.com/c/156758
Reviewed-by: Damien Neil <dneil@google.com>
These methods are no longer used. Drop them.
Change-Id: Ie5595c1a56dd76183782019384d16ea97d5f7e9e
Reviewed-on: https://go-review.googlesource.com/c/160357
Reviewed-by: Herbie Ong <herbie@google.com>
Test the fileinit and protodesc packages by verifying that passing a
FileDescriptorProto through a round trip of these packages leaves it
unchanged.
Change-Id: I6bfb894d95f1736f9908adee1ab63e9653b3f1be
Reviewed-on: https://go-review.googlesource.com/c/159762
Reviewed-by: Herbie Ong <herbie@google.com>
Add a collection of functions which take a reflect.*Descriptor and
return the corresponding google.protobuf.*DescriptorProto.
Change-Id: Ic186c412c8d3b7bc582c31bb8d8274459ce51a20
Reviewed-on: https://go-review.googlesource.com/c/159761
Reviewed-by: Herbie Ong <herbie@google.com>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
