Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mixing with optional fields: core dump --experimental_allow_proto3_optional #7463

Closed
dwsutherland opened this issue May 4, 2020 · 3 comments

Comments

@dwsutherland
Copy link

dwsutherland commented May 4, 2020

Version: v3.12.0-rc-1
Language: Python3.7.5
Operating system: linux-x86_64
(Linux cortex-vbox 5.3.0-51-generic #44-Ubuntu SMP Wed Apr 22 21:09:44 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux)

protoc pre-compile:
https://github.com/protocolbuffers/protobuf/releases/download/v3.12.0-rc1/protoc-3.12.0-rc-1-linux-x86_64.zip

Install:
pip install protobuf==3.12.0-rc1

It appears the new (3.12.0-rc-1) optional fields work if all singular fields in a message are optional:

message PbTest {
    optional string test_string = 1;
    optional bool test_bool = 2;
    optional int32 test_int32 = 3;
}
>>> from cylc.flow.data_messages_pb2 import PbWorkflow, PbTest
>>> test1 = PbTest(test_string='foo', test_bool=True, test_int32=1234)
>>> test1
test_string: "foo"
test_bool: true
test_int32: 1234
>>> test2 = PbTest(test_string='', test_bool=False, test_int32=0)
>>> test2
test_string: ""
test_bool: false
test_int32: 0
>>> test1.MergeFrom(test2)
>>> test1
test_string: ""
test_bool: false
test_int32: 0

But not if mixed with non-optional:

message PbTest {
    string no_opt_string = 1;
    optional string test_string = 2;
    //optional bool test_bool = 2;
    //optional int32 test_int32 = 3;
}
>>> from cylc.flow.data_messages_pb2 import PbWorkflow, PbTest
>>> test1 = PbTest()
>>> test1
[libprotobuf FATAL google/protobuf/generated_message_reflection.cc:1015] CHECK failed: (has_bit_index) != (~0u): 
terminate called after throwing an instance of 'google::protobuf::FatalException'
  what():  CHECK failed: (has_bit_index) != (~0u): 
Aborted (core dumped)

Also doesn't mix with singular message fields, i.e.:

message PbMeta {
    string title = 1;
    string description = 2;
    string URL = 3;
    repeated string user_defined = 4;
}
message PbTest {
    //repeated string no_opt_strings = 1;
    optional string test_string = 2;
    Meta meta = 3;
    //optional bool test_bool = 2;
    //optional int32 test_int32 = 3;
}

(optional single message fields do work)

@haberman
Copy link
Member

haberman commented May 7, 2020

Thank you for the report, and for trying out a prerelease package! I have reproduced the error and will make sure a fix is in before 3.12.0 is released.

@haberman
Copy link
Member

The fix for this is released in https://github.com/protocolbuffers/protobuf/releases/tag/v3.12.0-rc2.

@dwsutherland
Copy link
Author

dwsutherland commented May 16, 2020

The fix for this is released in https://github.com/protocolbuffers/protobuf/releases/tag/v3.12.0-rc2.

Thanks @haberman ! . . . legend.

We'll implement this version in the coming week, thanks again 👍

clrpackages pushed a commit to clearlinux-pkgs/protobuf that referenced this issue Aug 5, 2020
… 3.12.4

Adam Cozzette (4):
      Allocated a custom option number for Analyze Re Graphene
      Attempt to mitigate "docker pull" failures
      Allocated extension number 1079 for grpc-graphql-gateway
      Allocated option number 1086 for ADLINK EdgeSDK

Andres Valdes (1):
      Correct @return in Any.unpack docblock (#7089)

Artem Kustikov (1):
      Fix js message pivot selection (#6813)

Asra Ali (1):
      fix ubsan warnings

Bo Yang (1):
      Remove add_proto_enumdesc and get_proto_enumdesc

Brian Wignall (1):
      Fix typos (#7050)

Charlie Jiang (1):
      Force to use U.S. English for Win32 error messages (#4317)

Christian Maurer (2):
      remove win32/64 conversion warning in generated code
      treat enum,bool as 32bit; shrink ReadVarint32

Chuck Atkins (2):
      Intel compiler: ifdef out an incorrectly evaluated is_pod type trait
      Intel compiler: silence noisy warning for incorrectly evaluated inlining

Daniel Azuma (1):
      Restore binary builds of Ruby 2.3 and 2.4 (#7529) (#7531)

Daniel G. Taylor (1):
      docs: add python-betterproto

Daniel Kurka (1):
      Project import generated by Copybara

Dave MacLachlan (6):
      Use references to Objective C classes instead of looking classes up by name.
      Change Objective C class references to using macros.
      Update pddm to work with clang-format
      Clean up some warning messages.
      Block subclassing
      Update GOOGLE_PROTOBUF_OBJC_VERSION to 30003

David L. Jones (9):
      Fix typo in Makefile.am.
      Make sure setup.py has a valid path.
      Don't expect to include Python ReadTheDocs metadata in the distribution.
      Change dependencies in the Conda environment.
      Update docs so we can generate better output from Sphinx. (#7295)
      Generate documentation for internal.container. (#7294)
      Add Ruby tests for oneof cases. (#7385)
      Add application note for explicit presence tracking. (#7390)
      Update protobuf version

Egor Pugin (1):
      bug: #7076 adds ERROR to windows portability files

Elliotte Rusty Harold (6):
      Update to 3.11.0 in docs
      deps: update guava to 28.2-android
      deps: update errorprone to 2.3.4 (#7125)
      Update JUnit and Truth (#7149)
      deps: update errorprone to 2.3.4 (#7125) (#7157)
      deps: update Guava to 29.0

Eric Walker (1):
      Rails2.7 segfaults (#7091)

Falko Axmann (1):
      cmake: extended protobuf_generate

Florian Enner (1):
      Added 3rd party Java library (QuickBuffers)

Greg Steuck (1):
      Typo in 'disciplines' (#7423)

James Roper (1):
      Add Cloudstate extensions

Jan Tattermusch (12):
      simpler fix
      Revert "Revert "C# upgrade dotnet SDK (#6877)" (#6888)" (#6920)
      enforce recursion depth checking for unknown fields
      enforce recursion depth checking for unknown fields (#7210)
      add benchmark for measuring raw primitive parsing speed
      address review comments
      Remove unnecessary branch from ReadTag (#7289)
      serialization benchmark improvements
      refactor WrapperBechmark
      improve ParseMessageBenchmark maintainability
      update Makefile.am
      Merge pull request #7434 from jtattermusch/csharp_expose_options

Jan-Gerd Tenberge (2):
      Add explicit cast to silence clang warning
      Add generate_descriptor_proto.sh output

Jesse Wilson (2):
      Reserve 1076, 1077 for Wire since and until
      Fix mergeable docs to be consistent with checks (#7021)

Jie Luo (4):
      add python38 (#7009)
      Add python3.8 in build_artifacts for linux and macos (#7019)
      Fix python release breakages by set MB_PYTHON_OSX_VER (#7038)
      Update README.md (#7329)

Jingwen Chen (2):
      Migrate from maven_jar to jvm_maven_import_external to prepare for Bazel 2.0
      Migrate from maven_jar to jvm_maven_import_external to prepare for Bazel 2.0

Joe Tsai (1):
      Add license header to benchmark/datasets .proto files

Jon Skeet (10):
      Register that the C# compiler supports proto3 presence.
      Support proto3 optional fields in the C# generator
      Regenerate C# code with the new generator, adding unittest_proto3_optional.proto
      Add unit tests for proto3 optional fields
      Add reflection support for proto3 optional fields
      Add new files into Makefile.am
      Don't generate Has/Clear members for proto2 message fields.
      Regenerate C# code based on the previous commit
      Fix to C# support library code
      Implement HasPresence for C#

Joshua Haberman (38):
      Fix for wrappers with a zero value (#7195)
      Fix for JSON serialization of 0/empty-valued wrapper types (#7198)
      Removed unnecessary includes of well-known types from PHP generator. (#7311)
      Sync from Piper @304070343
      Regenerated protos with ./generate_descriptor_proto.sh
      Revert mistake in the previous integration.
      Some fixes to make the tests pass on Bazel.
      Fixed build errors on MacOS.
      Set execute bit on files if and only if they begin with (#!). (#7347)
      Sync from Piper @305505267
      Fixed compile errors from lack of "override" keyword.
      Update to new upb version (#7372)
      Sync from Piper @306496510
      Sync from Piper @307316823
      Ruby: assigning 'nil' to submessage should clear the field. (#7397)
      Implemented proto3 presence for Ruby. (#7406)
      Howto doc for implementing proto3 presence in a code generator. (#7407)
      Added some info about Reflection, and a note about timeline. (#7416)
      Sync from Piper @308829107
      Added changelog for 3.12.x. (#7442)
      Updated version to 3.12.0-rc1. (#7449)
      Stop building binary gems for ruby <2.5. (#7453)
      Added changelog entry for removing binary gems for Ruby <2.5. (#7454)
      Cherry-pick the fix for protocolbuffers/protobuf#7463.
      Cherry-pick the fix to #7480 from #7485.
      Update protobuf version
      Updated Changelog for 3.12.0-rc2.
      Added background information about proto3 presence. (#7501)
      Fixed bug in map key sorting for Java TextFormat. (#7508)
      Update protobuf version
      Added a changelog entry about the Java fix. (#7516)
      Update protobuf version (#7535)
      Added changelog entry for 3.12.1.
      Switch to using git@ for clone, to avoid having to enter password. (#7556)
      Update protobuf version (#7557)
      Updated the changelog for the 3.12.2 release.
      Fixed Python release script to upload using twine instead of distutils. (#7571)
      Updated version to 3.12.3 and updated CHANGES.txt. (#7580)

Luca Santarella (1):
      Refactored ulong to zend_ulong for php7.4 compatibility (#7147)

Mark Schaller (1):
      Reserve extension for Bazel failure detail metadata

Martin Pärtel (2):
      Made protoc --descriptor_set_out deterministic.
      Destruct CodedOutputStream instead of using Trim()

Masaki Hara (2):
      Build extensions for Ruby 2.7 (#7027)
      Test in Ruby 2.7 (#7386)

Maxime Guerreiro (1):
      Cleanup the RPC Implementations section

Nick Presta (1):
      Update shell format in installation instructions.

Paul Yang (11):
      Persistent Descriptor Pool (#6899)
      Implement lazy loading of php class for proto messages (#6911)
      Add php 7.4 to docker image (#6971)
      Maven requires https connection (#7110)
      Call register_class before getClass from desc (#7077)
      Maven requires https connection (#7110) (#7114)
      Add continuous test for php7.4 on mac (#7153)
      Aggregate Metadata Files (#7155)
      Aggregate Metadata Files (#7155) (#7194)
      Drop 3.3, 3.4 and use single version docker images for all python tests (#7396)
      Ignore unknown enum value when ignore_unknown specified (#7455) (#7462)

Penelope Phippen (1):
      Call "Class#new" over rb_class_new_instance in decoding (#7352)

Protobuf Team (3):
      Project import generated by Copybara.
      Project import generated by Copybara
      Project import generated by Copybara

Rafi Kamal (33):
      Update protobuf version (#6898)
      Merge 3.11.0-rc1 changes to master (#6917)
      Add a proto_lang_toolchain for javalite (#6882)
      Remove duplicate license header from test protos (#7011)
      Remove the section for uploading protoc packages to the GitHub release page (#6805)
      Install RubyGem bundler version specified in Gemfile.lock to fix release failure (#7144)
      Install RubyGem bundler version specified in Gemfile.lock to fix release failure (#7144) (#7156)
      Update protobuf version (#7143)
      Update CHANGES.txt for 3.11.3 release (#7162)
      Changed the maven URL to use https when building dist artifacts
      Add js files added in #7176 to Makefiles (#7189)
      Merge pull request #6938 from ObsidianMinor/csharp/fix/6936 (#7188)
      Update the Xcode version number in Kokoro (#7202)
      Update the Xcode version number in Kokoro (#7202) (#7203)
      Fix for wrappers with a zero value (#7195) (#7201)
      Update CHANGES.txt for 3.11.4 release
      Fix for JSON serialization of 0/empty-valued wrapper types (#7198) (#7204)
      Update protobuf version (#7206)
      Remove protoc release for 32-bit Macs (#7209)
      Update CHANGES.txt to include the description of #7210 (#7212)
      Skip building distribution tar files for 32 bit Mac (#7214)
      Remove 32-bit Mac binary from csharp build tools (#7215)
      Update Google.Protobuf.Tools.nuspec to remove 32-bit Mac protoc reference (#7227)
      Remove 32-bit Mac protoc reference from collect_all_artifacts (#7228)
      Project import generated by Copybara
      Project import generated by Copybara
      Add bazel to the list of acceptable mergeable tags
      Update mergeable.yml
      Project import generated by Copybara
      Project import generated by Copybara
      Project import generated by Copybara
      Project import generated by Copybara
      Update missing/renamed kernel files (#7310)

Robert Morris (1):
      Request SummaFT extension (#7314)

Scott Hart (2):
      bug: #7076 adds OUT and OPTIONAL to windows portability files (#7087)
      bug: #7076 adds OUT and OPTIONAL to windows portability files (#7088)

Stanley Cheung (2):
      Cherry-pick of df719723: Simplify template exporting macros (#7539)
      Fix :protobuf_objc bazel target (#7538)

Stephan Hartmann (1):
      Fix GCC build with PROTOBUF_USE_DLLS enabled

Sydney Acksman (14):
      Add some missing null-checks
      Fix conformance test failures for Google.Protobuf
      Fix exception message on unsupported request output format
      Fix formatting
      Fix readability.
      Add test for not throwing on missing required
      Use Distinct on depended extensions to filter duplicate extensions
      Add tests
      Improve test to make sure the extensions are actually loaded for CustomOptions
      Adjust based on review feedback
      Add comment and Assert.DoesNotThrow to RequiredFieldsNoThrow
      Use explicit comparer for extension identifiers
      Make test comment a summary
      Rename files and revert changes to generate protos script

Thomas Van Lenten (19):
      Remove use of VLA.
      Make the unittest proto file generation handle additions better.
      Move a few more `size() > 0` calls over to `!empty()`.
      Revisit how the WKTs are bundled with ObjC.
      Update the build/test script for newer Xcode 11 versions
      [ObjC] Fix some tests checking the wrong Message class.
      [ObjC] Remove helper to avoid extra lookups.
      [ObjC] Improve validation on public apis.
      Move to constexpr.
      [ObjC] Move over some custom logic to Descriptor provided info.
      [ObjC] Update oneof generation for proto3 optional.
      [ObjC] Runtime support for proto3 optional.
      [ObjC] Update some library internals to not pass syntax versions.
      [ObjC] Update oneof clearing internals.
      [ObjC] Generation changes around proto3 optional.
      [ObjC] Add tests for proto3 optional behaviors.
      [ObjC] Typos and comment improvements.
      Use the new helpers from Descriptor.
      Tweak the union used for Extensions to support old generated code.

Tim Swast (3):
      python: add sphinx docs (#6525)
      python: publish sphinx docs to googleapis.dev
      python: publish sphinx docs to read the docs

Tomo Suzuki (1):
      Simplifying linkage monitor test setup (#6855)

William A Rowe Jr (2):
      Correct interpretation of utf-8 0xf8-0xff
      Treat F5-FF octets as single (invalid) characters

Yannic (10):
      Add --<lang>_opt flag for all built-in generators
      Blacklist .proto source files if Bazel allows us to (#7065)
      Blacklist .proto source files if Bazel allows us to (#7065)
      [bazel] Fix blacklisted_protos in cc_toolchain and add test (#7096)
      [bazel] Fix blacklisted_protos in cc_toolchain and add test (#7075)
      [bazel] Move Java runtime/toolchains into //java (#7190)
      [bazel] Update gtest and deprecate //external:{gtest,gtest_main} (#7237)
      Add compile-time tests for RTTI to port_def.inc
      Fix typo
      Remove check that C++ version is at least C++98

Yannic Bonenberger (2):
      Use PROTOBUF_RTTI from port_def.inc macro instead of checking for absence of GOOGLE_PROTOBUF_NO_RTTI macro
      [bazel] Remove bootstrap hack from cc_proto_library and add interop with proto_library

Zhao Junwang (1):
      add `-std=c++11` to make it compile

afshinpir (2):
      Incorrect selection of base name
      Correcting import path selection for protoc

dmaclach (3):
      Fix up parameter name so decl matches defn
      Clean up use of size() > 0 to !empty()
      Move min Xcode version to 10.3

rafikamal (3):
      Project import generated by Copybara
      Project import generated by Copybara
      Project import generated by Copybara

salamaniibm (1):
      correcting the s390x Arch name

summerCol (1):
      fix typos in benchmarks/README.md

wsw2016 (1):
      Improves performance of json_stream_parser.cc by factor 1000
bithium pushed a commit to bithium/protobuf that referenced this issue Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants