Skip to content
This repository was archived by the owner on Aug 30, 2022. It is now read-only.

Update to latest version of Thrift #195

Closed
david-antiteum opened this issue Dec 23, 2019 · 16 comments
Closed

Update to latest version of Thrift #195

david-antiteum opened this issue Dec 23, 2019 · 16 comments

Comments

@david-antiteum
Copy link

The Jaeger client cpp uses an old version of Thrift that lacks proper support for cmake config files. This has been addressed in the current versions but they are not compatible with jaeger as they have remove the stdcxx.h header file (see case #174 for other affected users).

Any chance to update to the latest Thrift version?

Thanks.

@yurishkuro
Copy link
Member

cc @mdouaihy

@mdouaihy
Copy link
Contributor

mdouaihy commented Jan 3, 2020

hello @david-antiteum,

It's always a good idea to update a dependency version. Do you have a PR to suggest?

Please note that the thrift update should not break the compatibility with the Jaeger backend.

@david-antiteum
Copy link
Author

Hi @mdouaihy

Unfortunately no. I have no experience with Thrift and your code must be changed a bit. My primary interest is to have jaeger in vcpck and this is the missing piece.

Thanks

@mdouaihy
Copy link
Contributor

mdouaihy commented Jan 3, 2020

ok, no problem, I can handle that but it'll require some time because we'll have to update the thrift version packaged in Hunter (0.12 -> 0.13).

@smartnet-club
Copy link

It's good idea do not add to repository generated thrift files, but add to CMakeLists.txt code to generate them.

@smartnet-club
Copy link

In version v0.13.0 of thrift, the name of generated config file ThriftConfig.cmake. (Starts with upper case)
So we have error:

CMake Error at jaeger-client-cpp/v0.5.0/lib/cmake/jaegertracing/jaegertracingConfig.cmake:35 (find_package):
Could not find a package configuration file provided by "thrift" with any
of the following names:

thriftConfig.cmake
thrift-config.cmake

@suab321321
Copy link

suab321321 commented Aug 13, 2020

@mdouaihy should we just create thriftConfig.cmake and use find_file and find_library to find thrift?

@sirzooro
Copy link

sirzooro commented Jun 2, 2021

Any update on this? I am trying to build on CentOS 8 with hunter disabled, and build fails because thrift-0.13.0-2 in CentOS8 does not have thrift/stdcxx.h.

@david-antiteum
Copy link
Author

david-antiteum commented Jun 3, 2021

Nothing yet. There is a pull request trying to solve this problem: #239 from @ideepika but it has not been accepted yet. And, as @smartnet-club already points out, the root of the problem is adding thrift-generated files to the repository rather than having a CMake rule to generate them during the build process.

@robem
Copy link

robem commented Jun 9, 2021

The following list of open CVEs suggests that an upgrade of Thrift is most desirable https://www.opencve.io/cve?vendor=apache&product=thrift

@fmoraes74
Copy link

What's the status of this? We are stuck on with older version of Thrift until jaeger-client-cpp supports a new version of Thrift.

@CC-Libby
Copy link

CC-Libby commented Oct 22, 2021

When I use the 0.13.0 version of thrift, the following error occurs when building my service. But when using 0.12.0 version of thrift, everything is normal.

/usr/bin/ld: CMakeFiles/code.dir/service/server.skeleton.cpp.o: in function main': server.skeleton.cpp:(.text+0xc39): undefined reference to apache::thrift::concurrency::PosixThreadFactory::PosixThreadFactory(apache::thrift::concurrency::PosixThreadFactory::POLICY, apache::thrift::concurrency::PosixThreadFactory::PRIORITY, int, bool)'
collect2: error: ld returned 1 exit status
make[2]: *** [CMakeFiles/code.dir/build.make:578: code] Error 1
make[1]: *** [CMakeFiles/Makefile2:83: CMakeFiles/code.dir/all] Error 2
make: *** [Makefile:91: all] Error 2`

@fmoraes74
Copy link

@yurishkuro Has this been fixed?

@yurishkuro
Copy link
Member

Library is deprecated, please see README

@fmoraes74
Copy link

@yurishkuro Thanks. It would have been nice to state so when you closed the issue.

@yurishkuro
Copy link
Member

Yes, sorry, I linked the corresponding README commit to closed PRs, but it was too much work to do the issues, and GH doesn't allow to add a comment in bulk when closing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants