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 crt-static for android #2848

Merged
merged 2 commits into from Aug 9, 2022
Merged

Conversation

Bryanskiy
Copy link
Contributor

No description provided.

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

Copy link
Member

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it'd be helpful to provide the context/reason for this change.

Comment on lines -29 to -32
#![cfg_attr(
feature = "rustc-dep-of-std",
feature(native_link_modifiers, native_link_modifiers_bundle)
)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain why this change is necessary?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

native_link_modifiers and native_link_modifiers_bundle are already stable, so these features produce #[warn(stable_features)] in rustc-dep-of-std mode (which turns into an error when building standard library in rust-lang/rust).

@petrochenkov
Copy link
Contributor

Also, it'd be helpful to provide the context/reason for this change.

This is the libc part of supporting statically linked executables on Android, the rustc part is rust-lang/rust#99421.

@Amanieu
Copy link
Member

Amanieu commented Jul 30, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Jul 30, 2022

📌 Commit 64d47e8 has been approved by Amanieu

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jul 30, 2022

⌛ Testing commit 64d47e8 with merge 09afbfb...

bors added a commit that referenced this pull request Jul 30, 2022
@bors
Copy link
Contributor

bors commented Jul 30, 2022

💔 Test failed - checks-actions

@Amanieu
Copy link
Member

Amanieu commented Jul 30, 2022

  = note: /android/ndk-arm/bin/../lib/gcc/arm-linux-androideabi/4.9.x/../../../../arm-linux-androideabi/bin/ld: error: cannot find -lrt
          /android/ndk-arm/bin/../lib/gcc/arm-linux-androideabi/4.9.x/../../../../arm-linux-androideabi/bin/ld: error: cannot find -lpthread

src/unix/mod.rs Outdated
} else if #[cfg(any(target_os = "macos",
target_os = "ios",
target_os = "watchos",
target_os = "android",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With these changes as is the case of Android without rustc-dep-of-std is never covered, so this line needs to be kept.
Sorry, didn't notice this previously.

@petrochenkov
Copy link
Contributor

Does anybody know why libc on musl and redox doesn't link anything at all in regular no-std mode (without rustc-dep-of-std)?
It looks like a bug but people apparently lived with this for years?

@petrochenkov
Copy link
Contributor

Implementation for glibc is also incorrect, but in a different way - in regular no-std mode the libraries are linked dynamically even if crt-static is enabled.

@petrochenkov
Copy link
Contributor

Ah, I see, all of this is indeed incorrect but cannot be properly fixed on stable version of libc because link time cfg is unstable.
(But unstable features can be used in feature = "rustc-dep-of-std" mode.)

Anyway, this PR is about Android and the least bad thing to do for this target would be to follow what glibc does (i.e. #2848 (comment)).

@Amanieu
Copy link
Member

Amanieu commented Aug 9, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Aug 9, 2022

📌 Commit d6d7bfd has been approved by Amanieu

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Aug 9, 2022

⌛ Testing commit d6d7bfd with merge 4b7908f...

@bors
Copy link
Contributor

bors commented Aug 9, 2022

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

@bors bors merged commit 4b7908f into rust-lang:master Aug 9, 2022
@petrochenkov
Copy link
Contributor

@Amanieu
Could you make a release?
It's needed to pull this into rust-lang/rust.

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.

None yet

6 participants