From 0ff639994d3b3be9252ece1f1b1ac65d65f6111e Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Thu, 28 May 2020 10:56:10 -0700 Subject: [PATCH 01/17] Fixed Python release script to upload using twine instead of distutils. (#7571) --- python/release.sh | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/python/release.sh b/python/release.sh index 8d35640e92..c915e124d8 100755 --- a/python/release.sh +++ b/python/release.sh @@ -80,14 +80,16 @@ python setup.py build python setup.py test # Deploy source package to testing PyPI -python setup.py sdist upload -r https://test.pypi.org/legacy/ +python setup.py sdist +twine upload --skip-existing -r testpypi -u protobuf-wheel-test dist/* # Test locally with different python versions. run_install_test ${TESTING_VERSION} python2.7 https://test.pypi.org/simple run_install_test ${TESTING_VERSION} python3 https://test.pypi.org/simple # Deploy egg/wheel packages to testing PyPI and test again. -python setup.py bdist_egg bdist_wheel upload -r https://test.pypi.org/legacy/ +python setup.py clean build bdist_wheel +twine upload --skip-existing -r testpypi -u protobuf-wheel-test dist/* run_install_test ${TESTING_VERSION} python2.7 https://test.pypi.org/simple run_install_test ${TESTING_VERSION} python3 https://test.pypi.org/simple @@ -103,13 +105,15 @@ if [ $TESTING_ONLY -eq 0 ]; then echo "Publishing to PyPI..." # Be sure to run build before sdist, because otherwise sdist will not include # well-known types. - python setup.py clean build sdist upload + python setup.py clean build sdist + twine upload --skip-existing -u protobuf-packages dist/* # Be sure to run clean before bdist_xxx, because otherwise bdist_xxx will # include files you may not want in the package. E.g., if you have built # and tested with --cpp_implemenation, bdist_xxx will include the _message.so # file even when you no longer pass the --cpp_implemenation flag. See: # https://github.com/protocolbuffers/protobuf/issues/3042 - python setup.py clean build bdist_egg bdist_wheel upload + python setup.py clean build bdist_wheel + twine upload --skip-existing -u protobuf-packages dist/* else # Set the version number back (i.e., remove dev suffix). sed -i -r "s/__version__ = '.*'/__version__ = '${VERSION}'/" google/protobuf/__init__.py From 2360bacd12f5cc3f234b4155a2b7474f26ca3c7d Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Thu, 28 May 2020 14:13:39 -0400 Subject: [PATCH 02/17] Tweak the union used for Extensions to support old generated code. Support the old field name as well as the union to keep the old generated code building. Fixes #7555 --- objectivec/GPBDescriptor_PackagePrivate.h | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/objectivec/GPBDescriptor_PackagePrivate.h b/objectivec/GPBDescriptor_PackagePrivate.h index b3d673043c..408f8d4e10 100644 --- a/objectivec/GPBDescriptor_PackagePrivate.h +++ b/objectivec/GPBDescriptor_PackagePrivate.h @@ -131,14 +131,29 @@ typedef NS_OPTIONS(uint8_t, GPBExtensionOptions) { typedef struct GPBExtensionDescription { GPBGenericValue defaultValue; const char *singletonName; + // Before 3.12, `extendedClass` was just a `const char *`. Thanks to nested + // initialization (https://en.cppreference.com/w/c/language/struct_initialization#Nested_initialization) + // old generated code with `.extendedClass = GPBStringifySymbol(Something)` + // still works; and the current generator can use `extendedClass.clazz`, to + // pass a Class reference. union { const char *name; Class clazz; } extendedClass; + // Before 3.12, this was `const char *messageOrGroupClassName`. In the + // initial 3.12 release, we moved the `union messageOrGroupClass`, and failed + // to realize that would break existing source code for extensions. So to + // keep existing source code working, we added an unnamed union (C11) to + // provide both the old field name and the new union. This keeps both older + // and newer code working. + // Background: https://github.com/protocolbuffers/protobuf/issues/7555 union { - const char *name; - Class clazz; - } messageOrGroupClass; + const char *messageOrGroupClassName; + union { + const char *name; + Class clazz; + } messageOrGroupClass; + }; GPBEnumDescriptorFunc enumDescriptorFunc; int32_t fieldNumber; GPBDataType dataType; From 9ce8c330e736b99c2bb00bdc6a60f00ab600c283 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Mon, 1 Jun 2020 13:36:50 -0700 Subject: [PATCH 03/17] Updated version to 3.12.3 and updated CHANGES.txt. (#7580) * Updated version to 3.12.3 and updated CHANGES.txt. * Re-ran generate_descriptor_protos.sh and made it more parallel. --- CHANGES.txt | 5 +++++ Protobuf-C++.podspec | 2 +- Protobuf.podspec | 2 +- configure.ac | 2 +- csharp/Google.Protobuf.Tools.nuspec | 2 +- .../Google.Protobuf/Google.Protobuf.csproj | 2 +- generate_descriptor_proto.sh | 2 +- java/bom/pom.xml | 2 +- java/core/pom.xml | 2 +- java/lite/pom.xml | 2 +- java/pom.xml | 2 +- java/util/pom.xml | 2 +- js/package.json | 2 +- php/ext/google/protobuf/package.xml | 22 +++++++++++++++---- php/ext/google/protobuf/protobuf.h | 2 +- protoc-artifacts/pom.xml | 2 +- python/google/protobuf/__init__.py | 2 +- ruby/google-protobuf.gemspec | 2 +- src/Makefile.am | 2 +- src/google/protobuf/any.pb.h | 2 +- src/google/protobuf/api.pb.h | 2 +- src/google/protobuf/compiler/plugin.pb.h | 2 +- src/google/protobuf/descriptor.pb.h | 2 +- src/google/protobuf/duration.pb.h | 2 +- src/google/protobuf/empty.pb.h | 2 +- src/google/protobuf/field_mask.pb.h | 2 +- src/google/protobuf/port_def.inc | 2 +- src/google/protobuf/source_context.pb.h | 2 +- src/google/protobuf/struct.pb.h | 2 +- src/google/protobuf/stubs/common.h | 2 +- src/google/protobuf/timestamp.pb.h | 2 +- src/google/protobuf/type.pb.h | 2 +- src/google/protobuf/wrappers.pb.h | 2 +- 33 files changed, 54 insertions(+), 35 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 4b18dbbec3..ce4d746868 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,8 @@ +2020-06-01 version 3.12.3 (C++/Java/Python/PHP/Objective-C/C#/Ruby/JavaScript) + + Objective-C + * Tweak the union used for Extensions to support old generated code. #7573 + 2020-05-26 version 3.12.2 (C++/Java/Python/PHP/Objective-C/C#/Ruby/JavaScript) C++ diff --git a/Protobuf-C++.podspec b/Protobuf-C++.podspec index 67a96cc7a0..6a51340d0f 100644 --- a/Protobuf-C++.podspec +++ b/Protobuf-C++.podspec @@ -1,6 +1,6 @@ Pod::Spec.new do |s| s.name = 'Protobuf-C++' - s.version = '3.12.2' + s.version = '3.12.3' s.summary = 'Protocol Buffers v3 runtime library for C++.' s.homepage = 'https://github.com/google/protobuf' s.license = '3-Clause BSD License' diff --git a/Protobuf.podspec b/Protobuf.podspec index a05b712326..3b7a716761 100644 --- a/Protobuf.podspec +++ b/Protobuf.podspec @@ -5,7 +5,7 @@ # dependent projects use the :git notation to refer to the library. Pod::Spec.new do |s| s.name = 'Protobuf' - s.version = '3.12.2' + s.version = '3.12.3' s.summary = 'Protocol Buffers v.3 runtime library for Objective-C.' s.homepage = 'https://github.com/protocolbuffers/protobuf' s.license = '3-Clause BSD License' diff --git a/configure.ac b/configure.ac index e473a57914..b7c7f53068 100644 --- a/configure.ac +++ b/configure.ac @@ -17,7 +17,7 @@ AC_PREREQ(2.59) # In the SVN trunk, the version should always be the next anticipated release # version with the "-pre" suffix. (We used to use "-SNAPSHOT" but this pushed # the size of one file name in the dist tarfile over the 99-char limit.) -AC_INIT([Protocol Buffers],[3.12.2],[protobuf@googlegroups.com],[protobuf]) +AC_INIT([Protocol Buffers],[3.12.3],[protobuf@googlegroups.com],[protobuf]) AM_MAINTAINER_MODE([enable]) diff --git a/csharp/Google.Protobuf.Tools.nuspec b/csharp/Google.Protobuf.Tools.nuspec index 50aca1b506..d0c088f2a1 100644 --- a/csharp/Google.Protobuf.Tools.nuspec +++ b/csharp/Google.Protobuf.Tools.nuspec @@ -5,7 +5,7 @@ Google Protocol Buffers tools Tools for Protocol Buffers - Google's data interchange format. See project site for more info. - 3.12.2 + 3.12.3 Google Inc. protobuf-packages https://github.com/protocolbuffers/protobuf/blob/master/LICENSE diff --git a/csharp/src/Google.Protobuf/Google.Protobuf.csproj b/csharp/src/Google.Protobuf/Google.Protobuf.csproj index c79357ef5f..44d95b66c6 100644 --- a/csharp/src/Google.Protobuf/Google.Protobuf.csproj +++ b/csharp/src/Google.Protobuf/Google.Protobuf.csproj @@ -4,7 +4,7 @@ C# runtime library for Protocol Buffers - Google's data interchange format. Copyright 2015, Google Inc. Google Protocol Buffers - 3.12.2 + 3.12.3 6 Google Inc. netstandard1.0;netstandard2.0;net45 diff --git a/generate_descriptor_proto.sh b/generate_descriptor_proto.sh index e533d05b43..dc03fee019 100755 --- a/generate_descriptor_proto.sh +++ b/generate_descriptor_proto.sh @@ -62,7 +62,7 @@ do PROTOC=$BOOTSTRAP_PROTOC BOOTSTRAP_PROTOC="" else - make $@ protoc + make -j$(nproc) $@ protoc if test $? -ne 0; then echo "Failed to build protoc." exit 1 diff --git a/java/bom/pom.xml b/java/bom/pom.xml index 5a63a1dc71..ff2d106764 100644 --- a/java/bom/pom.xml +++ b/java/bom/pom.xml @@ -4,7 +4,7 @@ com.google.protobuf protobuf-bom - 3.12.2 + 3.12.3 pom Protocol Buffers [BOM] diff --git a/java/core/pom.xml b/java/core/pom.xml index 4112eb2c17..b2b0b0386e 100644 --- a/java/core/pom.xml +++ b/java/core/pom.xml @@ -4,7 +4,7 @@ com.google.protobuf protobuf-parent - 3.12.2 + 3.12.3 protobuf-java diff --git a/java/lite/pom.xml b/java/lite/pom.xml index 30e6f37132..49686cd7ad 100644 --- a/java/lite/pom.xml +++ b/java/lite/pom.xml @@ -4,7 +4,7 @@ com.google.protobuf protobuf-parent - 3.12.2 + 3.12.3 protobuf-javalite diff --git a/java/pom.xml b/java/pom.xml index f61c85755d..0a813e02dc 100644 --- a/java/pom.xml +++ b/java/pom.xml @@ -4,7 +4,7 @@ com.google.protobuf protobuf-parent - 3.12.2 + 3.12.3 pom Protocol Buffers [Parent] diff --git a/java/util/pom.xml b/java/util/pom.xml index a9b8b4ab00..daa123a944 100644 --- a/java/util/pom.xml +++ b/java/util/pom.xml @@ -4,7 +4,7 @@ com.google.protobuf protobuf-parent - 3.12.2 + 3.12.3 protobuf-java-util diff --git a/js/package.json b/js/package.json index 7fd5e69679..e4934e544c 100644 --- a/js/package.json +++ b/js/package.json @@ -1,6 +1,6 @@ { "name": "google-protobuf", - "version": "3.12.2", + "version": "3.12.3", "description": "Protocol Buffers for JavaScript", "main": "google-protobuf.js", "files": [ diff --git a/php/ext/google/protobuf/package.xml b/php/ext/google/protobuf/package.xml index 5c9d810584..01a4b86d67 100644 --- a/php/ext/google/protobuf/package.xml +++ b/php/ext/google/protobuf/package.xml @@ -10,11 +10,11 @@ protobuf-opensource@google.com yes - 2020-05-26 - + 2020-06-01 + - 3.12.2 - 3.12.2 + 3.12.3 + 3.12.3 stable @@ -599,5 +599,19 @@ G A release. 3-Clause BSD License GA release. + + + 3.12.3 + 3.12.3 + + + stable + stable + + 2020-06-01 + + 3-Clause BSD License + GA release. + diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index 090f96018c..4b8c7b15ad 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -37,7 +37,7 @@ #include "upb.h" #define PHP_PROTOBUF_EXTNAME "protobuf" -#define PHP_PROTOBUF_VERSION "3.12.2" +#define PHP_PROTOBUF_VERSION "3.12.3" #define MAX_LENGTH_OF_INT64 20 #define SIZEOF_INT64 8 diff --git a/protoc-artifacts/pom.xml b/protoc-artifacts/pom.xml index ed028d5d4e..6d750c5b67 100644 --- a/protoc-artifacts/pom.xml +++ b/protoc-artifacts/pom.xml @@ -8,7 +8,7 @@ com.google.protobuf protoc - 3.12.2 + 3.12.3 pom Protobuf Compiler diff --git a/python/google/protobuf/__init__.py b/python/google/protobuf/__init__.py index 8b3b580922..fce24efdf6 100644 --- a/python/google/protobuf/__init__.py +++ b/python/google/protobuf/__init__.py @@ -30,7 +30,7 @@ # Copyright 2007 Google Inc. All Rights Reserved. -__version__ = '3.12.2' +__version__ = '3.12.3' if __name__ != '__main__': try: diff --git a/ruby/google-protobuf.gemspec b/ruby/google-protobuf.gemspec index 469074fe85..2160d520ab 100644 --- a/ruby/google-protobuf.gemspec +++ b/ruby/google-protobuf.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |s| s.name = "google-protobuf" - s.version = "3.12.2" + s.version = "3.12.3" git_tag = "v#{s.version.to_s.sub('.rc.', '-rc')}" # Converts X.Y.Z.rc.N to vX.Y.Z-rcN, used for the git tag s.licenses = ["BSD-3-Clause"] s.summary = "Protocol Buffers" diff --git a/src/Makefile.am b/src/Makefile.am index d73195c405..f1099d9fec 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -18,7 +18,7 @@ else PTHREAD_DEF = endif -PROTOBUF_VERSION = 23:2:0 +PROTOBUF_VERSION = 23:3:0 if GCC # Turn on all warnings except for sign comparison (we ignore sign comparison diff --git a/src/google/protobuf/any.pb.h b/src/google/protobuf/any.pb.h index 36c69edbd2..f09ca570ce 100644 --- a/src/google/protobuf/any.pb.h +++ b/src/google/protobuf/any.pb.h @@ -13,7 +13,7 @@ #error incompatible with your Protocol Buffer headers. Please update #error your headers. #endif -#if 3012002 < PROTOBUF_MIN_PROTOC_VERSION +#if 3012003 < PROTOBUF_MIN_PROTOC_VERSION #error This file was generated by an older version of protoc which is #error incompatible with your Protocol Buffer headers. Please #error regenerate this file with a newer version of protoc. diff --git a/src/google/protobuf/api.pb.h b/src/google/protobuf/api.pb.h index d1a054ade9..2fe757ff21 100644 --- a/src/google/protobuf/api.pb.h +++ b/src/google/protobuf/api.pb.h @@ -13,7 +13,7 @@ #error incompatible with your Protocol Buffer headers. Please update #error your headers. #endif -#if 3012002 < PROTOBUF_MIN_PROTOC_VERSION +#if 3012003 < PROTOBUF_MIN_PROTOC_VERSION #error This file was generated by an older version of protoc which is #error incompatible with your Protocol Buffer headers. Please #error regenerate this file with a newer version of protoc. diff --git a/src/google/protobuf/compiler/plugin.pb.h b/src/google/protobuf/compiler/plugin.pb.h index 0fee42a759..27b66635d7 100644 --- a/src/google/protobuf/compiler/plugin.pb.h +++ b/src/google/protobuf/compiler/plugin.pb.h @@ -13,7 +13,7 @@ #error incompatible with your Protocol Buffer headers. Please update #error your headers. #endif -#if 3012002 < PROTOBUF_MIN_PROTOC_VERSION +#if 3012003 < PROTOBUF_MIN_PROTOC_VERSION #error This file was generated by an older version of protoc which is #error incompatible with your Protocol Buffer headers. Please #error regenerate this file with a newer version of protoc. diff --git a/src/google/protobuf/descriptor.pb.h b/src/google/protobuf/descriptor.pb.h index 54eac71967..96695b836c 100644 --- a/src/google/protobuf/descriptor.pb.h +++ b/src/google/protobuf/descriptor.pb.h @@ -13,7 +13,7 @@ #error incompatible with your Protocol Buffer headers. Please update #error your headers. #endif -#if 3012002 < PROTOBUF_MIN_PROTOC_VERSION +#if 3012003 < PROTOBUF_MIN_PROTOC_VERSION #error This file was generated by an older version of protoc which is #error incompatible with your Protocol Buffer headers. Please #error regenerate this file with a newer version of protoc. diff --git a/src/google/protobuf/duration.pb.h b/src/google/protobuf/duration.pb.h index b8b16340db..da2253b2c1 100644 --- a/src/google/protobuf/duration.pb.h +++ b/src/google/protobuf/duration.pb.h @@ -13,7 +13,7 @@ #error incompatible with your Protocol Buffer headers. Please update #error your headers. #endif -#if 3012002 < PROTOBUF_MIN_PROTOC_VERSION +#if 3012003 < PROTOBUF_MIN_PROTOC_VERSION #error This file was generated by an older version of protoc which is #error incompatible with your Protocol Buffer headers. Please #error regenerate this file with a newer version of protoc. diff --git a/src/google/protobuf/empty.pb.h b/src/google/protobuf/empty.pb.h index cf77a6995f..5240bfdebb 100644 --- a/src/google/protobuf/empty.pb.h +++ b/src/google/protobuf/empty.pb.h @@ -13,7 +13,7 @@ #error incompatible with your Protocol Buffer headers. Please update #error your headers. #endif -#if 3012002 < PROTOBUF_MIN_PROTOC_VERSION +#if 3012003 < PROTOBUF_MIN_PROTOC_VERSION #error This file was generated by an older version of protoc which is #error incompatible with your Protocol Buffer headers. Please #error regenerate this file with a newer version of protoc. diff --git a/src/google/protobuf/field_mask.pb.h b/src/google/protobuf/field_mask.pb.h index da74b583df..141aa90f81 100644 --- a/src/google/protobuf/field_mask.pb.h +++ b/src/google/protobuf/field_mask.pb.h @@ -13,7 +13,7 @@ #error incompatible with your Protocol Buffer headers. Please update #error your headers. #endif -#if 3012002 < PROTOBUF_MIN_PROTOC_VERSION +#if 3012003 < PROTOBUF_MIN_PROTOC_VERSION #error This file was generated by an older version of protoc which is #error incompatible with your Protocol Buffer headers. Please #error regenerate this file with a newer version of protoc. diff --git a/src/google/protobuf/port_def.inc b/src/google/protobuf/port_def.inc index 4aa0969623..c524915c07 100644 --- a/src/google/protobuf/port_def.inc +++ b/src/google/protobuf/port_def.inc @@ -300,7 +300,7 @@ // Shared google3/opensource definitions. ////////////////////////////////////// -#define PROTOBUF_VERSION 3012002 +#define PROTOBUF_VERSION 3012003 #define PROTOBUF_MIN_HEADER_VERSION_FOR_PROTOC 3012000 #define PROTOBUF_MIN_PROTOC_VERSION 3012000 #define PROTOBUF_VERSION_SUFFIX "" diff --git a/src/google/protobuf/source_context.pb.h b/src/google/protobuf/source_context.pb.h index ccfcb88dc5..b69e1ed5dd 100644 --- a/src/google/protobuf/source_context.pb.h +++ b/src/google/protobuf/source_context.pb.h @@ -13,7 +13,7 @@ #error incompatible with your Protocol Buffer headers. Please update #error your headers. #endif -#if 3012002 < PROTOBUF_MIN_PROTOC_VERSION +#if 3012003 < PROTOBUF_MIN_PROTOC_VERSION #error This file was generated by an older version of protoc which is #error incompatible with your Protocol Buffer headers. Please #error regenerate this file with a newer version of protoc. diff --git a/src/google/protobuf/struct.pb.h b/src/google/protobuf/struct.pb.h index 7d8fd61369..afce42af39 100644 --- a/src/google/protobuf/struct.pb.h +++ b/src/google/protobuf/struct.pb.h @@ -13,7 +13,7 @@ #error incompatible with your Protocol Buffer headers. Please update #error your headers. #endif -#if 3012002 < PROTOBUF_MIN_PROTOC_VERSION +#if 3012003 < PROTOBUF_MIN_PROTOC_VERSION #error This file was generated by an older version of protoc which is #error incompatible with your Protocol Buffer headers. Please #error regenerate this file with a newer version of protoc. diff --git a/src/google/protobuf/stubs/common.h b/src/google/protobuf/stubs/common.h index 0fd679a6fb..5843b5ba5b 100644 --- a/src/google/protobuf/stubs/common.h +++ b/src/google/protobuf/stubs/common.h @@ -81,7 +81,7 @@ namespace internal { // The current version, represented as a single integer to make comparison // easier: major * 10^6 + minor * 10^3 + micro -#define GOOGLE_PROTOBUF_VERSION 3012002 +#define GOOGLE_PROTOBUF_VERSION 3012003 // A suffix string for alpha, beta or rc releases. Empty for stable releases. #define GOOGLE_PROTOBUF_VERSION_SUFFIX "" diff --git a/src/google/protobuf/timestamp.pb.h b/src/google/protobuf/timestamp.pb.h index 64249978e7..7f4cf97da8 100644 --- a/src/google/protobuf/timestamp.pb.h +++ b/src/google/protobuf/timestamp.pb.h @@ -13,7 +13,7 @@ #error incompatible with your Protocol Buffer headers. Please update #error your headers. #endif -#if 3012002 < PROTOBUF_MIN_PROTOC_VERSION +#if 3012003 < PROTOBUF_MIN_PROTOC_VERSION #error This file was generated by an older version of protoc which is #error incompatible with your Protocol Buffer headers. Please #error regenerate this file with a newer version of protoc. diff --git a/src/google/protobuf/type.pb.h b/src/google/protobuf/type.pb.h index ffe832d0be..b337202764 100644 --- a/src/google/protobuf/type.pb.h +++ b/src/google/protobuf/type.pb.h @@ -13,7 +13,7 @@ #error incompatible with your Protocol Buffer headers. Please update #error your headers. #endif -#if 3012002 < PROTOBUF_MIN_PROTOC_VERSION +#if 3012003 < PROTOBUF_MIN_PROTOC_VERSION #error This file was generated by an older version of protoc which is #error incompatible with your Protocol Buffer headers. Please #error regenerate this file with a newer version of protoc. diff --git a/src/google/protobuf/wrappers.pb.h b/src/google/protobuf/wrappers.pb.h index e8f82007fe..7af6e994de 100644 --- a/src/google/protobuf/wrappers.pb.h +++ b/src/google/protobuf/wrappers.pb.h @@ -13,7 +13,7 @@ #error incompatible with your Protocol Buffer headers. Please update #error your headers. #endif -#if 3012002 < PROTOBUF_MIN_PROTOC_VERSION +#if 3012003 < PROTOBUF_MIN_PROTOC_VERSION #error This file was generated by an older version of protoc which is #error incompatible with your Protocol Buffer headers. Please #error regenerate this file with a newer version of protoc. From 921bdaaa61b8b62437a3cd999c1962c453ff937b Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 23 Apr 2020 16:13:06 +1200 Subject: [PATCH 04/17] Improve repeated fixed parsing performance --- .../ParseMessagesBenchmark.cs | 39 ++++++++ .../Collections/RepeatedFieldTest.cs | 89 +++++++++++++++++++ .../ReadOnlySequenceFactory.cs | 4 +- .../Collections/RepeatedField.cs | 45 +++++++++- 4 files changed, 173 insertions(+), 4 deletions(-) diff --git a/csharp/src/Google.Protobuf.Benchmarks/ParseMessagesBenchmark.cs b/csharp/src/Google.Protobuf.Benchmarks/ParseMessagesBenchmark.cs index 30f3e9ef37..92c3dbe5f0 100644 --- a/csharp/src/Google.Protobuf.Benchmarks/ParseMessagesBenchmark.cs +++ b/csharp/src/Google.Protobuf.Benchmarks/ParseMessagesBenchmark.cs @@ -37,6 +37,7 @@ using System.IO; using System.Linq; using System.Buffers; using Google.Protobuf.WellKnownTypes; +using Benchmarks.Proto3; namespace Google.Protobuf.Benchmarks { @@ -50,6 +51,7 @@ namespace Google.Protobuf.Benchmarks SubTest manyWrapperFieldsTest = new SubTest(CreateManyWrapperFieldsMessage(), ManyWrapperFieldsMessage.Parser, () => new ManyWrapperFieldsMessage(), MaxMessages); SubTest manyPrimitiveFieldsTest = new SubTest(CreateManyPrimitiveFieldsMessage(), ManyPrimitiveFieldsMessage.Parser, () => new ManyPrimitiveFieldsMessage(), MaxMessages); + SubTest repeatedFieldTest = new SubTest(CreateRepeatedFieldMessage(), GoogleMessage1.Parser, () => new GoogleMessage1(), MaxMessages); SubTest emptyMessageTest = new SubTest(new Empty(), Empty.Parser, () => new Empty(), MaxMessages); public IEnumerable MessageCountValues => new[] { 10, 100 }; @@ -83,6 +85,18 @@ namespace Google.Protobuf.Benchmarks return manyPrimitiveFieldsTest.ParseFromReadOnlySequence(); } + [Benchmark] + public IMessage RepeatedFieldMessage_ParseFromByteArray() + { + return repeatedFieldTest.ParseFromByteArray(); + } + + [Benchmark] + public IMessage RepeatedFieldMessage_ParseFromReadOnlySequence() + { + return repeatedFieldTest.ParseFromReadOnlySequence(); + } + [Benchmark] public IMessage EmptyMessage_ParseFromByteArray() { @@ -123,6 +137,20 @@ namespace Google.Protobuf.Benchmarks manyPrimitiveFieldsTest.ParseDelimitedMessagesFromReadOnlySequence(messageCount); } + [Benchmark] + [ArgumentsSource(nameof(MessageCountValues))] + public void RepeatedFieldMessage_ParseDelimitedMessagesFromByteArray(int messageCount) + { + repeatedFieldTest.ParseDelimitedMessagesFromByteArray(messageCount); + } + + [Benchmark] + [ArgumentsSource(nameof(MessageCountValues))] + public void RepeatedFieldMessage_ParseDelimitedMessagesFromReadOnlySequence(int messageCount) + { + repeatedFieldTest.ParseDelimitedMessagesFromReadOnlySequence(messageCount); + } + private static ManyWrapperFieldsMessage CreateManyWrapperFieldsMessage() { // Example data match data of an internal benchmarks @@ -157,6 +185,17 @@ namespace Google.Protobuf.Benchmarks }; } + private static GoogleMessage1 CreateRepeatedFieldMessage() + { + // Message with a repeated fixed length item collection + var message = new GoogleMessage1(); + for (ulong i = 0; i < 1000; i++) + { + message.Field5.Add(i); + } + return message; + } + private class SubTest { private readonly IMessage message; diff --git a/csharp/src/Google.Protobuf.Test/Collections/RepeatedFieldTest.cs b/csharp/src/Google.Protobuf.Test/Collections/RepeatedFieldTest.cs index 527c7d404c..61453e5ab2 100644 --- a/csharp/src/Google.Protobuf.Test/Collections/RepeatedFieldTest.cs +++ b/csharp/src/Google.Protobuf.Test/Collections/RepeatedFieldTest.cs @@ -595,6 +595,95 @@ namespace Google.Protobuf.Collections Assert.AreEqual(((SampleEnum)(-5)), values[5]); } + [Test] + public void TestPackedRepeatedFieldCollectionNonDivisibleLength() + { + uint tag = WireFormat.MakeTag(10, WireFormat.WireType.LengthDelimited); + var codec = FieldCodec.ForFixed32(tag); + var stream = new MemoryStream(); + var output = new CodedOutputStream(stream); + output.WriteTag(tag); + output.WriteString("A long string"); + output.WriteTag(codec.Tag); + output.WriteRawVarint32((uint)codec.FixedSize - 1); // Length not divisible by FixedSize + output.WriteFixed32(uint.MaxValue); + output.Flush(); + stream.Position = 0; + + var input = new CodedInputStream(stream); + input.ReadTag(); + input.ReadString(); + input.ReadTag(); + var field = new RepeatedField(); + Assert.Throws(() => field.AddEntriesFrom(input, codec)); + + // Collection was not pre-initialized + Assert.AreEqual(0, field.Count); + } + + [Test] + public void TestPackedRepeatedFieldCollectionNotAllocatedWhenLengthExceedsBuffer() + { + uint tag = WireFormat.MakeTag(10, WireFormat.WireType.LengthDelimited); + var codec = FieldCodec.ForFixed32(tag); + var stream = new MemoryStream(); + var output = new CodedOutputStream(stream); + output.WriteTag(tag); + output.WriteString("A long string"); + output.WriteTag(codec.Tag); + output.WriteRawVarint32((uint)codec.FixedSize); + // Note that there is no content for the packed field. + // The field length exceeds the remaining length of content. + output.Flush(); + stream.Position = 0; + + var input = new CodedInputStream(stream); + input.ReadTag(); + input.ReadString(); + input.ReadTag(); + var field = new RepeatedField(); + Assert.Throws(() => field.AddEntriesFrom(input, codec)); + + // Collection was not pre-initialized + Assert.AreEqual(0, field.Count); + } + + [Test] + public void TestPackedRepeatedFieldCollectionNotAllocatedWhenLengthExceedsRemainingData() + { + uint tag = WireFormat.MakeTag(10, WireFormat.WireType.LengthDelimited); + var codec = FieldCodec.ForFixed32(tag); + var stream = new MemoryStream(); + var output = new CodedOutputStream(stream); + output.WriteTag(tag); + output.WriteString("A long string"); + output.WriteTag(codec.Tag); + output.WriteRawVarint32((uint)codec.FixedSize); + // Note that there is no content for the packed field. + // The field length exceeds the remaining length of the buffer. + output.Flush(); + stream.Position = 0; + + var sequence = ReadOnlySequenceFactory.CreateWithContent(stream.ToArray()); + ParseContext.Initialize(sequence, out ParseContext ctx); + + ctx.ReadTag(); + ctx.ReadString(); + ctx.ReadTag(); + var field = new RepeatedField(); + try + { + field.AddEntriesFrom(ref ctx, codec); + Assert.Fail(); + } + catch (InvalidProtocolBufferException) + { + } + + // Collection was not pre-initialized + Assert.AreEqual(0, field.Count); + } + // Fairly perfunctory tests for the non-generic IList implementation [Test] public void IList_Indexer() diff --git a/csharp/src/Google.Protobuf.Test/ReadOnlySequenceFactory.cs b/csharp/src/Google.Protobuf.Test/ReadOnlySequenceFactory.cs index d2050d37ca..588b559e9f 100644 --- a/csharp/src/Google.Protobuf.Test/ReadOnlySequenceFactory.cs +++ b/csharp/src/Google.Protobuf.Test/ReadOnlySequenceFactory.cs @@ -50,9 +50,9 @@ namespace Google.Protobuf while (currentIndex < data.Length) { var segment = new List(); - for (; currentIndex < Math.Min(currentIndex + segmentSize, data.Length); currentIndex++) + while (segment.Count < segmentSize && currentIndex < data.Length) { - segment.Add(data[currentIndex]); + segment.Add(data[currentIndex++]); } segments.Add(segment.ToArray()); segments.Add(new byte[0]); diff --git a/csharp/src/Google.Protobuf/Collections/RepeatedField.cs b/csharp/src/Google.Protobuf/Collections/RepeatedField.cs index 5c39aabafb..b1bd9b13d1 100644 --- a/csharp/src/Google.Protobuf/Collections/RepeatedField.cs +++ b/csharp/src/Google.Protobuf/Collections/RepeatedField.cs @@ -35,6 +35,7 @@ using System.Collections; using System.Collections.Generic; using System.IO; using System.Security; +using System.Threading; namespace Google.Protobuf.Collections { @@ -126,9 +127,31 @@ namespace Google.Protobuf.Collections if (length > 0) { int oldLimit = SegmentedBufferHelper.PushLimit(ref ctx.state, length); - while (!SegmentedBufferHelper.IsReachedLimit(ref ctx.state)) + + // If the content is fixed size then we can calculate the length + // of the repeated field and pre-initialize the underlying collection. + // + // Check that the supplied length doesn't exceed the underlying buffer. + // That prevents a malicious length from initializing a very large collection. + if (codec.FixedSize > 0 && length % codec.FixedSize == 0 && IsDataAvailable(ref ctx, length)) + { + EnsureSize(count + (length / codec.FixedSize)); + + while (!SegmentedBufferHelper.IsReachedLimit(ref ctx.state)) + { + // Only FieldCodecs with a fixed size can reach here, and they are all known + // types that don't allow the user to specify a custom reader action. + // reader action will never return null. + array[count++] = reader(ref ctx); + } + } + else { - Add(reader(ref ctx)); + // Content is variable size so add until we reach the limit. + while (!SegmentedBufferHelper.IsReachedLimit(ref ctx.state)) + { + Add(reader(ref ctx)); + } } SegmentedBufferHelper.PopLimit(ref ctx.state, oldLimit); } @@ -144,6 +167,24 @@ namespace Google.Protobuf.Collections } } + private bool IsDataAvailable(ref ParseContext ctx, int size) + { + // Data fits in remaining buffer + if (size <= ctx.state.bufferSize - ctx.state.bufferPos) + { + return true; + } + + // Data fits in remaining source data. + // Note that this will never be true when reading from a stream as the total length is unknown. + if (size < ctx.state.segmentedBufferHelper.TotalLength - ctx.state.totalBytesRetired - ctx.state.bufferPos) + { + return true; + } + + return false; + } + /// /// Calculates the size of this collection based on the given codec. /// From b9cf3866c5f742d7a08c99b55c701c917c9b0d70 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Tue, 2 Jun 2020 16:27:34 -0400 Subject: [PATCH 05/17] Tweak return types for GetSupportedFeatures to unit64_t. --- src/google/protobuf/compiler/csharp/csharp_generator.cc | 2 +- src/google/protobuf/compiler/csharp/csharp_generator.h | 2 +- src/google/protobuf/compiler/objectivec/objectivec_generator.h | 2 +- src/google/protobuf/compiler/ruby/ruby_generator.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/google/protobuf/compiler/csharp/csharp_generator.cc b/src/google/protobuf/compiler/csharp/csharp_generator.cc index c2170f17d1..8d90a1d366 100644 --- a/src/google/protobuf/compiler/csharp/csharp_generator.cc +++ b/src/google/protobuf/compiler/csharp/csharp_generator.cc @@ -51,7 +51,7 @@ namespace csharp { Generator::Generator() {} Generator::~Generator() {} -uint64 Generator::GetSupportedFeatures() const { +uint64_t Generator::GetSupportedFeatures() const { return CodeGenerator::Feature::FEATURE_PROTO3_OPTIONAL; } diff --git a/src/google/protobuf/compiler/csharp/csharp_generator.h b/src/google/protobuf/compiler/csharp/csharp_generator.h index 76806db16c..8875d4caac 100644 --- a/src/google/protobuf/compiler/csharp/csharp_generator.h +++ b/src/google/protobuf/compiler/csharp/csharp_generator.h @@ -57,7 +57,7 @@ class PROTOC_EXPORT Generator : public CodeGenerator { const string& parameter, GeneratorContext* generator_context, string* error) const override; - uint64 GetSupportedFeatures() const override; + uint64_t GetSupportedFeatures() const override; }; } // namespace csharp diff --git a/src/google/protobuf/compiler/objectivec/objectivec_generator.h b/src/google/protobuf/compiler/objectivec/objectivec_generator.h index b09e2b2d3f..d1e490c8c9 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_generator.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_generator.h @@ -67,7 +67,7 @@ class PROTOC_EXPORT ObjectiveCGenerator : public CodeGenerator { GeneratorContext* context, string* error) const override; - uint64 GetSupportedFeatures() const override { + uint64_t GetSupportedFeatures() const override { return FEATURE_PROTO3_OPTIONAL; } }; diff --git a/src/google/protobuf/compiler/ruby/ruby_generator.h b/src/google/protobuf/compiler/ruby/ruby_generator.h index ea4f30a5c5..b40fcd01dd 100644 --- a/src/google/protobuf/compiler/ruby/ruby_generator.h +++ b/src/google/protobuf/compiler/ruby/ruby_generator.h @@ -52,7 +52,7 @@ class PROTOC_EXPORT Generator : public CodeGenerator { bool Generate(const FileDescriptor* file, const string& parameter, GeneratorContext* generator_context, string* error) const override; - uint64 GetSupportedFeatures() const override { + uint64_t GetSupportedFeatures() const override { return FEATURE_PROTO3_OPTIONAL; } }; From 28cc693f3d05bab5f779b6fc8d514868130e81c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thiago=20C=2E=20D=27=C3=81vila?= Date: Tue, 2 Jun 2020 18:45:30 -0300 Subject: [PATCH 06/17] Initial module docstring for python _pb2 (#7528) Generated Python modules now have a module-level docstring. --- src/google/protobuf/compiler/python/python_generator.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/google/protobuf/compiler/python/python_generator.cc b/src/google/protobuf/compiler/python/python_generator.cc index a95b37f1ce..6f70e68551 100644 --- a/src/google/protobuf/compiler/python/python_generator.cc +++ b/src/google/protobuf/compiler/python/python_generator.cc @@ -183,7 +183,7 @@ void PrintTopBoilerplate(io::Printer* printer, const FileDescriptor* file, "# -*- coding: utf-8 -*-\n" "# Generated by the protocol buffer compiler. DO NOT EDIT!\n" "# source: $filename$\n" - "\n", + "\"\"\"Generated protocol buffer code.\"\"\"\n", "filename", file->name()); if (HasTopLevelEnums(file)) { printer->Print( From 5f5efe50c5bef20042645b51a697f58b0704ac89 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Wed, 3 Jun 2020 00:12:51 -0700 Subject: [PATCH 07/17] Added changelog entries for all changes already merged from google3. (#7585) --- CHANGES.txt | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index fee2a691bf..0f540f7752 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -3,15 +3,24 @@ Unreleased Changes C++: * Removed deprecated unsafe arena string accessors * Enabled heterogeneous lookup for std::string keys in maps. - * Improved the randomness of map ordering. * Removed implicit conversion from StringPiece to std::string * Fix use-after-destroy bug when the Map is allocated in the arena. - * Improved the randomness of proto map ordering + * Improved the randomness of map ordering + * Added stack overflow protection for text format with unknown fields + * Use std::hash for proto maps to help with portability. + * Added more Windows macros to proto whitelist. + * Arena constructors for map entry messages are now marked "explicit" + (for regular messages they were already explicit). Python: * Reject lowercase t for Timestamp json format. Fixes a conformance test. * Improved the error message when AttributeError is returned from __getattr__ in EnumTypeWrapper. + * Json format will print full_name directly for extensions. + + Java: + * Fixed a bug where setting optional proto3 enums with setFooValue() would + not mark the value as present. C#: * Dropped support for netstandard1.0 (replaced by support for netstandard1.1). From a6d1ed1712bc77ccdd5074256945778474c4e0b5 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 29 May 2020 13:53:26 +0200 Subject: [PATCH 08/17] annotate ByteString.Span and ByteString.Memory as SecuritySafeCritical --- .../Google.Protobuf.Test/ByteStringTest.cs | 16 ++++++++++++++++ csharp/src/Google.Protobuf/ByteString.cs | 19 +++++++++++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/csharp/src/Google.Protobuf.Test/ByteStringTest.cs b/csharp/src/Google.Protobuf.Test/ByteStringTest.cs index 84e6341e95..0ef8d8f4e7 100644 --- a/csharp/src/Google.Protobuf.Test/ByteStringTest.cs +++ b/csharp/src/Google.Protobuf.Test/ByteStringTest.cs @@ -233,5 +233,21 @@ namespace Google.Protobuf ByteString b2 = ByteString.CopyFrom(200, 1, 2, 3, 4); Assert.AreNotEqual(b1.GetHashCode(), b2.GetHashCode()); } + + [Test] + public void GetContentsAsReadOnlySpan() + { + var byteString = ByteString.CopyFrom(1, 2, 3, 4, 5); + var copied = byteString.Span.ToArray(); + CollectionAssert.AreEqual(byteString, copied); + } + + [Test] + public void GetContentsAsReadOnlyMemory() + { + var byteString = ByteString.CopyFrom(1, 2, 3, 4, 5); + var copied = byteString.Memory.ToArray(); + CollectionAssert.AreEqual(byteString, copied); + } } } \ No newline at end of file diff --git a/csharp/src/Google.Protobuf/ByteString.cs b/csharp/src/Google.Protobuf/ByteString.cs index 027c8d81c8..74dc865e9d 100644 --- a/csharp/src/Google.Protobuf/ByteString.cs +++ b/csharp/src/Google.Protobuf/ByteString.cs @@ -34,6 +34,7 @@ using System; using System.Collections; using System.Collections.Generic; using System.IO; +using System.Security; using System.Text; #if !NET35 using System.Threading; @@ -115,13 +116,27 @@ namespace Google.Protobuf /// Provides read-only access to the data of this . /// No data is copied so this is the most efficient way of accessing. /// - public ReadOnlySpan Span => new ReadOnlySpan(bytes); + public ReadOnlySpan Span + { + [SecuritySafeCritical] + get + { + return new ReadOnlySpan(bytes); + } + } /// /// Provides read-only access to the data of this . /// No data is copied so this is the most efficient way of accessing. /// - public ReadOnlyMemory Memory => new ReadOnlyMemory(bytes); + public ReadOnlyMemory Memory + { + [SecuritySafeCritical] + get + { + return new ReadOnlyMemory(bytes); + } + } #endif /// From 57be643a36ee4745de9da1503a418c193c929e8a Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Fri, 5 Jun 2020 15:26:05 -0400 Subject: [PATCH 09/17] Now that the paths are the same, use one ivar to track the values. This changes when we moved the WKTs to the root of the runtime package. --- .../compiler/objectivec/objectivec_helpers.cc | 16 ++++++---------- .../compiler/objectivec/objectivec_helpers.h | 3 +-- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc b/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc index df11689f61..9e250bf672 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc @@ -1585,8 +1585,7 @@ void ImportWriter::AddFile(const FileDescriptor* file, if (include_wkt_imports_) { const string header_name = "GPB" + FilePathBasename(file) + header_extension; - protobuf_framework_imports_.push_back(header_name); - protobuf_non_framework_imports_.push_back(header_name); + protobuf_imports_.push_back(header_name); } return; } @@ -1616,20 +1615,17 @@ void ImportWriter::AddFile(const FileDescriptor* file, } void ImportWriter::Print(io::Printer* printer) const { - assert(protobuf_non_framework_imports_.size() == - protobuf_framework_imports_.size()); - bool add_blank_line = false; - if (!protobuf_framework_imports_.empty()) { + if (!protobuf_imports_.empty()) { const string framework_name(ProtobufLibraryFrameworkName); const string cpp_symbol(ProtobufFrameworkImportSymbol(framework_name)); printer->Print( "#if $cpp_symbol$\n", "cpp_symbol", cpp_symbol); - for (std::vector::const_iterator iter = protobuf_framework_imports_.begin(); - iter != protobuf_framework_imports_.end(); ++iter) { + for (std::vector::const_iterator iter = protobuf_imports_.begin(); + iter != protobuf_imports_.end(); ++iter) { printer->Print( " #import <$framework_name$/$header$>\n", "framework_name", framework_name, @@ -1637,8 +1633,8 @@ void ImportWriter::Print(io::Printer* printer) const { } printer->Print( "#else\n"); - for (std::vector::const_iterator iter = protobuf_non_framework_imports_.begin(); - iter != protobuf_non_framework_imports_.end(); ++iter) { + for (std::vector::const_iterator iter = protobuf_imports_.begin(); + iter != protobuf_imports_.end(); ++iter) { printer->Print( " #import \"$header$\"\n", "header", *iter); diff --git a/src/google/protobuf/compiler/objectivec/objectivec_helpers.h b/src/google/protobuf/compiler/objectivec/objectivec_helpers.h index 5f91f4e4ed..05dc499407 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_helpers.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers.h @@ -305,8 +305,7 @@ class PROTOC_EXPORT ImportWriter { std::map proto_file_to_framework_name_; bool need_to_parse_mapping_file_; - std::vector protobuf_framework_imports_; - std::vector protobuf_non_framework_imports_; + std::vector protobuf_imports_; std::vector other_framework_imports_; std::vector other_imports_; }; From c6c8bab9e221ee268ad48494c00e64de88aa925a Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Fri, 5 Jun 2020 15:50:54 -0400 Subject: [PATCH 10/17] Move runtime import writing into a helper. File and ImportWriter duplicated the logic, so move it to a helper (on ImportWriter), and share the impl instead of duplicating it since it includes some conditional logic around Framework based import support. --- .../compiler/objectivec/objectivec_file.cc | 54 +++------------- .../compiler/objectivec/objectivec_file.h | 2 +- .../compiler/objectivec/objectivec_helpers.cc | 63 ++++++++++++------- .../compiler/objectivec/objectivec_helpers.h | 4 ++ 4 files changed, 54 insertions(+), 69 deletions(-) diff --git a/src/google/protobuf/compiler/objectivec/objectivec_file.cc b/src/google/protobuf/compiler/objectivec/objectivec_file.cc index 04733ca24c..8c0305b154 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_file.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_file.cc @@ -209,15 +209,15 @@ FileGenerator::FileGenerator(const FileDescriptor *file, const Options& options) FileGenerator::~FileGenerator() {} void FileGenerator::GenerateHeader(io::Printer *printer) { - std::set headers; + std::vector headers; // Generated files bundled with the library get minimal imports, everything // else gets the wrapper so everything is usable. if (is_bundled_proto_) { - headers.insert("GPBRootObject.h"); - headers.insert("GPBMessage.h"); - headers.insert("GPBDescriptor.h"); + headers.push_back("GPBDescriptor.h"); + headers.push_back("GPBMessage.h"); + headers.push_back("GPBRootObject.h"); } else { - headers.insert("GPBProtocolBuffers.h"); + headers.push_back("GPBProtocolBuffers.h"); } PrintFileRuntimePreamble(printer, headers); @@ -337,8 +337,8 @@ void FileGenerator::GenerateHeader(io::Printer *printer) { void FileGenerator::GenerateSource(io::Printer *printer) { // #import the runtime support. - std::set headers; - headers.insert("GPBProtocolBuffers_RuntimeSupport.h"); + std::vector headers; + headers.push_back("GPBProtocolBuffers_RuntimeSupport.h"); PrintFileRuntimePreamble(printer, headers); // Enums use atomic in the generated code, so add the system import as needed. @@ -590,48 +590,14 @@ void FileGenerator::GenerateSource(io::Printer *printer) { // files. This currently only supports the runtime coming from a framework // as defined by the official CocoaPod. void FileGenerator::PrintFileRuntimePreamble( - io::Printer* printer, const std::set& headers_to_import) const { + io::Printer* printer, const std::vector& headers_to_import) const { printer->Print( "// Generated by the protocol buffer compiler. DO NOT EDIT!\n" "// source: $filename$\n" "\n", "filename", file_->name()); - - const string framework_name(ProtobufLibraryFrameworkName); - const string cpp_symbol(ProtobufFrameworkImportSymbol(framework_name)); - - printer->Print( - "// This CPP symbol can be defined to use imports that match up to the framework\n" - "// imports needed when using CocoaPods.\n" - "#if !defined($cpp_symbol$)\n" - " #define $cpp_symbol$ 0\n" - "#endif\n" - "\n" - "#if $cpp_symbol$\n", - "cpp_symbol", cpp_symbol); - - - for (std::set::const_iterator iter = headers_to_import.begin(); - iter != headers_to_import.end(); ++iter) { - printer->Print( - " #import <$framework_name$/$header$>\n", - "header", *iter, - "framework_name", framework_name); - } - - printer->Print( - "#else\n"); - - for (std::set::const_iterator iter = headers_to_import.begin(); - iter != headers_to_import.end(); ++iter) { - printer->Print( - " #import \"$header$\"\n", - "header", *iter); - } - - printer->Print( - "#endif\n" - "\n"); + ImportWriter::PrintRuntimeImports(printer, headers_to_import, true); + printer->Print("\n"); } } // namespace objectivec diff --git a/src/google/protobuf/compiler/objectivec/objectivec_file.h b/src/google/protobuf/compiler/objectivec/objectivec_file.h index fd57263484..258bd13dc6 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_file.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_file.h @@ -72,7 +72,7 @@ class FileGenerator { const Options options_; void PrintFileRuntimePreamble( - io::Printer* printer, const std::set& headers_to_import) const; + io::Printer* printer, const std::vector& headers_to_import) const; }; } // namespace objectivec diff --git a/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc b/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc index 9e250bf672..24c4546d90 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc @@ -1618,30 +1618,7 @@ void ImportWriter::Print(io::Printer* printer) const { bool add_blank_line = false; if (!protobuf_imports_.empty()) { - const string framework_name(ProtobufLibraryFrameworkName); - const string cpp_symbol(ProtobufFrameworkImportSymbol(framework_name)); - - printer->Print( - "#if $cpp_symbol$\n", - "cpp_symbol", cpp_symbol); - for (std::vector::const_iterator iter = protobuf_imports_.begin(); - iter != protobuf_imports_.end(); ++iter) { - printer->Print( - " #import <$framework_name$/$header$>\n", - "framework_name", framework_name, - "header", *iter); - } - printer->Print( - "#else\n"); - for (std::vector::const_iterator iter = protobuf_imports_.begin(); - iter != protobuf_imports_.end(); ++iter) { - printer->Print( - " #import \"$header$\"\n", - "header", *iter); - } - printer->Print( - "#endif\n"); - + PrintRuntimeImports(printer, protobuf_imports_); add_blank_line = true; } @@ -1674,6 +1651,44 @@ void ImportWriter::Print(io::Printer* printer) const { } } +void ImportWriter::PrintRuntimeImports( + io::Printer* printer, + const std::vector& header_to_import, + bool default_cpp_symbol) { + const string framework_name(ProtobufLibraryFrameworkName); + const string cpp_symbol(ProtobufFrameworkImportSymbol(framework_name)); + + if (default_cpp_symbol) { + printer->Print( + "// This CPP symbol can be defined to use imports that match up to the framework\n" + "// imports needed when using CocoaPods.\n" + "#if !defined($cpp_symbol$)\n" + " #define $cpp_symbol$ 0\n" + "#endif\n" + "\n", + "cpp_symbol", cpp_symbol); + } + + printer->Print( + "#if $cpp_symbol$\n", + "cpp_symbol", cpp_symbol); + for (const auto& header : header_to_import) { + printer->Print( + " #import <$framework_name$/$header$>\n", + "framework_name", framework_name, + "header", header); + } + printer->Print( + "#else\n"); + for (const auto& header : header_to_import) { + printer->Print( + " #import \"$header$\"\n", + "header", header); + } + printer->Print( + "#endif\n"); +} + void ImportWriter::ParseFrameworkMappings() { need_to_parse_mapping_file_ = false; if (named_framework_to_proto_path_mappings_path_.empty()) { diff --git a/src/google/protobuf/compiler/objectivec/objectivec_helpers.h b/src/google/protobuf/compiler/objectivec/objectivec_helpers.h index 05dc499407..90e3fd1ab2 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_helpers.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers.h @@ -285,6 +285,10 @@ class PROTOC_EXPORT ImportWriter { void AddFile(const FileDescriptor* file, const string& header_extension); void Print(io::Printer *printer) const; + static void PrintRuntimeImports(io::Printer *printer, + const std::vector& header_to_import, + bool default_cpp_symbol = false); + private: class ProtoFrameworkCollector : public LineConsumer { public: From 4d6712e73995e0c64eb5f208e7388f824175b3b8 Mon Sep 17 00:00:00 2001 From: Anton Kast Date: Fri, 5 Jun 2020 16:39:02 -0700 Subject: [PATCH 11/17] Enable experimental presence detection in JS. (#7592) Co-authored-by: David L. Jones --- js/gulpfile.js | 4 +- js/proto3_test.js | 234 +++++++++++------- js/proto3_test.proto | 53 ++-- .../protobuf/compiler/js/js_generator.cc | 39 ++- .../protobuf/compiler/js/js_generator.h | 17 +- 5 files changed, 216 insertions(+), 131 deletions(-) diff --git a/js/gulpfile.js b/js/gulpfile.js index a81c011179..2e72a88898 100644 --- a/js/gulpfile.js +++ b/js/gulpfile.js @@ -72,7 +72,7 @@ gulp.task('genproto_group1_closure', function (cb) { }); gulp.task('genproto_group2_closure', function (cb) { - exec(protoc + ' --js_out=library=testproto_libs2,binary:. -I ../src -I . -I commonjs ' + group2Protos.join(' '), + exec(protoc + ' --experimental_allow_proto3_optional --js_out=library=testproto_libs2,binary:. -I ../src -I . -I commonjs ' + group2Protos.join(' '), function (err, stdout, stderr) { console.log(stdout); console.log(stderr); @@ -99,7 +99,7 @@ gulp.task('genproto_group1_commonjs', function (cb) { }); gulp.task('genproto_group2_commonjs', function (cb) { - exec('mkdir -p commonjs_out && ' + protoc + ' --js_out=import_style=commonjs,binary:commonjs_out -I ../src -I commonjs -I . ' + group2Protos.join(' '), + exec('mkdir -p commonjs_out && ' + protoc + ' --experimental_allow_proto3_optional --js_out=import_style=commonjs,binary:commonjs_out -I ../src -I commonjs -I . ' + group2Protos.join(' '), function (err, stdout, stderr) { console.log(stdout); console.log(stderr); diff --git a/js/proto3_test.js b/js/proto3_test.js index 79acc3c261..bd7cce517a 100644 --- a/js/proto3_test.js +++ b/js/proto3_test.js @@ -77,7 +77,7 @@ describe('proto3Test', function() { it('testEqualsProto3', function() { var msg1 = new proto.jspb.test.TestProto3(); var msg2 = new proto.jspb.test.TestProto3(); - msg2.setOptionalString(''); + msg2.setSingularString(''); assertTrue(jspb.Message.equals(msg1, msg2)); }); @@ -90,12 +90,12 @@ describe('proto3Test', function() { var msg = new proto.jspb.test.TestProto3(); // Setting should work normally. - msg.setOptionalString('optionalString'); - assertEquals(msg.getOptionalString(), 'optionalString'); + msg.setSingularString('optionalString'); + assertEquals(msg.getSingularString(), 'optionalString'); // Clearing should work too ... - msg.setOptionalString(''); - assertEquals(msg.getOptionalString(), ''); + msg.setSingularString(''); + assertEquals(msg.getSingularString(), ''); // ... and shouldn't affect the equality with a brand new message. assertTrue(jspb.Message.equals(msg, new proto.jspb.test.TestProto3())); @@ -107,6 +107,58 @@ describe('proto3Test', function() { it('testProto3FieldDefaults', function() { var msg = new proto.jspb.test.TestProto3(); + assertEquals(msg.getSingularInt32(), 0); + assertEquals(msg.getSingularInt64(), 0); + assertEquals(msg.getSingularUint32(), 0); + assertEquals(msg.getSingularUint64(), 0); + assertEquals(msg.getSingularSint32(), 0); + assertEquals(msg.getSingularSint64(), 0); + assertEquals(msg.getSingularFixed32(), 0); + assertEquals(msg.getSingularFixed64(), 0); + assertEquals(msg.getSingularSfixed32(), 0); + assertEquals(msg.getSingularSfixed64(), 0); + assertEquals(msg.getSingularFloat(), 0); + assertEquals(msg.getSingularDouble(), 0); + assertEquals(msg.getSingularString(), ''); + + // TODO(b/26173701): when we change bytes fields default getter to return + // Uint8Array, we'll want to switch this assertion to match the u8 case. + assertEquals(typeof msg.getSingularBytes(), 'string'); + assertEquals(msg.getSingularBytes_asU8() instanceof Uint8Array, true); + assertEquals(typeof msg.getSingularBytes_asB64(), 'string'); + assertEquals(msg.getSingularBytes().length, 0); + assertEquals(msg.getSingularBytes_asU8().length, 0); + assertEquals(msg.getSingularBytes_asB64(), ''); + + assertEquals(msg.getSingularForeignEnum(), + proto.jspb.test.Proto3Enum.PROTO3_FOO); + assertEquals(msg.getSingularForeignMessage(), undefined); + assertEquals(msg.getSingularForeignMessage(), undefined); + + assertEquals(msg.getRepeatedInt32List().length, 0); + assertEquals(msg.getRepeatedInt64List().length, 0); + assertEquals(msg.getRepeatedUint32List().length, 0); + assertEquals(msg.getRepeatedUint64List().length, 0); + assertEquals(msg.getRepeatedSint32List().length, 0); + assertEquals(msg.getRepeatedSint64List().length, 0); + assertEquals(msg.getRepeatedFixed32List().length, 0); + assertEquals(msg.getRepeatedFixed64List().length, 0); + assertEquals(msg.getRepeatedSfixed32List().length, 0); + assertEquals(msg.getRepeatedSfixed64List().length, 0); + assertEquals(msg.getRepeatedFloatList().length, 0); + assertEquals(msg.getRepeatedDoubleList().length, 0); + assertEquals(msg.getRepeatedStringList().length, 0); + assertEquals(msg.getRepeatedBytesList().length, 0); + assertEquals(msg.getRepeatedForeignEnumList().length, 0); + assertEquals(msg.getRepeatedForeignMessageList().length, 0); + }); + + /** + * Test presence for proto3 optional fields. + */ + it('testProto3Optional', function() { + var msg = new proto.jspb.test.TestProto3(); + assertEquals(msg.getOptionalInt32(), 0); assertEquals(msg.getOptionalInt64(), 0); assertEquals(msg.getOptionalUint32(), 0); @@ -135,51 +187,65 @@ describe('proto3Test', function() { assertEquals(msg.getOptionalForeignMessage(), undefined); assertEquals(msg.getOptionalForeignMessage(), undefined); - assertEquals(msg.getRepeatedInt32List().length, 0); - assertEquals(msg.getRepeatedInt64List().length, 0); - assertEquals(msg.getRepeatedUint32List().length, 0); - assertEquals(msg.getRepeatedUint64List().length, 0); - assertEquals(msg.getRepeatedSint32List().length, 0); - assertEquals(msg.getRepeatedSint64List().length, 0); - assertEquals(msg.getRepeatedFixed32List().length, 0); - assertEquals(msg.getRepeatedFixed64List().length, 0); - assertEquals(msg.getRepeatedSfixed32List().length, 0); - assertEquals(msg.getRepeatedSfixed64List().length, 0); - assertEquals(msg.getRepeatedFloatList().length, 0); - assertEquals(msg.getRepeatedDoubleList().length, 0); - assertEquals(msg.getRepeatedStringList().length, 0); - assertEquals(msg.getRepeatedBytesList().length, 0); - assertEquals(msg.getRepeatedForeignEnumList().length, 0); - assertEquals(msg.getRepeatedForeignMessageList().length, 0); + // Serializing an empty proto yields the empty string. + assertEquals(msg.serializeBinary().length, 0); - }); + // Values start as unset, but can be explicitly set even to default values + // like 0. + assertFalse(msg.hasOptionalInt32()); + msg.setOptionalInt32(0); + assertTrue(msg.hasOptionalInt32()); + + assertFalse(msg.hasOptionalInt64()); + msg.setOptionalInt64(0); + assertTrue(msg.hasOptionalInt64()); + + assertFalse(msg.hasOptionalString()); + msg.setOptionalString(""); + assertTrue(msg.hasOptionalString()); + + // Now the proto will have a non-zero size, even though its values are 0. + var serialized = msg.serializeBinary(); + assertNotEquals(serialized.length, 0); + var msg2 = proto.jspb.test.TestProto3.deserializeBinary(serialized); + assertTrue(msg2.hasOptionalInt32()); + assertTrue(msg2.hasOptionalInt64()); + assertTrue(msg2.hasOptionalString()); + + // We can clear fields to go back to empty. + msg2.clearOptionalInt32(); + assertFalse(msg2.hasOptionalInt32()); + + msg2.clearOptionalString(); + assertFalse(msg2.hasOptionalString()); + }); /** - * Test that all fields can be set and read via a serialization roundtrip. + * Test that all fields can be set ,and read via a serialization roundtrip. */ - it('testProto3FieldSetGet', function() { + it('testProto3FieldSetGet', function () { var msg = new proto.jspb.test.TestProto3(); - msg.setOptionalInt32(-42); - msg.setOptionalInt64(-0x7fffffff00000000); - msg.setOptionalUint32(0x80000000); - msg.setOptionalUint64(0xf000000000000000); - msg.setOptionalSint32(-100); - msg.setOptionalSint64(-0x8000000000000000); - msg.setOptionalFixed32(1234); - msg.setOptionalFixed64(0x1234567800000000); - msg.setOptionalSfixed32(-1234); - msg.setOptionalSfixed64(-0x1234567800000000); - msg.setOptionalFloat(1.5); - msg.setOptionalDouble(-1.5); - msg.setOptionalBool(true); - msg.setOptionalString('hello world'); - msg.setOptionalBytes(BYTES); + msg.setSingularInt32(-42); + msg.setSingularInt64(-0x7fffffff00000000); + msg.setSingularUint32(0x80000000); + msg.setSingularUint64(0xf000000000000000); + msg.setSingularSint32(-100); + msg.setSingularSint64(-0x8000000000000000); + msg.setSingularFixed32(1234); + msg.setSingularFixed64(0x1234567800000000); + msg.setSingularSfixed32(-1234); + msg.setSingularSfixed64(-0x1234567800000000); + msg.setSingularFloat(1.5); + msg.setSingularDouble(-1.5); + msg.setSingularBool(true); + msg.setSingularString('hello world'); + msg.setSingularBytes(BYTES); var submsg = new proto.jspb.test.ForeignMessage(); submsg.setC(16); - msg.setOptionalForeignMessage(submsg); - msg.setOptionalForeignEnum(proto.jspb.test.Proto3Enum.PROTO3_BAR); + msg.setSingularForeignMessage(submsg); + msg.setSingularForeignEnum(proto.jspb.test.Proto3Enum.PROTO3_BAR); msg.setRepeatedInt32List([-42]); msg.setRepeatedInt64List([-0x7fffffff00000000]); @@ -206,23 +272,23 @@ describe('proto3Test', function() { var serialized = msg.serializeBinary(); msg = proto.jspb.test.TestProto3.deserializeBinary(serialized); - assertEquals(msg.getOptionalInt32(), -42); - assertEquals(msg.getOptionalInt64(), -0x7fffffff00000000); - assertEquals(msg.getOptionalUint32(), 0x80000000); - assertEquals(msg.getOptionalUint64(), 0xf000000000000000); - assertEquals(msg.getOptionalSint32(), -100); - assertEquals(msg.getOptionalSint64(), -0x8000000000000000); - assertEquals(msg.getOptionalFixed32(), 1234); - assertEquals(msg.getOptionalFixed64(), 0x1234567800000000); - assertEquals(msg.getOptionalSfixed32(), -1234); - assertEquals(msg.getOptionalSfixed64(), -0x1234567800000000); - assertEquals(msg.getOptionalFloat(), 1.5); - assertEquals(msg.getOptionalDouble(), -1.5); - assertEquals(msg.getOptionalBool(), true); - assertEquals(msg.getOptionalString(), 'hello world'); - assertEquals(true, bytesCompare(msg.getOptionalBytes(), BYTES)); - assertEquals(msg.getOptionalForeignMessage().getC(), 16); - assertEquals(msg.getOptionalForeignEnum(), + assertEquals(msg.getSingularInt32(), -42); + assertEquals(msg.getSingularInt64(), -0x7fffffff00000000); + assertEquals(msg.getSingularUint32(), 0x80000000); + assertEquals(msg.getSingularUint64(), 0xf000000000000000); + assertEquals(msg.getSingularSint32(), -100); + assertEquals(msg.getSingularSint64(), -0x8000000000000000); + assertEquals(msg.getSingularFixed32(), 1234); + assertEquals(msg.getSingularFixed64(), 0x1234567800000000); + assertEquals(msg.getSingularSfixed32(), -1234); + assertEquals(msg.getSingularSfixed64(), -0x1234567800000000); + assertEquals(msg.getSingularFloat(), 1.5); + assertEquals(msg.getSingularDouble(), -1.5); + assertEquals(msg.getSingularBool(), true); + assertEquals(msg.getSingularString(), 'hello world'); + assertEquals(true, bytesCompare(msg.getSingularBytes(), BYTES)); + assertEquals(msg.getSingularForeignMessage().getC(), 16); + assertEquals(msg.getSingularForeignEnum(), proto.jspb.test.Proto3Enum.PROTO3_BAR); assertElementsEquals(msg.getRepeatedInt32List(), [-42]); @@ -327,20 +393,20 @@ describe('proto3Test', function() { // Set each primitive to a non-default value, then back to its default, to // ensure that the serialization is actually checking the value and not just // whether it has ever been set. - msg.setOptionalInt32(42); - msg.setOptionalInt32(0); - msg.setOptionalDouble(3.14); - msg.setOptionalDouble(0.0); - msg.setOptionalBool(true); - msg.setOptionalBool(false); - msg.setOptionalString('hello world'); - msg.setOptionalString(''); - msg.setOptionalBytes(goog.crypt.base64.encodeString('\u00FF\u00FF')); - msg.setOptionalBytes(''); - msg.setOptionalForeignMessage(new proto.jspb.test.ForeignMessage()); - msg.setOptionalForeignMessage(null); - msg.setOptionalForeignEnum(proto.jspb.test.Proto3Enum.PROTO3_BAR); - msg.setOptionalForeignEnum(proto.jspb.test.Proto3Enum.PROTO3_FOO); + msg.setSingularInt32(42); + msg.setSingularInt32(0); + msg.setSingularDouble(3.14); + msg.setSingularDouble(0.0); + msg.setSingularBool(true); + msg.setSingularBool(false); + msg.setSingularString('hello world'); + msg.setSingularString(''); + msg.setSingularBytes(goog.crypt.base64.encodeString('\u00FF\u00FF')); + msg.setSingularBytes(''); + msg.setSingularForeignMessage(new proto.jspb.test.ForeignMessage()); + msg.setSingularForeignMessage(null); + msg.setSingularForeignEnum(proto.jspb.test.Proto3Enum.PROTO3_BAR); + msg.setSingularForeignEnum(proto.jspb.test.Proto3Enum.PROTO3_FOO); msg.setOneofUint32(32); msg.clearOneofUint32(); @@ -355,27 +421,25 @@ describe('proto3Test', function() { it('testBytesFieldsInterop', function() { var msg = new proto.jspb.test.TestProto3(); // Set as a base64 string and check all the getters work. - msg.setOptionalBytes(BYTES_B64); - assertTrue(bytesCompare(msg.getOptionalBytes_asU8(), BYTES)); - assertTrue(bytesCompare(msg.getOptionalBytes_asB64(), BYTES)); - assertTrue(bytesCompare(msg.getOptionalBytes(), BYTES)); + msg.setSingularBytes(BYTES_B64); + assertTrue(bytesCompare(msg.getSingularBytes_asU8(), BYTES)); + assertTrue(bytesCompare(msg.getSingularBytes_asB64(), BYTES)); + assertTrue(bytesCompare(msg.getSingularBytes(), BYTES)); // Test binary serialize round trip doesn't break it. msg = proto.jspb.test.TestProto3.deserializeBinary(msg.serializeBinary()); - assertTrue(bytesCompare(msg.getOptionalBytes_asU8(), BYTES)); - assertTrue(bytesCompare(msg.getOptionalBytes_asB64(), BYTES)); - assertTrue(bytesCompare(msg.getOptionalBytes(), BYTES)); + assertTrue(bytesCompare(msg.getSingularBytes_asU8(), BYTES)); + assertTrue(bytesCompare(msg.getSingularBytes_asB64(), BYTES)); + assertTrue(bytesCompare(msg.getSingularBytes(), BYTES)); msg = new proto.jspb.test.TestProto3(); // Set as a Uint8Array and check all the getters work. - msg.setOptionalBytes(BYTES); - assertTrue(bytesCompare(msg.getOptionalBytes_asU8(), BYTES)); - assertTrue(bytesCompare(msg.getOptionalBytes_asB64(), BYTES)); - assertTrue(bytesCompare(msg.getOptionalBytes(), BYTES)); - + msg.setSingularBytes(BYTES); + assertTrue(bytesCompare(msg.getSingularBytes_asU8(), BYTES)); + assertTrue(bytesCompare(msg.getSingularBytes_asB64(), BYTES)); + assertTrue(bytesCompare(msg.getSingularBytes(), BYTES)); }); - it('testTimestampWellKnownType', function() { var msg = new proto.google.protobuf.Timestamp(); msg.fromDate(new Date(123456789)); diff --git a/js/proto3_test.proto b/js/proto3_test.proto index f23e19c9d8..1f6bbed060 100644 --- a/js/proto3_test.proto +++ b/js/proto3_test.proto @@ -35,24 +35,43 @@ import "testbinary.proto"; package jspb.test; message TestProto3 { - int32 optional_int32 = 1; - int64 optional_int64 = 2; - uint32 optional_uint32 = 3; - uint64 optional_uint64 = 4; - sint32 optional_sint32 = 5; - sint64 optional_sint64 = 6; - fixed32 optional_fixed32 = 7; - fixed64 optional_fixed64 = 8; - sfixed32 optional_sfixed32 = 9; - sfixed64 optional_sfixed64 = 10; - float optional_float = 11; - double optional_double = 12; - bool optional_bool = 13; - string optional_string = 14; - bytes optional_bytes = 15; + int32 singular_int32 = 1; + int64 singular_int64 = 2; + uint32 singular_uint32 = 3; + uint64 singular_uint64 = 4; + sint32 singular_sint32 = 5; + sint64 singular_sint64 = 6; + fixed32 singular_fixed32 = 7; + fixed64 singular_fixed64 = 8; + sfixed32 singular_sfixed32 = 9; + sfixed64 singular_sfixed64 = 10; + float singular_float = 11; + double singular_double = 12; + bool singular_bool = 13; + string singular_string = 14; + bytes singular_bytes = 15; - ForeignMessage optional_foreign_message = 19; - Proto3Enum optional_foreign_enum = 22; + ForeignMessage singular_foreign_message = 19; + Proto3Enum singular_foreign_enum = 22; + + optional int32 optional_int32 = 121; + optional int64 optional_int64 = 122; + optional uint32 optional_uint32 = 123; + optional uint64 optional_uint64 = 124; + optional sint32 optional_sint32 = 125; + optional sint64 optional_sint64 = 126; + optional fixed32 optional_fixed32 = 127; + optional fixed64 optional_fixed64 = 128; + optional sfixed32 optional_sfixed32 = 129; + optional sfixed64 optional_sfixed64 = 130; + optional float optional_float = 131; + optional double optional_double = 132; + optional bool optional_bool = 133; + optional string optional_string = 134; + optional bytes optional_bytes = 135; + + optional ForeignMessage optional_foreign_message = 136; + optional Proto3Enum optional_foreign_enum =137; repeated int32 repeated_int32 = 31; repeated int64 repeated_int64 = 32; diff --git a/src/google/protobuf/compiler/js/js_generator.cc b/src/google/protobuf/compiler/js/js_generator.cc index c1216f8ce9..02a89549d8 100644 --- a/src/google/protobuf/compiler/js/js_generator.cc +++ b/src/google/protobuf/compiler/js/js_generator.cc @@ -467,6 +467,7 @@ bool IgnoreMessage(const Descriptor* d) { return d->options().map_entry(); } // Does JSPB ignore this entire oneof? True only if all fields are ignored. bool IgnoreOneof(const OneofDescriptor* oneof) { + if (oneof->is_synthetic()) return true; for (int i = 0; i < oneof->field_count(); i++) { if (!IgnoreField(oneof->field(i))) { return false; @@ -576,6 +577,7 @@ std::string JSOneofIndex(const OneofDescriptor* oneof) { int index = -1; for (int i = 0; i < oneof->containing_type()->oneof_decl_count(); i++) { const OneofDescriptor* o = oneof->containing_type()->oneof_decl(i); + if (o->is_synthetic()) continue; // If at least one field in this oneof is not JSPB-ignored, count the oneof. for (int j = 0; j < o->field_count(); j++) { const FieldDescriptor* f = o->field(j); @@ -794,6 +796,11 @@ std::string DoubleToString(double value) { return PostProcessFloat(result); } +bool InRealOneof(const FieldDescriptor* field) { + return field->containing_oneof() && + !field->containing_oneof()->is_synthetic(); +} + // Return true if this is an integral field that should be represented as string // in JS. bool IsIntegralFieldWithStringJSType(const FieldDescriptor* field) { @@ -1173,7 +1180,7 @@ std::string RepeatedFieldsArrayName(const GeneratorOptions& options, bool HasOneofFields(const Descriptor* desc) { for (int i = 0; i < desc->field_count(); i++) { - if (desc->field(i)->containing_oneof()) { + if (InRealOneof(desc->field(i))) { return true; } } @@ -1411,15 +1418,9 @@ std::string GetPivot(const Descriptor* desc) { // generate extra methods (clearFoo() and hasFoo()) for this field. bool HasFieldPresence(const GeneratorOptions& options, const FieldDescriptor* field) { - if (field->is_repeated() || field->is_map()) { - // We say repeated fields and maps don't have presence, but we still do - // generate clearFoo() methods for them through a special case elsewhere. - return false; - } - - return field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE || - field->containing_oneof() != NULL || - field->file()->syntax() == FileDescriptor::SYNTAX_PROTO2; + // This returns false for repeated fields and maps, but we still do + // generate clearFoo() methods for these through a special case elsewhere. + return field->has_presence(); } // We use this to implement the semantics that same file can be generated @@ -2676,7 +2677,7 @@ void Generator::GenerateClassField(const GeneratorOptions& options, /* singular_if_not_packed = */ false), "class", GetMessagePath(options, field->containing_type()), "settername", "set" + JSGetterName(options, field), "oneoftag", - (field->containing_oneof() ? "Oneof" : ""), "repeatedtag", + (InRealOneof(field) ? "Oneof" : ""), "repeatedtag", (field->is_repeated() ? "Repeated" : "")); printer->Annotate("settername", field); @@ -2686,7 +2687,7 @@ void Generator::GenerateClassField(const GeneratorOptions& options, "\n" "\n", "index", JSFieldIndex(field), "oneofgroup", - (field->containing_oneof() ? (", " + JSOneofArray(options, field)) + (InRealOneof(field) ? (", " + JSOneofArray(options, field)) : "")); if (field->is_repeated()) { @@ -2811,8 +2812,7 @@ void Generator::GenerateClassField(const GeneratorOptions& options, " return jspb.Message.set$oneoftag$Field(this, $index$", "class", GetMessagePath(options, field->containing_type()), "settername", "set" + JSGetterName(options, field), "oneoftag", - (field->containing_oneof() ? "Oneof" : ""), "index", - JSFieldIndex(field)); + (InRealOneof(field) ? "Oneof" : ""), "index", JSFieldIndex(field)); printer->Annotate("settername", field); printer->Print( "$oneofgroup$, $type$value$rptvalueinit$$typeclose$);\n" @@ -2822,8 +2822,7 @@ void Generator::GenerateClassField(const GeneratorOptions& options, "type", untyped ? "/** @type{string|number|boolean|Array|undefined} */(" : "", "typeclose", untyped ? ")" : "", "oneofgroup", - (field->containing_oneof() ? (", " + JSOneofArray(options, field)) - : ""), + (InRealOneof(field) ? (", " + JSOneofArray(options, field)) : ""), "rptvalueinit", (field->is_repeated() ? " || []" : "")); } @@ -2899,8 +2898,8 @@ void Generator::GenerateClassField(const GeneratorOptions& options, "$index$$maybeoneofgroup$, ", "class", GetMessagePath(options, field->containing_type()), "clearername", "clear" + JSGetterName(options, field), - "maybeoneof", (field->containing_oneof() ? "Oneof" : ""), - "maybeoneofgroup", (field->containing_oneof() + "maybeoneof", (InRealOneof(field) ? "Oneof" : ""), + "maybeoneofgroup", (InRealOneof(field) ? (", " + JSOneofArray(options, field)) : ""), "index", JSFieldIndex(field)); @@ -2965,7 +2964,7 @@ void Generator::GenerateRepeatedPrimitiveHelperMethods( "\n", "type", untyped ? "/** @type{string|number|boolean|!Uint8Array} */(" : "", "typeclose", untyped ? ")" : "", "oneofgroup", - (field->containing_oneof() ? (", " + JSOneofArray(options, field)) : ""), + (InRealOneof(field) ? (", " + JSOneofArray(options, field)) : ""), "rptvalueinit", ""); // clang-format on } @@ -2994,7 +2993,7 @@ void Generator::GenerateRepeatedMessageHelperMethods( "\n" "\n", "index", JSFieldIndex(field), "oneofgroup", - (field->containing_oneof() ? (", " + JSOneofArray(options, field)) : ""), + (InRealOneof(field) ? (", " + JSOneofArray(options, field)) : ""), "ctor", GetMessagePath(options, field->message_type())); } diff --git a/src/google/protobuf/compiler/js/js_generator.h b/src/google/protobuf/compiler/js/js_generator.h index 49f19f6eb6..3e7221b77c 100644 --- a/src/google/protobuf/compiler/js/js_generator.h +++ b/src/google/protobuf/compiler/js/js_generator.h @@ -142,18 +142,21 @@ class PROTOC_EXPORT Generator : public CodeGenerator { Generator() {} virtual ~Generator() {} - virtual bool Generate(const FileDescriptor* file, - const std::string& parameter, GeneratorContext* context, - std::string* error) const { + bool Generate(const FileDescriptor* file, const std::string& parameter, + GeneratorContext* context, std::string* error) const override { *error = "Unimplemented Generate() method. Call GenerateAll() instead."; return false; } - virtual bool HasGenerateAll() const { return true; } + bool HasGenerateAll() const override { return true; } - virtual bool GenerateAll(const std::vector& files, - const std::string& parameter, - GeneratorContext* context, std::string* error) const; + bool GenerateAll(const std::vector& files, + const std::string& parameter, GeneratorContext* context, + std::string* error) const override; + + uint64 GetSupportedFeatures() const override { + return FEATURE_PROTO3_OPTIONAL; + } private: void GenerateHeader(const GeneratorOptions& options, From 2655a0aa0f3a8984a80c886642f9969450aeccb2 Mon Sep 17 00:00:00 2001 From: Bart Hertog Date: Sat, 30 May 2020 21:47:23 +0200 Subject: [PATCH 12/17] Added Embedded Proto to third_party.md. --- docs/third_party.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/third_party.md b/docs/third_party.md index 11a6efb5b1..257f9d5952 100644 --- a/docs/third_party.md +++ b/docs/third_party.md @@ -18,6 +18,7 @@ These are projects we know about implementing Protocol Buffers for other program * C: https://github.com/squidfunk/protobluff * C: https://github.com/eerimoq/pbtools * C++: https://github.com/google/protobuf (Google-official implementation) +* C++: https://EmbeddedProto.com * C/C++: http://spbc.sf.net/ * C#: http://code.google.com/p/protobuf-csharp-port * C#: https://silentorbit.com/protobuf/ From ce8b7af46b82567c077611bae2e1285e04cfa7f2 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Fri, 5 Jun 2020 16:35:54 -0400 Subject: [PATCH 13/17] Update the brew workflow - Remove the `brew install`; the kokoro image already has it, just update/upgrade instead. - Remove `prune`, logs had: """ Error: Unknown command: prune """ - Remove `uninstall`, logs had: """ Error: Refusing to uninstall /usr/local/Cellar/cmake/3.16.4 and /usr/local/Cellar/icu4c/64.2 because they are required by ceres-solver, ffmpeg, harfbuzz, libass and opencv, which are currently installed. You can override this and force removal with: brew uninstall --ignore-dependencies node icu4c cmake wget """ - Skip installing some things since they are already in the base image, logs had: """ Warning: gflags 2.2.2 is already installed and up-to-date To reinstall 2.2.2, run `brew reinstall gflags` Warning: openssl@1.1 1.1.1g is already installed and up-to-date To reinstall 1.1.1g, run `brew reinstall openssl@1.1` Warning: pcre 8.44 is already installed and up-to-date To reinstall 8.44, run `brew reinstall pcre` """ - Don't install gpg gpg2 as gnupg is already installed, also use gpg instead of gpg2 for commands (and update the commands), logs had: """ kokoro/macos/prepare_build_macos_rc: line 44: gpg2: command not found kokoro/macos/prepare_build_macos_rc: line 45: gpg2: command not found """ - Add env guards to control all the option installs and only request them be installed in the cases that need it. This avoids having to install/update the things like ruby when some other tool only needed in some configs is install differently and could have conflicts. - Switch to brew for cocoapods to avoid compat issues on the supporting libraries. --- .../objectivec_cocoapods_integration/build.sh | 1 + kokoro/macos/prepare_build_macos_rc | 63 +++++++++++++++---- kokoro/macos/python/build.sh | 1 + kokoro/macos/python_cpp/build.sh | 1 + kokoro/macos/ruby23/build.sh | 2 + kokoro/macos/ruby24/build.sh | 2 + kokoro/macos/ruby25/build.sh | 2 + kokoro/macos/ruby26/build.sh | 2 + kokoro/macos/ruby27/build.sh | 2 + tests.sh | 2 - 10 files changed, 64 insertions(+), 14 deletions(-) diff --git a/kokoro/macos/objectivec_cocoapods_integration/build.sh b/kokoro/macos/objectivec_cocoapods_integration/build.sh index f96d2899d9..8f3c9b4c15 100755 --- a/kokoro/macos/objectivec_cocoapods_integration/build.sh +++ b/kokoro/macos/objectivec_cocoapods_integration/build.sh @@ -6,6 +6,7 @@ cd $(dirname $0)/../../.. # Prepare worker environment to run tests +KOKORO_INSTALL_COCOAPODS=yes source kokoro/macos/prepare_build_macos_rc ./tests.sh objectivec_cocoapods_integration diff --git a/kokoro/macos/prepare_build_macos_rc b/kokoro/macos/prepare_build_macos_rc index 2428750a8a..18ce60b17f 100755 --- a/kokoro/macos/prepare_build_macos_rc +++ b/kokoro/macos/prepare_build_macos_rc @@ -2,6 +2,8 @@ # # This script sets up a Kokoro MacOS worker for running Protobuf tests +set -eux + ## # Select Xcode version @@ -18,24 +20,61 @@ export CC=gcc export CXX=g++ ## -# Install Brew and core softwares +# Brew: update, then upgrade the installed tools to current version and install +# some needed ones not in the Kokoro base image. This ensure current versions +# of CMake, autotools, etc. + +# But first... +# +# The transitive deps of the installed tools need protobuf, but Kokoro manually +# installed it outside of brew so it needs to be removed so brew can install the +# tools (and a newer version of protobuf). g/kokoro-users/7FRvQMUdN40 about why +# it is a manual install vs. a brew install in the first place. +sudo rm -rf \ + /usr/local/include/google/protobuf \ + /usr/local/bin/protoc +# Likewise, updating python can have issues because of some existing binaries. +sudo rm -rf \ + /usr/local/bin/2to3* \ + /usr/local/bin/idle3* \ + /usr/local/bin/pydoc3* \ + /usr/local/bin/python3* \ + /usr/local/bin/pyvenv* + +brew update +brew upgrade + +## +# Install Ruby -ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)" -source $HOME/.rvm/scripts/rvm -brew uninstall node icu4c cmake wget -brew prune -brew install gflags gpg gpg2 node openssl pcre ruby cmake wget -sudo chown -R $(whoami) /usr/local -brew postinstall node +if [[ "${KOKORO_INSTALL_RUBY:-}" == "yes" ]] ; then + brew install ruby +fi + +## +# Install Cocoapods + +if [[ "${KOKORO_INSTALL_COCOAPODS:-}" == "yes" ]] ; then + # The existing cocoapods was installed via gem, but that doesn't work well + # with the overlap in deps with things managed by brew (errors around ruby + # versions, etc.); so remove it and install in via brew instead. + gem uninstall -a "$(gem list | grep cocoapods | cut -d ' ' -f 1)" + brew install cocoapods +fi ## # Install Tox -sudo pip install tox==2.4.1 +if [[ "${KOKORO_INSTALL_TOX:-}" == "yes" ]] ; then + sudo pip install tox==2.4.1 +fi ## # Install RVM -gpg2 --keyserver hkp://keys.gnupg.net --recv-keys 409B6B1796C275462A1703113804BB82D39DC0E3 -command curl -sSL https://rvm.io/mpapis.asc | gpg2 --import - -curl -sSL https://get.rvm.io | bash -s stable --ruby +if [[ "${KOKORO_INSTALL_RVM:-}" == "yes" ]] ; then + curl -sSL https://rvm.io/mpapis.asc | gpg --import - + curl -sSL https://rvm.io/pkuczynski.asc | gpg --import - + + curl -sSL https://get.rvm.io | bash -s stable --ruby +fi diff --git a/kokoro/macos/python/build.sh b/kokoro/macos/python/build.sh index 6b17b95446..388e24b4ac 100755 --- a/kokoro/macos/python/build.sh +++ b/kokoro/macos/python/build.sh @@ -6,6 +6,7 @@ cd $(dirname $0)/../../.. # Prepare worker environment to run tests +KOKORO_INSTALL_TOX=yes source kokoro/macos/prepare_build_macos_rc ./tests.sh python diff --git a/kokoro/macos/python_cpp/build.sh b/kokoro/macos/python_cpp/build.sh index cb53def9d3..f86dd6f76e 100755 --- a/kokoro/macos/python_cpp/build.sh +++ b/kokoro/macos/python_cpp/build.sh @@ -6,6 +6,7 @@ cd $(dirname $0)/../../.. # Prepare worker environment to run tests +KOKORO_INSTALL_TOX=yes source kokoro/macos/prepare_build_macos_rc g++ --version diff --git a/kokoro/macos/ruby23/build.sh b/kokoro/macos/ruby23/build.sh index dc950a20d4..9317838781 100755 --- a/kokoro/macos/ruby23/build.sh +++ b/kokoro/macos/ruby23/build.sh @@ -6,6 +6,8 @@ cd $(dirname $0)/../../.. # Prepare worker environment to run tests +KOKORO_INSTALL_RUBY=yes +KOKORO_INSTALL_RVM=yes source kokoro/macos/prepare_build_macos_rc ./tests.sh ruby23 diff --git a/kokoro/macos/ruby24/build.sh b/kokoro/macos/ruby24/build.sh index 3c3a190d9d..51bb2e603b 100755 --- a/kokoro/macos/ruby24/build.sh +++ b/kokoro/macos/ruby24/build.sh @@ -6,6 +6,8 @@ cd $(dirname $0)/../../.. # Prepare worker environment to run tests +KOKORO_INSTALL_RUBY=yes +KOKORO_INSTALL_RVM=yes source kokoro/macos/prepare_build_macos_rc ./tests.sh ruby24 diff --git a/kokoro/macos/ruby25/build.sh b/kokoro/macos/ruby25/build.sh index 38e90aa75c..ba2d0a4d19 100755 --- a/kokoro/macos/ruby25/build.sh +++ b/kokoro/macos/ruby25/build.sh @@ -6,6 +6,8 @@ cd $(dirname $0)/../../.. # Prepare worker environment to run tests +KOKORO_INSTALL_RUBY=yes +KOKORO_INSTALL_RVM=yes source kokoro/macos/prepare_build_macos_rc ./tests.sh ruby25 diff --git a/kokoro/macos/ruby26/build.sh b/kokoro/macos/ruby26/build.sh index 6c33ed0a7e..5a4c243222 100755 --- a/kokoro/macos/ruby26/build.sh +++ b/kokoro/macos/ruby26/build.sh @@ -6,6 +6,8 @@ cd $(dirname $0)/../../.. # Prepare worker environment to run tests +KOKORO_INSTALL_RUBY=yes +KOKORO_INSTALL_RVM=yes source kokoro/macos/prepare_build_macos_rc ./tests.sh ruby26 diff --git a/kokoro/macos/ruby27/build.sh b/kokoro/macos/ruby27/build.sh index 16bcbd6cbd..b1529b9da0 100755 --- a/kokoro/macos/ruby27/build.sh +++ b/kokoro/macos/ruby27/build.sh @@ -6,6 +6,8 @@ cd $(dirname $0)/../../.. # Prepare worker environment to run tests +KOKORO_INSTALL_RUBY=yes +KOKORO_INSTALL_RVM=yes source kokoro/macos/prepare_build_macos_rc ./tests.sh ruby27 diff --git a/tests.sh b/tests.sh index 0a0e7988fc..eee3e99e96 100755 --- a/tests.sh +++ b/tests.sh @@ -305,8 +305,6 @@ build_objectivec_tvos_release() { } build_objectivec_cocoapods_integration() { - # Update pod to the latest version. - gem install cocoapods --no_document objectivec/Tests/CocoaPods/run_tests.sh } From 9f546ba61ba23213dfc0d8b603e49ead210fe7d2 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Tue, 9 Jun 2020 11:19:15 -0400 Subject: [PATCH 14/17] ObjC Support a runtime import override. Option to add a prefix to generated #imports incase ObjC Protos are used in a build system where one wants to avoid adding a header search path and have more complete imports. --- objectivec/README.md | 6 ++++++ .../compiler/objectivec/objectivec_file.cc | 5 ++++- .../compiler/objectivec/objectivec_generator.cc | 7 +++++++ .../compiler/objectivec/objectivec_helpers.cc | 17 ++++++++++++++++- .../compiler/objectivec/objectivec_helpers.h | 4 ++++ 5 files changed, 37 insertions(+), 2 deletions(-) diff --git a/objectivec/README.md b/objectivec/README.md index 69fe631644..5982939290 100644 --- a/objectivec/README.md +++ b/objectivec/README.md @@ -172,6 +172,12 @@ supported keys are: lot of proto files included in it; and having multiple lines makes things easier to read. + * `runtime_import_prefix`: The `value` used for this key to be used as a + prefix on `#import`s of runtime provided headers in the generated files. + When integrating ObjC protos into a build system, this can be used to avoid + having to add the runtime directory to the header search path since the + generate `#import` will be more complete. + Contributing ------------ diff --git a/src/google/protobuf/compiler/objectivec/objectivec_file.cc b/src/google/protobuf/compiler/objectivec/objectivec_file.cc index 8c0305b154..58ca9c195b 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_file.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_file.cc @@ -242,6 +242,7 @@ void FileGenerator::GenerateHeader(io::Printer *printer) { ImportWriter import_writer( options_.generate_for_named_framework, options_.named_framework_to_proto_path_mappings_path, + options_.runtime_import_prefix, is_bundled_proto_); const string header_extension(kHeaderExtension); for (int i = 0; i < file_->public_dependency_count(); i++) { @@ -355,6 +356,7 @@ void FileGenerator::GenerateSource(io::Printer *printer) { ImportWriter import_writer( options_.generate_for_named_framework, options_.named_framework_to_proto_path_mappings_path, + options_.runtime_import_prefix, is_bundled_proto_); const string header_extension(kHeaderExtension); @@ -596,7 +598,8 @@ void FileGenerator::PrintFileRuntimePreamble( "// source: $filename$\n" "\n", "filename", file_->name()); - ImportWriter::PrintRuntimeImports(printer, headers_to_import, true); + ImportWriter::PrintRuntimeImports( + printer, headers_to_import, options_.runtime_import_prefix, true); printer->Print("\n"); } diff --git a/src/google/protobuf/compiler/objectivec/objectivec_generator.cc b/src/google/protobuf/compiler/objectivec/objectivec_generator.cc index 25338ad939..257e4f9abc 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_generator.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_generator.cc @@ -127,6 +127,13 @@ bool ObjectiveCGenerator::GenerateAll(const std::vector& // with generate_for_named_framework, or the relative path to it's include // path otherwise. generation_options.named_framework_to_proto_path_mappings_path = options[i].second; + } else if (options[i].first == "runtime_import_prefix") { + // Path to use as a prefix on #imports of runtime provided headers in the + // generated files. When integrating ObjC protos into a build system, + // this can be used to avoid having to add the runtime directory to the + // header search path since the generate #import will be more complete. + generation_options.runtime_import_prefix = + StripSuffixString(options[i].second, "/"); } else { *error = "error: Unknown generator option: " + options[i].first; return false; diff --git a/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc b/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc index 24c4546d90..8b2b719e03 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc @@ -1566,10 +1566,12 @@ bool ParseSimpleFile( ImportWriter::ImportWriter( const string& generate_for_named_framework, const string& named_framework_to_proto_path_mappings_path, + const string& runtime_import_prefix, bool include_wkt_imports) : generate_for_named_framework_(generate_for_named_framework), named_framework_to_proto_path_mappings_path_( named_framework_to_proto_path_mappings_path), + runtime_import_prefix_(runtime_import_prefix), include_wkt_imports_(include_wkt_imports), need_to_parse_mapping_file_(true) { } @@ -1618,7 +1620,7 @@ void ImportWriter::Print(io::Printer* printer) const { bool add_blank_line = false; if (!protobuf_imports_.empty()) { - PrintRuntimeImports(printer, protobuf_imports_); + PrintRuntimeImports(printer, protobuf_imports_, runtime_import_prefix_); add_blank_line = true; } @@ -1654,7 +1656,20 @@ void ImportWriter::Print(io::Printer* printer) const { void ImportWriter::PrintRuntimeImports( io::Printer* printer, const std::vector& header_to_import, + const string& runtime_import_prefix, bool default_cpp_symbol) { + + // Given an override, use that. + if (!runtime_import_prefix.empty()) { + for (const auto& header : header_to_import) { + printer->Print( + " #import \"$import_prefix$/$header$\"\n", + "import_prefix", runtime_import_prefix, + "header", header); + } + return; + } + const string framework_name(ProtobufLibraryFrameworkName); const string cpp_symbol(ProtobufFrameworkImportSymbol(framework_name)); diff --git a/src/google/protobuf/compiler/objectivec/objectivec_helpers.h b/src/google/protobuf/compiler/objectivec/objectivec_helpers.h index 90e3fd1ab2..02f540f874 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_helpers.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers.h @@ -53,6 +53,7 @@ struct Options { std::vector expected_prefixes_suppressions; string generate_for_named_framework; string named_framework_to_proto_path_mappings_path; + string runtime_import_prefix; }; // Escape C++ trigraphs by escaping question marks to "\?". @@ -279,6 +280,7 @@ class PROTOC_EXPORT ImportWriter { public: ImportWriter(const string& generate_for_named_framework, const string& named_framework_to_proto_path_mappings_path, + const string& runtime_import_prefix, bool include_wkt_imports); ~ImportWriter(); @@ -287,6 +289,7 @@ class PROTOC_EXPORT ImportWriter { static void PrintRuntimeImports(io::Printer *printer, const std::vector& header_to_import, + const string& runtime_import_prefix, bool default_cpp_symbol = false); private: @@ -305,6 +308,7 @@ class PROTOC_EXPORT ImportWriter { const string generate_for_named_framework_; const string named_framework_to_proto_path_mappings_path_; + const string runtime_import_prefix_; const bool include_wkt_imports_; std::map proto_file_to_framework_name_; bool need_to_parse_mapping_file_; From f1ce8663ac88df54cf212d29ce5123b69203b135 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Tue, 9 Jun 2020 15:40:29 -0700 Subject: [PATCH 15/17] De-duplicated code to generate PHP protos and install phpunit. (#7605) * De-duplicated code to generate PHP protos and install phpunit. * Removed all references to generate_php_test_proto. * Replaced with internal_build_cpp. * Make Timestamp::__construct() static to avoid conflicts with MongoDB. * Replicated PHPUnit versions and added new script to Makefile.am. * Fixed filename in Makefile.am. * Disabled test that SEGV's on macOS. * Removed extraneous "set -e". * Make sure generate_protos.sh happens on every test path. * Removed stray '$' chars. * Added proper support for aggregate_metadata tests. But now I get a stack overflow. Stack overflow: /Users/haberman/code/protobuf/php/tests/generated/GPBMetadata/Proto/TestDescriptors.php:16 /Users/haberman/code/protobuf/php/tests/generated/GPBMetadata/Proto/TestDescriptors.php:16 /Users/haberman/code/protobuf/php/tests/generated/GPBMetadata/Proto/TestDescriptors.php:16 /Users/haberman/code/protobuf/php/tests/generated/GPBMetadata/Proto/TestDescriptors.php:16 /Users/haberman/code/protobuf/php/tests/generated/GPBMetadata/Proto/TestDescriptors.php:16 /Users/haberman/code/protobuf/php/tests/generated/GPBMetadata/Proto/TestDescriptors.php:16 /Users/haberman/code/protobuf/php/tests/generated/GPBMetadata/Proto/TestDescriptors.php:16 /Users/haberman/code/protobuf/php/tests/generated/GPBMetadata/Proto/TestDescriptors.php:16 namespace GPBMetadata\Proto; class TestDescriptors { public static $is_initialized = false; public static function initOnce() { $pool = \Google\Protobuf\Internal\DescriptorPool::getGeneratedPool(); if (static::$is_initialized == true) { return; } \GPBMetadata\Proto\TestDescriptors::initOnce(); $pool->internalAddGeneratedFile(hex2bin( "" ), true); static::$is_initialized = true; } } * Fixed and verified metadata aggregation testing. --- Makefile.am | 1 + php/composer.json | 4 +- php/ext/google/protobuf/message.c | 2 +- php/ext/google/protobuf/protobuf.h | 2 +- php/tests/compile_extension.sh | 5 +- php/tests/generate_protos.sh | 16 ++++++ php/tests/generated_class_test.php | 17 ++++++- php/tests/test.sh | 20 ++++++-- tests.sh | 80 +++++++----------------------- 9 files changed, 74 insertions(+), 73 deletions(-) create mode 100755 php/tests/generate_protos.sh diff --git a/Makefile.am b/Makefile.am index 8778b17cee..46d44d9599 100644 --- a/Makefile.am +++ b/Makefile.am @@ -909,6 +909,7 @@ php_EXTRA_DIST= \ php/tests/descriptors_test.php \ php/tests/encode_decode_test.php \ php/tests/gdb_test.sh \ + php/tests/generate_protos.sh \ php/tests/generated_class_test.php \ php/tests/generated_phpdoc_test.php \ php/tests/generated_service_test.php \ diff --git a/php/composer.json b/php/composer.json index b618ea1507..abcc293b2b 100644 --- a/php/composer.json +++ b/php/composer.json @@ -23,7 +23,7 @@ } }, "scripts": { - "test": "(cd tests && rm -rf generated && mkdir -p generated && ../../src/protoc --php_out=generated -I../../src -I. proto/empty/echo.proto proto/test.proto proto/test_include.proto proto/test_no_namespace.proto proto/test_prefix.proto proto/test_php_namespace.proto proto/test_empty_php_namespace.proto proto/test_reserved_enum_lower.proto proto/test_reserved_enum_upper.proto proto/test_reserved_enum_value_lower.proto proto/test_reserved_enum_value_upper.proto proto/test_reserved_message_lower.proto proto/test_reserved_message_upper.proto proto/test_service.proto proto/test_service_namespace.proto proto/test_wrapper_type_setters.proto proto/test_descriptors.proto) && (cd ../src && ./protoc --php_out=../php/tests/generated -I../php/tests -I. ../php/tests/proto/test_import_descriptor_proto.proto) && vendor/bin/phpunit", - "aggregate_metadata_test": "(cd tests && rm -rf generated && mkdir -p generated && ../../src/protoc --php_out=aggregate_metadata=foo#bar:generated -I../../src -I. proto/test.proto proto/test_include.proto && ../../src/protoc --php_out=generated -I../../src -I. proto/empty/echo.proto proto/test_no_namespace.proto proto/test_empty_php_namespace.proto proto/test_prefix.proto proto/test_php_namespace.proto proto/test_reserved_enum_lower.proto proto/test_reserved_enum_upper.proto proto/test_reserved_enum_value_lower.proto proto/test_reserved_enum_value_upper.proto proto/test_reserved_message_lower.proto proto/test_reserved_message_upper.proto proto/test_service.proto proto/test_service_namespace.proto proto/test_wrapper_type_setters.proto proto/test_descriptors.proto) && (cd ../src && ./protoc --php_out=aggregate_metadata=foo:../php/tests/generated -I../php/tests -I. ../php/tests/proto/test_import_descriptor_proto.proto) && vendor/bin/phpunit" + "test": "tests/generate_protos.sh && vendor/bin/phpunit", + "aggregate_metadata_test": "tests/generate_protos.sh --aggregate_metadata && vendor/bin/phpunit" } } diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index e8d4c6ff1d..036633781d 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -1726,7 +1726,7 @@ PHP_PROTO_INIT_SUBMSGCLASS_START("Google\\Protobuf\\Timestamp", 0 ,ZEND_ACC_PRIVATE TSRMLS_CC); PHP_PROTO_INIT_SUBMSGCLASS_END -PHP_METHOD(Timestamp, __construct) { +static PHP_METHOD(Timestamp, __construct) { init_file_timestamp(TSRMLS_C); INIT_MESSAGE_WITH_ARRAY; } diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index 4b8c7b15ad..520888733f 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -1292,7 +1292,7 @@ PHP_METHOD(Duration, setSeconds); PHP_METHOD(Duration, getNanos); PHP_METHOD(Duration, setNanos); -PHP_METHOD(Timestamp, __construct); +static PHP_METHOD(Timestamp, __construct); PHP_METHOD(Timestamp, fromDateTime); PHP_METHOD(Timestamp, toDateTime); PHP_METHOD(Timestamp, getSeconds); diff --git a/php/tests/compile_extension.sh b/php/tests/compile_extension.sh index 3d6759e5ea..ae0a6a3958 100755 --- a/php/tests/compile_extension.sh +++ b/php/tests/compile_extension.sh @@ -1,8 +1,10 @@ #!/bin/bash +set -ex + cd $(dirname $0) -if [ "$1" = "--release"]; then +if [ "$1" = "--release" ]; then CFLAGS="-Wall" else # To get debugging symbols in PHP itself, build PHP with: @@ -12,6 +14,5 @@ fi pushd ../ext/google/protobuf make clean || true -set -e phpize && ./configure --with-php-config=$(which php-config) CFLAGS="$CFLAGS" && make popd diff --git a/php/tests/generate_protos.sh b/php/tests/generate_protos.sh new file mode 100755 index 0000000000..0c2a5550aa --- /dev/null +++ b/php/tests/generate_protos.sh @@ -0,0 +1,16 @@ +#!/bin/bash + +set -ex + +cd `dirname $0` + +rm -rf generated +mkdir -p generated + +find proto -type f -name "*.proto"| xargs ../../src/protoc --php_out=generated -I../../src -I. + +if [ "$1" = "--aggregate_metadata" ]; then + # Overwrite some of the files to use aggregation. + AGGREGATED_FILES="proto/test.proto proto/test_include.proto proto/test_import_descriptor_proto.proto" + ../../src/protoc --php_out=aggregate_metadata=foo#bar:generated -I../../src -I. $AGGREGATED_FILES +fi diff --git a/php/tests/generated_class_test.php b/php/tests/generated_class_test.php index 6af00effe2..053697d2ec 100644 --- a/php/tests/generated_class_test.php +++ b/php/tests/generated_class_test.php @@ -1527,6 +1527,21 @@ class GeneratedClassTest extends TestBase public function testNoExceptionWithVarDump() { $m = new Sub(['a' => 1]); - var_dump($m); + /* + * This line currently segfaults on macOS with: + * + * frame #0: 0x00000001029936cc xdebug.so`xdebug_zend_hash_is_recursive + 4 + * frame #1: 0x00000001029a6736 xdebug.so`xdebug_var_export_text_ansi + 1006 + * frame #2: 0x00000001029a715d xdebug.so`xdebug_get_zval_value_text_ansi + 273 + * frame #3: 0x000000010298a441 xdebug.so`zif_xdebug_var_dump + 297 + * frame #4: 0x000000010298d558 xdebug.so`xdebug_execute_internal + 640 + * frame #5: 0x000000010046d47f php`ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER + 364 + * frame #6: 0x000000010043cabc php`execute_ex + 44 + * frame #7: 0x000000010298d151 xdebug.so`xdebug_execute_ex + 1662 + * frame #8: 0x000000010046d865 php`ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER + 426 + * + * The value we are passing to var_dump() appears to be corrupt somehow. + */ + /* var_dump($m); */ } } diff --git a/php/tests/test.sh b/php/tests/test.sh index 3ecc0c7516..bb2ada36be 100755 --- a/php/tests/test.sh +++ b/php/tests/test.sh @@ -1,9 +1,23 @@ #!/bin/bash +set -ex + cd $(dirname $0) +./generate_protos.sh ./compile_extension.sh +PHP_VERSION=$(php -r "echo PHP_VERSION;") + +# Each version of PHPUnit supports a fairly narrow range of PHP versions. +case "$PHP_VERSION" in + 5.6.*) PHPUNIT=phpunit-5.6.8.phar;; + 7.0.*) PHPUNIT=phpunit-5.6.0.phar;; # Oddly older than for 5.6. Not sure the reason. + 7.3.*) PHPUNIT=phpunit-8.phar;; +esac + +[ -f $PHPUNIT ] || wget https://phar.phpunit.de/$PHPUNIT + tests=( array_test.php encode_decode_test.php generated_class_test.php map_field_test.php well_known_test.php descriptors_test.php wrapper_type_setters_test.php) for t in "${tests[@]}" @@ -11,7 +25,7 @@ do echo "****************************" echo "* $t" echo "****************************" - php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php $t + php -dextension=../ext/google/protobuf/modules/protobuf.so $PHPUNIT --bootstrap autoload.php $t echo "" done @@ -20,7 +34,7 @@ do echo "****************************" echo "* $t persistent" echo "****************************" - php -d protobuf.keep_descriptor_pool_after_request=1 -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php $t + php -d protobuf.keep_descriptor_pool_after_request=1 -dextension=../ext/google/protobuf/modules/protobuf.so $PHPUNIT --bootstrap autoload.php $t echo "" done @@ -40,6 +54,6 @@ valgrind --leak-check=yes php -d protobuf.keep_descriptor_pool_after_request=1 - # echo "****************************" # echo "* $t (memory leak)" # echo "****************************" -# valgrind --leak-check=yes php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php $t +# valgrind --leak-check=yes php -dextension=../ext/google/protobuf/modules/protobuf.so $PHPUNIT --bootstrap autoload.php $t # echo "" # done diff --git a/tests.sh b/tests.sh index eee3e99e96..ca998e1e17 100755 --- a/tests.sh +++ b/tests.sh @@ -455,48 +455,16 @@ build_javascript() { cd conformance && make test_nodejs && cd .. } -generate_php_test_proto() { - internal_build_cpp - pushd php/tests - # Generate test file - rm -rf generated - mkdir generated - ../../src/protoc --php_out=generated \ - -I../../src -I. \ - proto/empty/echo.proto \ - proto/test.proto \ - proto/test_include.proto \ - proto/test_no_namespace.proto \ - proto/test_prefix.proto \ - proto/test_php_namespace.proto \ - proto/test_empty_php_namespace.proto \ - proto/test_reserved_enum_lower.proto \ - proto/test_reserved_enum_upper.proto \ - proto/test_reserved_enum_value_lower.proto \ - proto/test_reserved_enum_value_upper.proto \ - proto/test_reserved_message_lower.proto \ - proto/test_reserved_message_upper.proto \ - proto/test_service.proto \ - proto/test_service_namespace.proto \ - proto/test_wrapper_type_setters.proto \ - proto/test_descriptors.proto - pushd ../../src - ./protoc --php_out=../php/tests/generated -I../php/tests -I. \ - ../php/tests/proto/test_import_descriptor_proto.proto - popd - popd -} - use_php() { VERSION=$1 export PATH=/usr/local/php-${VERSION}/bin:$PATH - generate_php_test_proto + internal_build_cpp } use_php_zts() { VERSION=$1 export PATH=/usr/local/php-${VERSION}-zts/bin:$PATH - generate_php_test_proto + internal_build_cpp } build_php5.5() { @@ -505,11 +473,9 @@ build_php5.5() { pushd php rm -rf vendor composer update - ./vendor/bin/phpunit - popd - pushd conformance - make test_php + composer test popd + (cd conformance && make test_php) } build_php5.5_c() { @@ -532,6 +498,7 @@ build_php5.5_mixed() { rm -rf vendor composer update tests/compile_extension.sh + tests/generate_protos.sh php -dextension=./ext/google/protobuf/modules/protobuf.so ./vendor/bin/phpunit popd } @@ -555,11 +522,9 @@ build_php5.6() { pushd php rm -rf vendor composer update - ./vendor/bin/phpunit - popd - pushd conformance - make test_php + composer test popd + (cd conformance && make test_php) } build_php5.6_c() { @@ -582,6 +547,7 @@ build_php5.6_mixed() { rm -rf vendor composer update tests/compile_extension.sh + tests/generate_protos.sh php -dextension=./ext/google/protobuf/modules/protobuf.so ./vendor/bin/phpunit popd } @@ -601,18 +567,13 @@ build_php5.6_zts_c() { } build_php5.6_mac() { - generate_php_test_proto + internal_build_cpp # Install PHP curl -s https://php-osx.liip.ch/install.sh | bash -s 5.6 PHP_FOLDER=`find /usr/local -type d -name "php5-5.6*"` # The folder name may change upon time test ! -z "$PHP_FOLDER" export PATH="$PHP_FOLDER/bin:$PATH" - # Install phpunit - curl https://phar.phpunit.de/phpunit-5.6.8.phar -L -o phpunit.phar - chmod +x phpunit.phar - sudo mv phpunit.phar /usr/local/bin/phpunit - # Install valgrind echo "#! /bin/bash" > valgrind chmod ug+x valgrind @@ -628,7 +589,7 @@ build_php7.0() { pushd php rm -rf vendor composer update - ./vendor/bin/phpunit + composer test popd (cd conformance && make test_php) } @@ -653,6 +614,7 @@ build_php7.0_mixed() { rm -rf vendor composer update tests/compile_extension.sh + tests/generate_protos.sh php -dextension=./ext/google/protobuf/modules/protobuf.so ./vendor/bin/phpunit popd } @@ -672,18 +634,13 @@ build_php7.0_zts_c() { } build_php7.0_mac() { - generate_php_test_proto + internal_build_cpp # Install PHP curl -s https://php-osx.liip.ch/install.sh | bash -s 7.0 PHP_FOLDER=`find /usr/local -type d -name "php5-7.0*"` # The folder name may change upon time test ! -z "$PHP_FOLDER" export PATH="$PHP_FOLDER/bin:$PATH" - # Install phpunit - curl https://phar.phpunit.de/phpunit-5.6.0.phar -L -o phpunit.phar - chmod +x phpunit.phar - sudo mv phpunit.phar /usr/local/bin/phpunit - # Install valgrind echo "#! /bin/bash" > valgrind chmod ug+x valgrind @@ -695,7 +652,7 @@ build_php7.0_mac() { } build_php7.3_mac() { - generate_php_test_proto + internal_build_cpp # Install PHP # We can't test PHP 7.4 with these binaries yet: # https://github.com/liip/php-osx/issues/276 @@ -704,11 +661,6 @@ build_php7.3_mac() { test ! -z "$PHP_FOLDER" export PATH="$PHP_FOLDER/bin:$PATH" - # Install phpunit - curl https://phar.phpunit.de/phpunit-8.phar -L -o phpunit.phar - chmod +x phpunit.phar - sudo mv phpunit.phar /usr/local/bin/phpunit - # Install valgrind echo "#! /bin/bash" > valgrind chmod ug+x valgrind @@ -734,7 +686,7 @@ build_php7.1() { pushd php rm -rf vendor composer update - ./vendor/bin/phpunit + composer test popd (cd conformance && make test_php) } @@ -759,6 +711,7 @@ build_php7.1_mixed() { rm -rf vendor composer update tests/compile_extension.sh + tests/generate_protos.sh php -dextension=./ext/google/protobuf/modules/protobuf.so ./vendor/bin/phpunit popd } @@ -782,7 +735,7 @@ build_php7.4() { pushd php rm -rf vendor composer update - ./vendor/bin/phpunit + composer test popd (cd conformance && make test_php) } @@ -808,6 +761,7 @@ build_php7.4_mixed() { rm -rf vendor composer update tests/compile_extension.sh + tests/generate_protos.sh php -dextension=./ext/google/protobuf/modules/protobuf.so ./vendor/bin/phpunit popd (cd php/ext/google/protobuf && phpize --clean) From ff70af6cfc19d77f845981dfee755e1490ed70bb Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Mon, 11 May 2020 09:52:28 +0100 Subject: [PATCH 16/17] Changes how JSON formatting works for fields supporting presence Fixes #7486. Note that this changes the behavior for message fields where "WithFormatDefaultValues(true)" has been specified. This is effectively fixing a bug, but will need to be noted in the release notes. Basically, FormatDefaultValues only affects fields that don't support presence - most commonly, singular primitive non-optional fields in proto3. --- conformance/failure_list_csharp.txt | 1 - .../Google.Protobuf.Test/JsonFormatterTest.cs | 58 ++++++++++++++++++- .../Google.Protobuf.Test/JsonParserTest.cs | 11 ++++ csharp/src/Google.Protobuf/JsonFormatter.cs | 37 +++++++----- 4 files changed, 88 insertions(+), 19 deletions(-) diff --git a/conformance/failure_list_csharp.txt b/conformance/failure_list_csharp.txt index f82f0f2d49..31bdf25ebf 100644 --- a/conformance/failure_list_csharp.txt +++ b/conformance/failure_list_csharp.txt @@ -1,4 +1,3 @@ Recommended.Proto3.JsonInput.BytesFieldBase64Url.JsonOutput Recommended.Proto3.JsonInput.BytesFieldBase64Url.ProtobufOutput -Required.Proto2.JsonInput.StoresDefaultPrimitive.Validator Recommended.Proto2.JsonInput.FieldNameExtension.Validator diff --git a/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs b/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs index b07a84161a..5e9730b2af 100644 --- a/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs +++ b/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs @@ -40,6 +40,7 @@ using Google.Protobuf.Reflection; using static Google.Protobuf.JsonParserTest; // For WrapInQuotes using System.IO; using Google.Protobuf.Collections; +using ProtobufUnittest; namespace Google.Protobuf { @@ -153,6 +154,48 @@ namespace Google.Protobuf AssertJson(expectedText, actualText); } + [Test] + public void WithFormatDefaultValues_DoesNotAffectMessageFields() + { + var message = new TestAllTypes(); + var formatter = new JsonFormatter(JsonFormatter.Settings.Default.WithFormatDefaultValues(true)); + var json = formatter.Format(message); + Assert.IsFalse(json.Contains("\"singleNestedMessage\"")); + Assert.IsFalse(json.Contains("\"singleForeignMessage\"")); + Assert.IsFalse(json.Contains("\"singleImportMessage\"")); + } + + [Test] + public void WithFormatDefaultValues_DoesNotAffectProto3OptionalFields() + { + var message = new TestProto3Optional(); + message.OptionalInt32 = 0; + var formatter = new JsonFormatter(JsonFormatter.Settings.Default.WithFormatDefaultValues(true)); + var json = formatter.Format(message); + // The non-optional proto3 fields are formatted, as is the optional-but-specified field. + AssertJson("{ 'optionalInt32': 0, 'singularInt32': 0, 'singularInt64': '0' }", json); + } + + [Test] + public void WithFormatDefaultValues_DoesNotAffectProto2Fields() + { + var message = new TestProtos.Proto2.ForeignMessage(); + message.C = 0; + var formatter = new JsonFormatter(JsonFormatter.Settings.Default.WithFormatDefaultValues(true)); + var json = formatter.Format(message); + // The specified field is formatted, but the non-specified field (d) is not. + AssertJson("{ 'c': 0 }", json); + } + + [Test] + public void WithFormatDefaultValues_DoesNotAffectOneofFields() + { + var message = new TestOneof(); + var formatter = new JsonFormatter(JsonFormatter.Settings.Default.WithFormatDefaultValues(true)); + var json = formatter.Format(message); + AssertJson("{ }", json); + } + [Test] public void RepeatedField() { @@ -313,14 +356,16 @@ namespace Google.Protobuf } [Test] - public void WrapperFormatting_IncludeNull() + public void WrapperFormatting_FormatDefaultValuesDoesNotFormatNull() { // The actual JSON here is very large because there are lots of fields. Just test a couple of them. var message = new TestWellKnownTypes { Int32Field = 10 }; var formatter = new JsonFormatter(JsonFormatter.Settings.Default.WithFormatDefaultValues(true)); var actualJson = formatter.Format(message); - Assert.IsTrue(actualJson.Contains("\"int64Field\": null")); - Assert.IsFalse(actualJson.Contains("\"int32Field\": null")); + // This *used* to include "int64Field": null, but that was a bug. + // WithDefaultValues should not affect message fields, including wrapper types. + Assert.IsFalse(actualJson.Contains("\"int64Field\": null")); + Assert.IsTrue(actualJson.Contains("\"int32Field\": 10")); } [Test] @@ -602,6 +647,13 @@ namespace Google.Protobuf AssertWriteValue(value, "[ 1, 2, 3 ]"); } + [Test] + public void Proto2_DefaultValuesWritten() + { + var value = new ProtobufTestMessages.Proto2.TestAllTypesProto2() { FieldName13 = 0 }; + AssertWriteValue(value, "{ 'FieldName13': 0 }"); + } + private static void AssertWriteValue(object value, string expectedJson) { var writer = new StringWriter(); diff --git a/csharp/src/Google.Protobuf.Test/JsonParserTest.cs b/csharp/src/Google.Protobuf.Test/JsonParserTest.cs index a6cf04ab7a..f9f8cde050 100644 --- a/csharp/src/Google.Protobuf.Test/JsonParserTest.cs +++ b/csharp/src/Google.Protobuf.Test/JsonParserTest.cs @@ -34,6 +34,7 @@ using Google.Protobuf.Reflection; using Google.Protobuf.TestProtos; using Google.Protobuf.WellKnownTypes; using NUnit.Framework; +using ProtobufTestMessages.Proto2; using System; namespace Google.Protobuf @@ -949,6 +950,16 @@ namespace Google.Protobuf Assert.Throws(() => TestAllTypes.Parser.ParseJson(json)); } + [Test] + public void Proto2_DefaultValuesPreserved() + { + string json = "{ \"FieldName13\": 0 }"; + var parsed = TestAllTypesProto2.Parser.ParseJson(json); + Assert.False(parsed.HasFieldName10); + Assert.True(parsed.HasFieldName13); + Assert.AreEqual(0, parsed.FieldName13); + } + [Test] [TestCase("5")] [TestCase("\"text\"")] diff --git a/csharp/src/Google.Protobuf/JsonFormatter.cs b/csharp/src/Google.Protobuf/JsonFormatter.cs index 5aaefe7a89..699a81de51 100644 --- a/csharp/src/Google.Protobuf/JsonFormatter.cs +++ b/csharp/src/Google.Protobuf/JsonFormatter.cs @@ -221,19 +221,12 @@ namespace Google.Protobuf foreach (var field in fields.InFieldNumberOrder()) { var accessor = field.Accessor; - if (field.ContainingOneof != null && field.ContainingOneof.Accessor.GetCaseFieldDescriptor(message) != field) - { - continue; - } - // Omit default values unless we're asked to format them, or they're oneofs (where the default - // value is still formatted regardless, because that's how we preserve the oneof case). - object value = accessor.GetValue(message); - if (field.ContainingOneof == null && !settings.FormatDefaultValues && IsDefaultValue(accessor, value)) + var value = accessor.GetValue(message); + if (!ShouldFormatFieldValue(message, field, value)) { continue; } - // Okay, all tests complete: let's write the field value... if (!first) { writer.Write(PropertySeparator); @@ -248,6 +241,18 @@ namespace Google.Protobuf return !first; } + /// + /// Determines whether or not a field value should be serialized according to the field, + /// its value in the message, and the settings of this formatter. + /// + private bool ShouldFormatFieldValue(IMessage message, FieldDescriptor field, object value) => + field.HasPresence + // Fields that support presence *just* use that + ? field.Accessor.HasValue(message) + // Otherwise, format if either we've been asked to format default values, or if it's + // not a default value anyway. + : settings.FormatDefaultValues || !IsDefaultValue(field, value); + // Converted from java/core/src/main/java/com/google/protobuf/Descriptors.java internal static string ToJsonName(string name) { @@ -295,19 +300,19 @@ namespace Google.Protobuf writer.Write("null"); } - private static bool IsDefaultValue(IFieldAccessor accessor, object value) + private static bool IsDefaultValue(FieldDescriptor descriptor, object value) { - if (accessor.Descriptor.IsMap) + if (descriptor.IsMap) { IDictionary dictionary = (IDictionary) value; return dictionary.Count == 0; } - if (accessor.Descriptor.IsRepeated) + if (descriptor.IsRepeated) { IList list = (IList) value; return list.Count == 0; } - switch (accessor.Descriptor.FieldType) + switch (descriptor.FieldType) { case FieldType.Bool: return (bool) value == false; @@ -793,8 +798,10 @@ namespace Google.Protobuf } /// - /// Whether fields whose values are the default for the field type (e.g. 0 for integers) - /// should be formatted (true) or omitted (false). + /// Whether fields which would otherwise not be included in the formatted data + /// should be formatted even when the value is not present, or has the default value. + /// This option only affects fields which don't support "presence" (e.g. + /// singular non-optional proto3 primitive fields). /// public bool FormatDefaultValues { get; } From 1dae8fdd6271d091bbf43c0fd5855603ba533c1d Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Thu, 11 Jun 2020 05:53:44 +0100 Subject: [PATCH 17/17] Add support for proto2 JSON parsing in C# conformance tests --- .../Google.Protobuf.Conformance/Program.cs | 52 +++++++++++-------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/csharp/src/Google.Protobuf.Conformance/Program.cs b/csharp/src/Google.Protobuf.Conformance/Program.cs index d1093abee1..d721ecfcb7 100644 --- a/csharp/src/Google.Protobuf.Conformance/Program.cs +++ b/csharp/src/Google.Protobuf.Conformance/Program.cs @@ -83,44 +83,52 @@ namespace Google.Protobuf.Conformance private static ConformanceResponse PerformRequest(ConformanceRequest request, TypeRegistry typeRegistry) { + ExtensionRegistry proto2ExtensionRegistry = new ExtensionRegistry + { + ProtobufTestMessages.Proto2.TestMessagesProto2Extensions.ExtensionInt32, + ProtobufTestMessages.Proto2.TestAllTypesProto2.Types.MessageSetCorrectExtension1.Extensions.MessageSetExtension, + ProtobufTestMessages.Proto2.TestAllTypesProto2.Types.MessageSetCorrectExtension2.Extensions.MessageSetExtension + }; IMessage message; try { switch (request.PayloadCase) { case ConformanceRequest.PayloadOneofCase.JsonPayload: - if (request.TestCategory == global::Conformance.TestCategory.JsonIgnoreUnknownParsingTest) { + if (request.TestCategory == global::Conformance.TestCategory.JsonIgnoreUnknownParsingTest) + { return new ConformanceResponse { Skipped = "CSharp doesn't support skipping unknown fields in json parsing." }; } var parser = new JsonParser(new JsonParser.Settings(20, typeRegistry)); - message = parser.Parse(request.JsonPayload); - break; - case ConformanceRequest.PayloadOneofCase.ProtobufPayload: - { - if (request.MessageType.Equals("protobuf_test_messages.proto3.TestAllTypesProto3")) + switch (request.MessageType) { - message = ProtobufTestMessages.Proto3.TestAllTypesProto3.Parser.ParseFrom(request.ProtobufPayload); + case "protobuf_test_messages.proto3.TestAllTypesProto3": + message = parser.Parse(request.JsonPayload); + break; + case "protobuf_test_messages.proto2.TestAllTypesProto2": + message = parser.Parse(request.JsonPayload); + break; + default: + throw new Exception($" Protobuf request doesn't have specific payload type ({request.MessageType})"); } - else if (request.MessageType.Equals("protobuf_test_messages.proto2.TestAllTypesProto2")) - { - ExtensionRegistry registry = new ExtensionRegistry() - { - ProtobufTestMessages.Proto2.TestMessagesProto2Extensions.ExtensionInt32, - ProtobufTestMessages.Proto2.TestAllTypesProto2.Types.MessageSetCorrectExtension1.Extensions.MessageSetExtension, - ProtobufTestMessages.Proto2.TestAllTypesProto2.Types.MessageSetCorrectExtension2.Extensions.MessageSetExtension - }; - message = ProtobufTestMessages.Proto2.TestAllTypesProto2.Parser.WithExtensionRegistry(registry).ParseFrom(request.ProtobufPayload); - } - else + break; + case ConformanceRequest.PayloadOneofCase.ProtobufPayload: + switch (request.MessageType) { - throw new Exception(" Protobuf request doesn't have specific payload type"); + case "protobuf_test_messages.proto3.TestAllTypesProto3": + message = ProtobufTestMessages.Proto3.TestAllTypesProto3.Parser.ParseFrom(request.ProtobufPayload); + break; + case "protobuf_test_messages.proto2.TestAllTypesProto2": + message = ProtobufTestMessages.Proto2.TestAllTypesProto2.Parser + .WithExtensionRegistry(proto2ExtensionRegistry) + .ParseFrom(request.ProtobufPayload); + break; + default: + throw new Exception($" Protobuf request doesn't have specific payload type ({request.MessageType})"); } break; - } case ConformanceRequest.PayloadOneofCase.TextPayload: - { return new ConformanceResponse { Skipped = "CSharp doesn't support text format" }; - } default: throw new Exception("Unsupported request payload: " + request.PayloadCase); }