Skip to content

add a Makefile to vendor/ #2218

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

Merged
merged 1 commit into from
Jul 18, 2025
Merged

Conversation

kevinjqliu
Copy link
Contributor

Rationale for this change

Add a Makefile to vendor/. This helps with running commands to regenerate vendor/

# Generate all vendor packages:
make all

# Generate individual packages:
make fb303           # FB303 Thrift client only
make hive-metastore  # Hive Metastore Thrift definitions only

Pulled this change out of #2217

Are these changes tested?

Yes, ran make all locally

Are there any user-facing changes?

Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

I like this a lot 👍

hive-metastore:
rm -rf /tmp/hive
mkdir -p /tmp/hive/share/fb303/if/
curl -s https://raw.githubusercontent.com/apache/thrift/master/contrib/fb303/if/fb303.thrift > /tmp/hive/share/fb303/if/fb303.thrift
Copy link
Contributor

Choose a reason for hiding this comment

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

While at it, should we pass in the Hive version explicitly, instead of using master?

Copy link
Contributor Author

@kevinjqliu kevinjqliu Jul 18, 2025

Choose a reason for hiding this comment

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

i was thinking about that too.
the fb303.thrift here is in the apache/thrift repo, which has its own versioning. Its currently on v0.22.0. See https://github.com/apache/thrift/tags

hive_metastore.thrift below is in the apache/hive repo which uses hive version. Its currently on 4.0.1. See https://github.com/apache/hive/tags

Its hard to figure out which version we should pick here for each. This might be left to the person upgrading the vendor/

@kevinjqliu kevinjqliu merged commit 406a0d7 into apache:main Jul 18, 2025
10 checks passed
@kevinjqliu kevinjqliu deleted the kevinjqliu/vendor-make branch July 18, 2025 21:00
@kevinjqliu
Copy link
Contributor Author

Merging this for now. We can revisit and see how we can explicitly pass in the hive version

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