-
Notifications
You must be signed in to change notification settings - Fork 111
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
cargo-apk: Work around missing libgcc on NDK r23 with linker script #189
+64
−5
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Rust still searches for libgcc even though [85806] replaces internal use with libunwind, especially now that the Android NDK (since r23-beta3) doesn't ship with any of gcc anymore. The apparent solution is to build your application with nightly and compile std locally (`-Zbuild-std`), but that is not desired for the majority of users. [7339] suggests to provide a local `libgcc.a` as linker script, which simply redirects linking to `libunwind` instead - and that has proven to work fine so far. Intead of shipping this file with the crate or writing it to an existing link-search directory on the system, we write it to a new directory that can be easily passed or removed to `rustc`, say in the event that a user switches to an older NDK and builds without cleaning. For this we need to switch from `cargo build` to `cargo rustc`, but the existing arguments and desired workflow remain identical. [85806]: rust-lang/rust#85806 [7339]: termux/termux-packages#7339 (comment)
dvc94ch
approved these changes
Nov 15, 2021
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.
Neat
msiglreith
approved these changes
Nov 15, 2021
MarijnS95
added a commit
that referenced
this pull request
Nov 23, 2021
As discussed in [197] setting `target_sdk_version` to the "arbitrary" highest available SDK version is nonsense. This target version (unlike `min_sdk_version` which defines the least set of symbols that should be available) has real impact on the runtime of an application, in particular the compatibility or stringency of rules Android applies to your application. Certain APIs may not work at all or be heavily restricted on newer target versions because they are deemed too dangerous, and Android expects the user has tested their app against these limitations and is communicating this by setting `target_sdk_version` to that particular value. Hence this shouldn't change purely based on the environment, even for the default. To retain some backwards compatibility with previous `cargo-apk` we set this to level 30 which is the least [required by Google Play] today, and exactly what users will have been targeting using NDK r22 (assuming the SDK for this `platform` was installed as well) since SDK version 31 support with NDK r23 only [arrived just last week]. [197]: #197 (comment) [required by Google Play]: https://developer.android.com/distribute/best-practices/develop/target-sdk [arrived just last week]: #189
MarijnS95
added a commit
that referenced
this pull request
Nov 24, 2021
As discussed in [197] setting `target_sdk_version` to the "arbitrary" highest available SDK version is nonsense. This target version (unlike `min_sdk_version` which defines the least set of symbols that should be available) has real impact on the runtime of an application, in particular the compatibility or stringency of rules Android applies to your application. Certain APIs may not work at all or be heavily restricted on newer target versions because they are deemed too dangerous, and Android expects the user has tested their app against these limitations and is communicating this by setting `target_sdk_version` to that particular value. Hence this shouldn't change purely based on the environment, even for the default. To retain some backwards compatibility with previous `cargo-apk` we set this to level 30 which is the least [required by Google Play] today, and exactly what users will have been targeting using NDK r22 (assuming the SDK for this `platform` was installed as well) since SDK version 31 support with NDK r23 only [arrived just last week]. [197]: #197 (comment) [required by Google Play]: https://developer.android.com/distribute/best-practices/develop/target-sdk [arrived just last week]: #189
MarijnS95
added a commit
that referenced
this pull request
Nov 24, 2021
As discussed in [197] setting `target_sdk_version` to the "arbitrary" highest available SDK version is nonsense. This target version (unlike `min_sdk_version` which defines the least set of symbols that should be available) has real impact on the runtime of an application, in particular the compatibility or stringency of rules Android applies to your application. Certain APIs may not work at all or be heavily restricted on newer target versions because they are deemed too dangerous, and Android expects the user has tested their app against these limitations and is communicating this by setting `target_sdk_version` to that particular value. Hence this shouldn't change purely based on the environment, even for the default. To retain some backwards compatibility with previous `cargo-apk` we set this to level 30 which is the least [required by Google Play] today, and exactly what users will have been targeting using NDK r22 (assuming the SDK for this `platform` was installed as well) since SDK version 31 support with NDK r23 only [arrived just last week]. [197]: #197 (comment) [required by Google Play]: https://developer.android.com/distribute/best-practices/develop/target-sdk [arrived just last week]: #189
MarijnS95
added a commit
that referenced
this pull request
Nov 24, 2021
As discussed in [197] setting `target_sdk_version` to the "arbitrary" highest available SDK version is nonsense. This target version (unlike `min_sdk_version` which defines the least set of symbols that should be available) has real impact on the runtime of an application, in particular the compatibility or stringency of rules Android applies to your application. Certain APIs may not work at all or be heavily restricted on newer target versions because they are deemed too dangerous, and Android expects the user has tested their app against these limitations and is communicating this by setting `target_sdk_version` to that particular value. Hence this shouldn't change purely based on the environment, even for the default. To retain some backwards compatibility with previous `cargo-apk` we set this to level 30 which is the least [required by Google Play] today, and exactly what users will have been targeting using NDK r22 (assuming the SDK for this `platform` was installed as well) since SDK version 31 support with NDK r23 only [arrived just last week]. [197]: #197 (comment) [required by Google Play]: https://developer.android.com/distribute/best-practices/develop/target-sdk [arrived just last week]: #189
3 tasks
@MarijnS95 Hi, many peoples on the Rust issue tracker are still hitting this issue and may not want to do a custom build of std. Would you be open to try to integrate this workaround directly in the official rust android target ? |
MarijnS95
added a commit
that referenced
this pull request
May 11, 2022
Any flags passed directly to `cargo rustc` will only be passed _to the final compiler invocation_. However `cdylib` library targets in transitive (dependency) crates will still be built (despite not being used in the final binary, that's what `rlib`s are for), but our NDK r23 workaround flags will not reach these compiler invocations rendering b912a6b ("cargo-apk: Work around missing libgcc on NDK r23 with linker script (#189)") ineffective. [The same page] that documents this discrepancy suggests to resort to `RUSTFLAGS` if arguments are intended for all compiler invocations, which is our intended use-case and unblocks builds with transitive `cdylib` dependencies such as `hyper` in [#265]. [The same page]: https://doc.rust-lang.org/cargo/commands/cargo-rustc.html#description [#265]: #265
MarijnS95
added a commit
that referenced
this pull request
May 12, 2022
Any flags passed directly to `cargo rustc` will only be passed _to the final compiler invocation_. However `cdylib` library targets in transitive (dependency) crates will still be built (despite not being used in the final binary, that's what `rlib`s are for), but our NDK r23 workaround flags will not reach these compiler invocations rendering b912a6b ("cargo-apk: Work around missing libgcc on NDK r23 with linker script (#189)") ineffective. [The same page] that documents this discrepancy suggests to resort to `RUSTFLAGS` if arguments are intended for all compiler invocations, which is our intended use-case and unblocks builds with transitive `cdylib` dependencies such as `hyper` in [#265]. [The same page]: https://doc.rust-lang.org/cargo/commands/cargo-rustc.html#description [#265]: #265
MarijnS95
added a commit
that referenced
this pull request
May 12, 2022
…270) Any flags passed directly to `cargo rustc` will only be passed _to the final compiler invocation_. However `cdylib` library targets in transitive (dependency) crates will still be built (despite not being used in the final binary, that's what `rlib`s are for), but our NDK r23 workaround flags will not reach these compiler invocations rendering b912a6b ("cargo-apk: Work around missing libgcc on NDK r23 with linker script (#189)") ineffective. [The same page] that documents this discrepancy suggests to resort to `RUSTFLAGS` if arguments are intended for all compiler invocations, which is our intended use-case and unblocks builds with transitive `cdylib` dependencies such as `hyper` in [#265]. [The same page]: https://doc.rust-lang.org/cargo/commands/cargo-rustc.html#description [#265]: #265
29 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #149
Rust still searches for libgcc even though 85806 replaces internal use with libunwind, especially now that the Android NDK (since r23-beta3) doesn't ship with any of gcc anymore. The apparent solution is to build your application with nightly and compile std locally (
-Zbuild-std
), but that is not desired for the majority of users. 7339 suggests to provide a locallibgcc.a
as linker script, which simply redirects linking tolibunwind
instead - and that has proven to work fine so far.Intead of shipping this file with the crate or writing it to an existing link-search directory on the system, we write it to a new directory that can be easily passed or removed to
rustc
, say in the event that a user switches to an older NDK and builds without cleaning. For this we need to switch fromcargo build
tocargo rustc
, but the existing arguments and desired workflow remain identical.