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

android has posix_memalign for API 16+ since NDK r10d #30601

Merged
merged 1 commit into from Jan 13, 2016

Conversation

tamird
Copy link
Contributor

@tamird tamird commented Dec 29, 2015

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@brson
Copy link
Contributor

brson commented Dec 29, 2015

@bors r+

@bors
Copy link
Contributor

bors commented Dec 29, 2015

📌 Commit 2a88cd0 has been approved by brson

@tamird tamird changed the title Use libc functions when possible android has posix_memalign for API 16+ since NDK r10d Dec 30, 2015
@tamird
Copy link
Contributor Author

tamird commented Dec 30, 2015

@brson: I rewrote this commit to remove some additional cruft while I'm here.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jan 6, 2016

📌 Commit 99cfe48 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Jan 6, 2016

⌛ Testing commit 99cfe48 with merge 037e822...

@bors
Copy link
Contributor

bors commented Jan 6, 2016

💔 Test failed - auto-linux-64-x-android-t

@tamird
Copy link
Contributor Author

tamird commented Jan 6, 2016

Ugh, this depends on rust-lang/libc#132

@alexcrichton
Copy link
Member

Thanks for the PR! Although Android may have this API now, our nightlies are compiled against a pretty old version of the NDK to have a "maximally compatible" libstd. I think it's the case that posix_memalign isn't in that NDK, and this will likely fail CI with a link error on android (e.g. the symbol isn't defined).

Increasing the minimum supported Android version is likely out of the scope of this PR (as it affects other projects like Servo), but I'm willing to wait for the libc update to see if this passes CI. If it does, then it seems fine to me!

@brson
Copy link
Contributor

brson commented Jan 11, 2016

Yes, we need to be careful about upgrading our minimum android requirements.

@tamird
Copy link
Contributor Author

tamird commented Jan 11, 2016

Cool beans. Let's see what the bots say!

@alexcrichton
Copy link
Member

The libc changes have been merged now, so you should be able to include the submodule update here

@tamird
Copy link
Contributor Author

tamird commented Jan 12, 2016

Done.

On Mon, Jan 11, 2016 at 7:42 PM, Alex Crichton notifications@github.com
wrote:

The libc changes have been merged now, so you should be able to include
the submodule update here


Reply to this email directly or view it on GitHub
#30601 (comment).

@alexcrichton
Copy link
Member

Maybe an amendment was forgotten? Looks like the submodule still hasn't been updated

@tamird
Copy link
Contributor Author

tamird commented Jan 12, 2016

Ah, sorry, I misunderstood you. Done now.

On Mon, Jan 11, 2016 at 8:41 PM, Alex Crichton notifications@github.com
wrote:

Maybe an amendment was forgotten? Looks like the submodule still hasn't
been updated


Reply to this email directly or view it on GitHub
#30601 (comment).

@alexcrichton
Copy link
Member

@bors: r+ f834051274545c212b4429b9a45117e8c1ba0bea

@bors
Copy link
Contributor

bors commented Jan 12, 2016

⌛ Testing commit f834051 with merge 21c4c0e...

@bors
Copy link
Contributor

bors commented Jan 12, 2016

💔 Test failed - auto-linux-cross-opt

@tamird
Copy link
Contributor Author

tamird commented Jan 12, 2016

@alexcrichton looks like mips failed because of the following type mismatch in src/liblibc/src/unix/notbsd/linux/mips.rs:

pub struct sigaction {
    pub sa_flags: ::c_int,
    ...
}
...

pub const SA_ONSTACK: ::c_uint = 0x08000000;
pub const SA_SIGINFO: ::c_uint = 0x00000008;

@alexcrichton
Copy link
Member

@tamird I've sent rust-lang/libc#135 and once that's merged I think that'll be fixed.

@alexcrichton
Copy link
Member

Ok, libc update merged, should be good to re-update submodule and re-try

@tamird
Copy link
Contributor Author

tamird commented Jan 12, 2016

Done.

On Tue, Jan 12, 2016 at 3:04 PM, Alex Crichton notifications@github.com
wrote:

Ok, libc update merged, should be good to re-update submodule and re-try


Reply to this email directly or view it on GitHub
#30601 (comment).

@alexcrichton
Copy link
Member

@bors: r+ 5657eef

@bors
Copy link
Contributor

bors commented Jan 12, 2016

⌛ Testing commit 5657eef with merge 49c3827...

bors added a commit that referenced this pull request Jan 12, 2016
@bors bors merged commit 5657eef into rust-lang:master Jan 13, 2016
@tamird tamird deleted the delegate-libc branch January 30, 2017 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants