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

Add Thrift 0.14.1 compatibility #1403

Merged
merged 1 commit into from
Feb 18, 2022
Merged

Conversation

msosyak
Copy link
Contributor

@msosyak msosyak commented Jan 28, 2022

Make saithrift compatible with thrift 0.14.1

How I did it:

  • Replace union by struct in *.thrift files as saithrift code relays on an old thrift bug which was fixed in thrift 0.11.0. Due to that BUG unios behaves as struct. In SONiC there is patch to revert the fix in 0.11.0 .
  • Use different shared_ptr based on thrift version. In Thrift 0.11.0 the boost::shared_ptr is usually used. But star from 0.13.0 the boost dependency was dropped in Thrift.
  • Starting from 0.14.1 the constants files are not generated for CPP language, so added them conditionally.

Verified these changes in SONIC build env with different thrift version:

msosyak@b3c1a8d88f02:/sonic/src/sonic-sairedis/SAI$ dpkg -l | grep thrift
ii  libthrift-0.11.0                       0.11.0-4                                amd64        Thrift C++ library
ii  libthrift-dev                          0.11.0-4                                amd64        Thrift C++ library (development headers)
ii  python-thrift                          0.11.0-4                                amd64        Python library for Thrift
ii  thrift-compiler                        0.11.0-4                                amd64        code generator/compiler for Thrift definitions
msosyak@b3c1a8d88f02:/sonic/src/sonic-sairedis/SAI$ dpkg-buildpackage -rfakeroot -b -us -uc -tc -j6
dpkg-buildpackage: info: source package saithrift
dpkg-buildpackage: info: source version 0.9.4
dpkg-buildpackage: info: source distribution stable
dpkg-buildpackage: info: source changed by Pavel Shirshov <pavelsh@microsoft.com>
dpkg-buildpackage: info: host architecture amd64
 dpkg-source --before-build .
 fakeroot debian/rules clean
dh clean
dh: warning: Compatibility levels before 10 are deprecated (level 9 in use)
   debian/rules override_dh_auto_clean
make[1]: Entering directory '/sonic/src/sonic-sairedis/SAI'
dh_auto_clean
* * * 
 dpkg-source --after-build .
dpkg-buildpackage: info: binary-only upload (no source included)
msosyak@b3c1a8d88f02:/sonic/src/sonic-sairedis/SAI$ echo $?
0
msosyak@b3c1a8d88f02:/sonic/src/sonic-sairedis/SAI$ dpkg -l | grep thrift
ii  libthrift-0.11.0                       0.11.0-4                                amd64        Thrift C++ library
ii  libthrift-dev                          0.14.1                                  amd64        Thrift C++ library (development headers)
ii  libthrift0                             0.14.1                                  amd64        Thrift C++ library
ii  python-thrift                          0.14.1                                  amd64        Python bindings for Thrift (Python 2)
ii  thrift-compiler                        0.14.1                                  amd64        Compiler for Thrift definition files
(reverse-i-search)`d': ^Ckg -l | grep thrift
msosyak@b3c1a8d88f02:/sonic/src/sonic-sairedis/SAI$ dpkg-buildpackage -rfakeroot -b -us -uc -tc -j100 
dpkg-buildpackage: info: source package saithrift
dpkg-buildpackage: info: source version 0.9.4
dpkg-buildpackage: info: source distribution stable
dpkg-buildpackage: info: source changed by Pavel Shirshov <pavelsh@microsoft.com>
dpkg-buildpackage: info: host architecture amd64
 dpkg-source --before-build .
 fakeroot debian/rules clean
dh clean
* * * * *
 dpkg-source --after-build .
dpkg-buildpackage: info: binary-only upload (no source included)
msosyak@b3c1a8d88f02:/sonic/src/sonic-sairedis/SAI$ echo $?
0

Signed-off-by: Myron Sosyak <myronx.sosyak@intel.com>
@msosyak
Copy link
Contributor Author

msosyak commented Feb 1, 2022

@richardyu-ms @kcudnik Please review

@msosyak
Copy link
Contributor Author

msosyak commented Feb 15, 2022

@richardyu-ms Could we merge this?

@richardyu-ms richardyu-ms merged commit ceaf267 into opencomputeproject:master Feb 18, 2022
msosyak pushed a commit to msosyak/SAI that referenced this pull request Feb 18, 2022
Signed-off-by: Myron Sosyak <myronx.sosyak@intel.com>
richardyu-ms pushed a commit that referenced this pull request Feb 23, 2022
Signed-off-by: Myron Sosyak <myronx.sosyak@intel.com>
@msosyak msosyak mentioned this pull request Mar 29, 2022
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

Successfully merging this pull request may close these issues.

4 participants