Skip to content

Commit 0a0f5b8

Browse files
stepanblyschaklguohan
authored andcommitted
[thrift] add a patch to revert THRIFT-3650 (#2688)
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 <[email protected]>
1 parent 698b248 commit 0a0f5b8

File tree

3 files changed

+48
-0
lines changed

3 files changed

+48
-0
lines changed

.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ src/telemetry/debian/*
4949
!src/telemetry/debian/rules
5050
!src/telemetry/debian/telemetry.init.d
5151
src/thrift/*
52+
!src/thrift/patch/
5253
!src/thrift/Makefile
5354

5455
# Autogenerated Dockerfiles

src/thrift/Makefile

+5
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ $(addprefix $(DEST)/, $(MAIN_TARGET)): $(DEST)/% :
2121

2222
dpkg-source -x thrift_$(THRIFT_VERSION_FULL).dsc
2323
pushd thrift-$(THRIFT_VERSION)
24+
25+
# Revert breaking change in thrift 0.11.0
26+
# saithrift implementation relies on the bug in union serialization
27+
# (https://jira.apache.org/jira/browse/THRIFT-3650)
28+
patch -p1 < ../patch/0001-Revert-THRIFT-3650-incorrect-union-serialization.patch
2429
CXXFLAGS="-DFORCE_BOOST_SMART_PTR" DEB_BUILD_OPTIONS=nocheck dpkg-buildpackage -d -rfakeroot -b -us -uc -j$(SONIC_CONFIG_MAKE_JOBS)
2530
popd
2631

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
From f44f148bb9cff81f320e7094e7956d98fc5ac423 Mon Sep 17 00:00:00 2001
2+
From: Stepan Blyschak <[email protected]>
3+
Date: Wed, 20 Mar 2019 16:32:56 +0200
4+
Subject: [PATCH] Revert "THRIFT-3650 incorrect union serialization"
5+
6+
This reverts commit b72bb94a8212edc83864edc435896fdcda6e796c.
7+
8+
Signed-off-by: Stepan Blyschak <[email protected]>
9+
---
10+
compiler/cpp/src/thrift/parse/t_struct.h | 16 ++++++----------
11+
1 file changed, 6 insertions(+), 10 deletions(-)
12+
13+
diff --git a/compiler/cpp/src/thrift/parse/t_struct.h b/compiler/cpp/src/thrift/parse/t_struct.h
14+
index 4102da7..880e79b 100644
15+
--- a/compiler/cpp/src/thrift/parse/t_struct.h
16+
+++ b/compiler/cpp/src/thrift/parse/t_struct.h
17+
@@ -66,16 +66,12 @@ public:
18+
void validate_union_member(t_field* field) {
19+
if (is_union_ && (!name_.empty())) {
20+
21+
- // 1) unions can't have required fields
22+
- // 2) union members are implicitly optional, otherwise bugs like THRIFT-3650 wait to happen
23+
- if (field->get_req() != t_field::T_OPTIONAL) {
24+
- // no warning on default requiredness, but do warn on anything else that is explicitly asked for
25+
- if(field->get_req() != t_field::T_OPT_IN_REQ_OUT) {
26+
- pwarning(1,
27+
- "Union %s field %s: union members must be optional, ignoring specified requiredness.\n",
28+
- name_.c_str(),
29+
- field->get_name().c_str());
30+
- }
31+
+ // unions can't have required fields
32+
+ if (field->get_req() == t_field::T_REQUIRED) {
33+
+ pwarning(1,
34+
+ "Required field %s of union %s set to optional.\n",
35+
+ field->get_name().c_str(),
36+
+ name_.c_str());
37+
field->set_req(t_field::T_OPTIONAL);
38+
}
39+
40+
--
41+
1.9.1
42+

0 commit comments

Comments
 (0)