Skip to content

Add memfd_create to linux and android #2069

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
Nov 6, 2021
Merged

Conversation

jwellhofer
Copy link

Android only added the wrapper in API level 30, i.e. Android 11. Should this have libc::syscall implementation for android ?

@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.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@Amanieu
Copy link
Member

Amanieu commented Feb 12, 2021

I'm personally in favor of just exposing everything and letting the user deal with linker errors if their toolchain is too old. The only limitation is whether our CI accepts it.

@Amanieu
Copy link
Member

Amanieu commented Feb 12, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Feb 12, 2021

📌 Commit 429cef2 has been approved by Amanieu

@bors
Copy link
Contributor

bors commented Feb 12, 2021

⌛ Testing commit 429cef2 with merge 0177682...

bors added a commit that referenced this pull request Feb 12, 2021
Add memfd_create to linux and android

Android only added the wrapper in API level 30, i.e. Android 11. Should this have libc::syscall implementation for android ?
@bors
Copy link
Contributor

bors commented Feb 12, 2021

💔 Test failed - checks-actions

@Amanieu
Copy link
Member

Amanieu commented Feb 12, 2021

Maybe try upgrading the Android NDK in the CI scripts?

@Amanieu
Copy link
Member

Amanieu commented Feb 12, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Feb 12, 2021

📌 Commit da1fc9c has been approved by Amanieu

bors added a commit that referenced this pull request Feb 12, 2021
Add memfd_create to linux and android

Android only added the wrapper in API level 30, i.e. Android 11. Should this have libc::syscall implementation for android ?
@bors
Copy link
Contributor

bors commented Feb 12, 2021

⌛ Testing commit da1fc9c with merge 103b534...

@bors
Copy link
Contributor

bors commented Feb 12, 2021

💔 Test failed - checks-actions

@jwellhofer
Copy link
Author

@bors r+

@bors
Copy link
Contributor

bors commented Feb 12, 2021

@jwellhofer: 🔑 Insufficient privileges: Not in reviewers

@Amanieu
Copy link
Member

Amanieu commented Feb 13, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Feb 13, 2021

📌 Commit 5920c8d has been approved by Amanieu

@bors
Copy link
Contributor

bors commented Feb 13, 2021

⌛ Testing commit 5920c8d with merge 88bd0b1...

bors added a commit that referenced this pull request Feb 13, 2021
Add memfd_create to linux and android

Android only added the wrapper in API level 30, i.e. Android 11. Should this have libc::syscall implementation for android ?
@bors
Copy link
Contributor

bors commented Feb 13, 2021

💔 Test failed - checks-actions

@jwellhofer
Copy link
Author

I tried to figure out the CI and get it to use Android API level 30, but couldn't figure out how to fiddle linker/linker64 out of the new sdk emulator image.
I gave up and just added a syscall implementation for android.

@Amanieu
Copy link
Member

Amanieu commented Feb 16, 2021

I'm not sure how I feel about this. Traditionally libc only contains bindings to existing functions rather than actual implementations.

Maybe just drop the Android part for now? Or is Android support something that you actually need?

@jwellhofer
Copy link
Author

I've found a way to get the newer android binaries, but it'll take me a while to figure out how to update the CI. Once i've got the CI working, i'll make a PR for that. I'll come back here when that is done.

@bors
Copy link
Contributor

bors commented Feb 27, 2021

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

@Ralith
Copy link
Contributor

Ralith commented Nov 4, 2021

Could we proceed with just the Linux version in the mean time? e.g. tokio-rs/tracing#1698 calls for it.

@jwellhofer
Copy link
Author

Updating the android CI was such a pain that i gave up and just used a patched rust-libc.
I've put the linux only into the PR branch. Couldn't run the tests on my machine though because of some sockaddr_vm padding size mismatches. Hopefully it'll work on here.

@Amanieu
Copy link
Member

Amanieu commented Nov 6, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Nov 6, 2021

📌 Commit f1bd231 has been approved by Amanieu

@bors
Copy link
Contributor

bors commented Nov 6, 2021

⌛ Testing commit f1bd231 with merge afd3951...

@bors
Copy link
Contributor

bors commented Nov 6, 2021

☀️ Test successful - checks-actions, checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13
Approved by: Amanieu
Pushing afd3951 to master...

@bors bors merged commit afd3951 into rust-lang:master Nov 6, 2021
@Ralith
Copy link
Contributor

Ralith commented Nov 6, 2021

Thanks!

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.

6 participants