-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
[arrow] Update to 18.1.0 #42357
[arrow] Update to 18.1.0 #42357
Conversation
@microsoft-github-policy-service agree |
Please fix the build error of |
I've filed apache/arrow#44886 to track the process of figuring out a fix. I'll post an update here once we have a fix. |
Note that vcpkg CI builds Android with API level 21. At that level, C runtime is very incomplete. It might be necessary to record an expected fail in scripts/ci.baseline.txt. |
I pushed up a patch for the Android failure and will keep an eye on CI now. |
ports/arrow/portfile.cmake
Outdated
0004_android.patch | ||
0005_android.patch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Patch names 🥹
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good yeah?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious if you're requesting a change here or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not in the position to request changes, but IMHO it is not good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you able to give any guidance on what would be good?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e.g. 0006-android-datetime.patch
.
(But I still wonder if patching a vendored datetime is the right thing to start with.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re: the filenames, I pushed changes to the filenames.
@LilyWangLL we added a patch to the portfile and that looks like it's fixed the two failing jobs from before. I'm seeing the By the way, we plan to upstream the patch and remove it from this port in a future update, see apache/arrow#43390. |
Co-authored-by: Kai Pastor <dg0yt@darc.de>
Thanks for the update! |
Thanks for the merge @BillyONeal and for the reviews @dg0yt. |
Fixes #42369
Any fixed CI baseline entries are removed from that file.Any patches that are no longer applied are deleted from the port's directory../vcpkg x-add-version --all
and committing the result.