-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
[android] Fix some tests and a doc flag #40977
Conversation
@@ -161,5 +161,5 @@ $ utils/build-script \ | |||
--android \ # Build for Android. | |||
--android-ndk ~/android-ndk-r23b \ # Path to an Android NDK. | |||
--android-arch armv7 \ # Optionally specify Android architecture, alternately aarch64 | |||
--android-ndk-version 21 |
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.
There is no such flag, which was corrected in the build script but missed in this doc.
// In Android l_len is __kernel_off_t which is not the same size as off_t in | ||
// 64 bits. | ||
flck.l_len = __kernel_off_t(data.utf8.count) | ||
#else | ||
flck.l_len = off_t(data.utf8.count) |
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 think the above comment was written before AArch64 support was added, off_t
actually works fine. This recently broke on Android AArch64 after an import was added to this file, with the compiler now complaining that __kernel_off_t
is an Int
and the count is an Int64
, but this change fixes it.
@swift-ci smoke test |
@swift-ci python lint |
7a014ab
to
112a3b5
Compare
Rebased and fixed the Python formatting issue the linter is complaining about, Distributed test failure is unrelated. |
@artemcm, would you run the CI on this? |
@swift-ci please test |
@CodaFi, this is ready to go. |
Use off_t for a recently failing stdlib test, remove Python hacks for some tests that now work without them, and correct build flag in the docs.
112a3b5
to
26aafe6
Compare
(args, shared_output_lock)) | ||
shared_output_lock = multiprocessing.Lock() | ||
pool = multiprocessing.Pool(args.jobs, set_up_child, | ||
(args, shared_output_lock)) |
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.
Someone recently ported sem_open
to Termux, termux/termux-packages#8993, so go the other way and remove the Android-specific code instead, now that this works.
@MaxDesiatov, would you run the CI on this? |
@swift-ci please smoke test |
@buttaface I highly recommend requesting CI access, sending that request shouldn't take much of your time. |
@MaxDesiatov, I know, you recommended that before but I'm winding down submitting my Android patches (this is the last patch I apply locally when building trunk on my phone for testing that is worth submitting), and I think it's better not to have CI access once I go into a low-commit mode from here on out (I'm down to only four trunk pulls pending, with plans to submit only one of my remaining patches), for security and other reasons. |
I don't see much problem in requesting CI access, and then requesting it to be revoked after you're done with your contributions. |
I'm almost out the door, probably not worth the hassle for all involved. |
Use
off_t
for a recently failing stdlib test, hack the Python script for some tests so that it works with newer Python, and correct build flag in the docs.@CodaFi, normally I'd ask @drodriguez but I haven't been able to get a hold of him lately, so would you review?