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

libthrift version pegging. #133

Closed
itissid opened this issue Feb 6, 2013 · 13 comments
Closed

libthrift version pegging. #133

itissid opened this issue Feb 6, 2013 · 13 comments

Comments

@itissid
Copy link

itissid commented Feb 6, 2013

Hi as I understand since you guys are extending over some of the thrift API's generated by scrooge you are probably pegged to some version of thrift?

@itissid
Copy link
Author

itissid commented Feb 6, 2013

I see version 0.5.0 for libthrift in the pom. That seems really old. Is it safe to upgrade?

@astubbs
Copy link

astubbs commented Jun 27, 2013

#158 wow, messy.

So why is finagle-thrift using such an old version of thrift?

@sritchie
Copy link

We're stuck on 0.5.0 internally at Twitter, and probably won't be moving any time soon.

@jduan
Copy link
Contributor

jduan commented Feb 21, 2017

Are there any updates? The last update was almost a year ago. Thanks!

@ryanoneill
Copy link
Contributor

Hi @jduan, not really much of an update here. We are still on the 0.5.x branch internally. It would be a very large undertaking to migrate everyone internally. Perhaps some day, but we're not actively working on it at the moment.

@jduan
Copy link
Contributor

jduan commented Feb 21, 2017

@ryanking thank you for the update anyway!

@ivankelly
Copy link

There's a licensing problem with using com.twitter.libthrift-0.5.0-7.

This is a derived work of apache libthrift, thus falls under the ASLv2. Clause 4(d) states that when you distribute the derived work, you must also distribute the NOTICE from the original work. Twitter libthrift does not do this, which put it in violation of the license.

What's more, anyone who pulls in finagle and redistributes in binary form with dependencies will also be in violation (which is my problem). Could the twitter libthrift source be made available, or at least the NOTICE added to the jar.

@mosesn
Copy link
Contributor

mosesn commented Dec 19, 2017

@ivankelly we're planning on migrating to 0.10.0 shortly, so we won't have to be on our fork anymore. I expect it to be within less than a month.

In the mean time, I believe that you can edit the libthrift JAR on your end to add the NOTICE file. Alternatively, you can instead exclude our fork of libthrift, and depend explicitly on vanilla libthrift-0.5.0.

Sorry for the oversight, and thanks for pointing it out to us.

@ivankelly
Copy link

I believe that you can edit the libthrift JAR on your end to add the NOTICE file.

I don't have the notice file though. The modified libthrift contains changes which are presumably copyrighted to twitter, so it's twitter that would have to provide the NOTICE.

Alternatively, you can instead exclude our fork of libthrift, and depend explicitly on vanilla libthrift-0.5.0.

Isn't there some concurrency bug though, which was the whole reason for the fork?

Sorry for the oversight, and thanks for pointing it out to us.

No worries. This stuff is very easy to overlook.

@mosesn
Copy link
Contributor

mosesn commented Dec 19, 2017

@ivankelly isn't the NOTICE that it needs https://github.com/apache/thrift/blob/master/NOTICE ?

When you say

you distribute the derived work, you must also distribute the NOTICE from the original work

I interpret that as, when you distribute the twitter JAR, you need the NOTICE from apache thrift.

If that doesn't work for you, how would you feel about waiting for ~ a month for us to sort out the 0.10.0 stuff? It's almost ready to go but we want to wait to do the cutover until after the holidays.

@ivankelly
Copy link

@mosesn if one were to put the thrift NOTICE directly into the jar, it would be assigning the copyright of the modifications twitter made to the ASF, which while I'm sure it's fine, is a little weird. Anyhow, this isn't a big deal. Marking libthrift as ASLv2 in my project for now, and as it's a ASF project, it'll be covered by the projects own NOTICE.

Anyhow, as I said, no big deal, especially with the upgrade to 0.10. Thanks for the response.

@mosesn
Copy link
Contributor

mosesn commented Dec 20, 2017

Sounds good, sorry I don't have a better answer for you :/. We'll make sure to get the 0.10.0 stuff going in a timely manner.

@stefanlance
Copy link

Closing this since we upgraded to libthrift-0.10.0 as of 61c7a71.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

8 participants