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

regenerate interface code on thrift 0.17.0 #577

Open
wants to merge 3 commits into
base: palantir-cassandra-2.2.18
Choose a base branch
from

Conversation

tpetracca
Copy link
Contributor

@tpetracca tpetracca commented Nov 8, 2024

In #432 we manually modified the generated code in order to bump the runtime dependency on libthrift to 0.17.0, so now we can't make changes to thrift and re-generate without having to re-apply the manual changes.

This change updates the code-gen to be generated from thrift 0.17.0, which shouldn't require the manual changes anymore.

In order to install thrift 0.17.0 locally I used an old git commit of the Homebrew source and installed from like this...

homebrew-core tpetracca$ git remote -v
origin	https://github.com/Homebrew/homebrew-core.git (fetch)
origin	https://github.com/Homebrew/homebrew-core.git (push)
### this commit hash is the last change to the thrift formula before they upgrade from 0.17.0 to 0.18.0
homebrew-core tpetracca$ git checkout 8f0b9dde69b560dbffad719e3a341fa3018cca7d
HEAD is now at 8f0b9dde69b thrift: brew style --fix
homebrew-core tpetracca$ brew install ./Formula/thrift.rb
... it installing ...

@@ -794,7 +794,7 @@
description="Generate Thrift Java artifacts">
<echo>Generating Thrift Java code from ${basedir}/interface/cassandra.thrift...</echo>
<exec executable="thrift" dir="${basedir}/interface" failonerror="true">
<arg line="--gen java:hashcode" />
<arg line="--gen java" />
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This argument was removed in 0.9.2 for THRIFT-2263 via apache/thrift@acdac81

Before 0.9.2 thrift's java impl did not generate equals() and hashcode() methods for classes by default unless the hashcode flag was set. As of 0.9.2 those methods are generated by default.

@@ -25,7 +25,6 @@

namespace java org.apache.cassandra.thrift
namespace cpp org.apache.cassandra
namespace csharp Apache.Cassandra
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The csharp generator was deprecated in thrift 0.13.0 and removed in 0.14.0. We don't have any C# cassandra thrift clients, so it doesn't matter.

@rhuffy
Copy link
Contributor

rhuffy commented Nov 22, 2024

I attempted the codegen locally and it included a license header on each file. Are you intentionally removing those headers?

@rhuffy
Copy link
Contributor

rhuffy commented Nov 22, 2024

I've also created a homebrew tap that should make it easier install thrift in thrift in the future

brew install rhuffy/thrift/[email protected]

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.

2 participants