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

Add misc constants and functions for android #2758

Merged
merged 1 commit into from
Dec 21, 2022
Merged

Conversation

fkm3
Copy link
Contributor

@fkm3 fkm3 commented Apr 13, 2022

These are the main diffs present in the downstream android opensource
project repo.

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Amanieu (or someone else) soon.

Please see the contribution instructions for more information.

@Amanieu
Copy link
Member

Amanieu commented Apr 18, 2022

@bors r+

1 similar comment
@Amanieu
Copy link
Member

Amanieu commented Apr 18, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Apr 18, 2022

📌 Commit 50bdc64 has been approved by Amanieu

@bors
Copy link
Contributor

bors commented Apr 18, 2022

⌛ Testing commit 50bdc64 with merge 44557c4...

bors added a commit that referenced this pull request Apr 18, 2022
Add misc constants and functions for android

These are the main diffs present in the downstream android opensource
project repo.
@bors
Copy link
Contributor

bors commented Apr 18, 2022

💔 Test failed - checks-actions

@Amanieu
Copy link
Member

Amanieu commented Apr 18, 2022

CI failure seems to be because we are using an old version of the Android emulator. You should try updating the CI scripts to use a newer version.

@fkm3
Copy link
Contributor Author

fkm3 commented Apr 18, 2022

Thanks. It looks like it is using android SDK version 24 for arm and version 28 for x86, but getrandom was only added in version 28. I'll try bumping the version. eb0794a seems to have chosen different versions on purpose though, so I'm a little wary.

@Amanieu
Copy link
Member

Amanieu commented Apr 18, 2022

This seems to have been due to an unavailability of emulator images for certain SDK versions: #1344 (comment)

@Amanieu
Copy link
Member

Amanieu commented Apr 18, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Apr 18, 2022

📌 Commit 483df98 has been approved by Amanieu

bors added a commit that referenced this pull request Apr 18, 2022
Add misc constants and functions for android

These are the main diffs present in the downstream android opensource
project repo.
@bors
Copy link
Contributor

bors commented Apr 18, 2022

⌛ Testing commit 483df98 with merge f131632...

@bors
Copy link
Contributor

bors commented Apr 18, 2022

💔 Test failed - checks-actions

@fkm3
Copy link
Contributor Author

fkm3 commented Apr 19, 2022

There are no system images for armeabi-v7a after API version 24. It may be that support was dropped. I'm unsure.

I'm going to try moving the getrandom declaration into b64/mod.rs and then only bump the API version for aarch64.

@Amanieu
Copy link
Member

Amanieu commented Apr 19, 2022

Maybe try using an arm64 image? It should support both 32-bit and 64-bit ARM.

@fkm3
Copy link
Contributor Author

fkm3 commented Apr 19, 2022

Good point. I've made that change. I can't get the tests to run locally on my machine, but I did verify the docker image could be built at least.

@Amanieu
Copy link
Member

Amanieu commented Apr 20, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Apr 22, 2022

💥 Test timed out

@fkm3
Copy link
Contributor Author

fkm3 commented Apr 22, 2022

Switched from dd to fallocate (12.892 secs vs 0.438 on my machine). Hopefully that will fix the time out.

@Amanieu
Copy link
Member

Amanieu commented Apr 23, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Apr 23, 2022

📌 Commit 8a64a22 has been approved by Amanieu

bors added a commit that referenced this pull request Apr 23, 2022
Add misc constants and functions for android

These are the main diffs present in the downstream android opensource
project repo.
@bors
Copy link
Contributor

bors commented Apr 23, 2022

⌛ Testing commit 8a64a22 with merge 05af8b7...

@bors
Copy link
Contributor

bors commented Apr 23, 2022

💥 Test timed out

@Amanieu
Copy link
Member

Amanieu commented Apr 23, 2022

@bors retry

@bors
Copy link
Contributor

bors commented Apr 23, 2022

⌛ Testing commit 8a64a22 with merge 2029969...

bors added a commit that referenced this pull request Apr 23, 2022
Add misc constants and functions for android

These are the main diffs present in the downstream android opensource
project repo.
@bors
Copy link
Contributor

bors commented Apr 23, 2022

💥 Test timed out

@Amanieu
Copy link
Member

Amanieu commented Apr 23, 2022

I think there might be something wrong with the arm image being used?

@fkm3
Copy link
Contributor Author

fkm3 commented Apr 29, 2022

Are there logs for the timeout?

@Amanieu
Copy link
Member

Amanieu commented Apr 29, 2022

@fkm3
Copy link
Contributor Author

fkm3 commented Apr 30, 2022

Thanks. I can't find a solution. It seems like an over constrained problem.

Some notes, possibly incorrect:

  • getrandom requires API version >= 28
  • Android emulator support for arm64 on a x64 host was dropped somewhere in the API range (24, 28].
  • There is some support for running arm on the x64 emulator (link), but I'm not sure if it is relevant to the CI tests. Additionally, running the x64 emulator on x64 requires KVM (which I assume the CI doesn't support, if it runs on github actions)

I wonder if we can avoid the android emulator altogether with some combination of qemu + the arm64 system image (similar to what the android x64 tests are doing). I might give that a try, but will probably be slow to follow up.

@Amanieu
Copy link
Member

Amanieu commented May 5, 2022

An alternative solution might be to simply exclude the new symbols from tests (see libc-test/build.rs) and keep using the old Android emulator image.

@bors
Copy link
Contributor

bors commented May 31, 2022

☔ The latest upstream changes (presumably #2808) made this pull request unmergeable. Please resolve the merge conflicts.

These are the main diffs present in the android opensource project.
@fkm3 fkm3 reopened this Dec 20, 2022
@fkm3
Copy link
Contributor Author

fkm3 commented Dec 20, 2022

Sorry for the slow follow up.

An alternative solution might be to simply exclude the new symbols from tests (see libc-test/build.rs) and keep using the old Android emulator image.

I've uploaded new changes that try this approach.

I've also added another io_uring constant.

Thanks!

@JohnTitor
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Dec 21, 2022

📌 Commit 572e11b has been approved by JohnTitor

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Dec 21, 2022

⌛ Testing commit 572e11b with merge 45b431a...

@bors
Copy link
Contributor

bors commented Dec 21, 2022

☀️ Test successful - checks-actions, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13, checks-cirrus-freebsd-14
Approved by: JohnTitor
Pushing 45b431a to master...

@bors bors merged commit 45b431a into rust-lang:master Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants