FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

GH-523: [Release] Add support for building .jar including JNI binaries by kou · Pull Request #517 · apache/arrow-java · GitHub

/ arrow-java Public

GH-523: [Release] Add support for building .jar including JNI binaries - #517

Merged
kou merged 7 commits into
apache:mainfrom
kou:ci-rc-jni
Jan 16, 2025
ConversationCommits (7)ChecksFiles changed
Merged

GH-523: [Release] Add support for building .jar including JNI binaries#517
kou merged 7 commits into
apache:mainfrom
kou:ci-rc-jni

Conversation

kou commented Jan 15, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Fixes GH-523.

Merge test_jni.yml to rc.yml so that .jar including JNI binaries can be built and uploaded to GitHub Releases for RC.

ccache support is also enabled for fast CI.

kou force-pushed the ci-rc-jni branch 2 times, most recently from 2b8fe63 to 76d58fc Compare January 15, 2025 01:11
kou commented Jan 15, 2025
View reviewed changes
Comment thread .github/workflows/rc.yml
tar -xf apache-arrow-java-*.tar.gz --strip-components=1
- name: Download the latest Apache Arrow C++
run: |
ci/scripts/download_cpp.sh

kou Jan 15, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We may want to use the main branch instead of related Apache Arrow C++ on daily run. But it's out-of-scope in this PR.

Comment thread .github/workflows/rc.yml
- name: Extract Download the latest Apache Arrow C++
run: |
tar -xf apache-arrow-java-*.tar.gz --strip-components=1
# We need 19.0.0 for latest Boost support

kou Jan 15, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

apache/arrow#45053

Comment thread .github/workflows/rc.yml Outdated
upload:
name: Upload
if: github.ref_type == 'tag'
# if: github.ref_type == 'tag'

kou Jan 15, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'll revert this after I confirmed that this job works.

kou Jan 15, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Reverted. This worked: https://github.com/apache/arrow-java/actions/runs/12781206977/job/35629480932#step:3:17

Comment thread .github/workflows/rc.yml Outdated
version=${version#v}
rc=${GITHUB_REF_NAME#*-rc}
gh release create ${GITHUB_REF_NAME} \
echo gh release create ${GITHUB_REF_NAME} \

kou Jan 15, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'll revert this after I confirmed that this job works.

lidavidm approved these changes Jan 15, 2025
View reviewed changes
Comment thread .github/workflows/rc.yml
Comment on lines +109 to +118
- name: Checkout apache/arrow-testing
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
repository: apache/arrow-testing
path: arrow/testing
- name: Checkout apache/parquet-testing
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
repository: apache/parquet-testing
path: arrow/cpp/submodules/parquet-testing

lidavidm Jan 15, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Do we need testing data? (I don't think we need to run C++ tests here?)

kou Jan 15, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

OK. I'll disable C++ tests.
(I thought that we need to run C++ tests because it seems that we don't have enough test for JNI related codes.)

lidavidm Jan 15, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The C++ tests we run here are just the same tests that apache/arrow already runs right? (I agree we need more testing for the JNI code itself, though...)

kou Jan 15, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Right but we use different build options that aren't covered by apache/arrow.

lidavidm Jan 15, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Hmm if they're that different then should we be testing the config upstream? Or otherwise in that case I don't mind running the C++ tests here if our config is very different (it mostly just adds a lot of build time)

kou Jan 15, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

OK. Let's do it in upstream.

kou commented Jan 15, 2025

Copy link
Copy Markdown
Member Author

CI passed. This is ready.

kou changed the title GH-500: [Release] Add support for releasing .jar including JNI binaries GH-500: [Release] Add support for building .jar including JNI binaries Jan 15, 2025
kou changed the title GH-500: [Release] Add support for building .jar including JNI binaries GH-523: [Release] Add support for building .jar including JNI binaries Jan 15, 2025

kou commented Jan 15, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

I'll merge this today.

I'll work on the followings as separated tasks:

  • Add support for building .jar for Windows
  • Sign uploaded .jar and related files
  • Upload signed .jar and related files to staging
  • Publish files on staging after release vote passed

kou merged commit 6e3c8ba into apache:main Jan 16, 2025
kou deleted the ci-rc-jni branch January 16, 2025 00:14
lidavidm added this to the 18.2.0 milestone Jan 30, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

lidavidm lidavidm approved these changes
laurentgo Awaiting requested review from laurentgo laurentgo is a code owner
wgtmac Awaiting requested review from wgtmac wgtmac is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

18.2.0

Development

Successfully merging this pull request may close these issues.

[Release] Add support for building .jar including JNI binaries

2 participants

Back | FazBrowse Home | New Git URL