From e8c7d3f0a7909511683d824be1d04b6804e03730 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Wed, 20 Mar 2019 17:24:01 +0000 Subject: [PATCH] [thrift] add a patch to revert THRIFT-3650 Revert breaking change in thrift 0.11.0; saithrift implementation relies on the bug in union serialization (details: https://jira.apache.org/jira/browse/THRIFT-3650) Add this revert patch untill saithrift is fixed according to correct thrift behaviour Signed-off-by: Stepan Blyschak --- .gitignore | 1 + src/thrift/Makefile | 5 +++ ...T-3650-incorrect-union-serialization.patch | 42 +++++++++++++++++++ 3 files changed, 48 insertions(+) create mode 100644 src/thrift/patch/0001-Revert-THRIFT-3650-incorrect-union-serialization.patch diff --git a/.gitignore b/.gitignore index 37ee9e6216f6..8cb8aa5fa921 100644 --- a/.gitignore +++ b/.gitignore @@ -49,6 +49,7 @@ src/telemetry/debian/* !src/telemetry/debian/rules !src/telemetry/debian/telemetry.init.d src/thrift/* +!src/thrift/patch/ !src/thrift/Makefile # Autogenerated Dockerfiles diff --git a/src/thrift/Makefile b/src/thrift/Makefile index 41c78655c259..89c9752fbe8b 100644 --- a/src/thrift/Makefile +++ b/src/thrift/Makefile @@ -21,6 +21,11 @@ $(addprefix $(DEST)/, $(MAIN_TARGET)): $(DEST)/% : dpkg-source -x thrift_$(THRIFT_VERSION_FULL).dsc pushd thrift-$(THRIFT_VERSION) + + # Revert breaking change in thrift 0.11.0 + # saithrift implementation relies on the bug in union serialization + # (https://jira.apache.org/jira/browse/THRIFT-3650) + patch -p1 < ../patch/0001-Revert-THRIFT-3650-incorrect-union-serialization.patch CXXFLAGS="-DFORCE_BOOST_SMART_PTR" DEB_BUILD_OPTIONS=nocheck dpkg-buildpackage -d -rfakeroot -b -us -uc -j$(SONIC_CONFIG_MAKE_JOBS) popd diff --git a/src/thrift/patch/0001-Revert-THRIFT-3650-incorrect-union-serialization.patch b/src/thrift/patch/0001-Revert-THRIFT-3650-incorrect-union-serialization.patch new file mode 100644 index 000000000000..7a0742b3dd08 --- /dev/null +++ b/src/thrift/patch/0001-Revert-THRIFT-3650-incorrect-union-serialization.patch @@ -0,0 +1,42 @@ +From f44f148bb9cff81f320e7094e7956d98fc5ac423 Mon Sep 17 00:00:00 2001 +From: Stepan Blyschak +Date: Wed, 20 Mar 2019 16:32:56 +0200 +Subject: [PATCH] Revert "THRIFT-3650 incorrect union serialization" + +This reverts commit b72bb94a8212edc83864edc435896fdcda6e796c. + +Signed-off-by: Stepan Blyschak +--- + compiler/cpp/src/thrift/parse/t_struct.h | 16 ++++++---------- + 1 file changed, 6 insertions(+), 10 deletions(-) + +diff --git a/compiler/cpp/src/thrift/parse/t_struct.h b/compiler/cpp/src/thrift/parse/t_struct.h +index 4102da7..880e79b 100644 +--- a/compiler/cpp/src/thrift/parse/t_struct.h ++++ b/compiler/cpp/src/thrift/parse/t_struct.h +@@ -66,16 +66,12 @@ public: + void validate_union_member(t_field* field) { + if (is_union_ && (!name_.empty())) { + +- // 1) unions can't have required fields +- // 2) union members are implicitly optional, otherwise bugs like THRIFT-3650 wait to happen +- if (field->get_req() != t_field::T_OPTIONAL) { +- // no warning on default requiredness, but do warn on anything else that is explicitly asked for +- if(field->get_req() != t_field::T_OPT_IN_REQ_OUT) { +- pwarning(1, +- "Union %s field %s: union members must be optional, ignoring specified requiredness.\n", +- name_.c_str(), +- field->get_name().c_str()); +- } ++ // unions can't have required fields ++ if (field->get_req() == t_field::T_REQUIRED) { ++ pwarning(1, ++ "Required field %s of union %s set to optional.\n", ++ field->get_name().c_str(), ++ name_.c_str()); + field->set_req(t_field::T_OPTIONAL); + } + +-- +1.9.1 +