Skip to content

Replace '-lgcc' option in ndk 23. Fixes #75. #83

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

Closed
wants to merge 1 commit into from

Conversation

minyung
Copy link
Contributor

@minyung minyung commented Feb 23, 2022

Hi. I am a user of this plugin and I am always grateful using it. 👍👍

When I ran the new rust library file with NDK 23, the same issue as #75 occurred.
There were no issues with rust projects that were already built with an older ndk version.
(Just change the name of the rust library you want to build, and you'll see the issue.)

In ndk 23, I confirmed that the gcc library doesn't exist.

After changing the code, the build proceeded normally.
I would really appreciate it if you could check this issue please.

Once again, Thank you :)

reference

@minyung minyung changed the title Remove '-lgcc' option in ndk 23. Fixes #75. Replace '-lgcc' option in ndk 23. Fixes #75. Feb 23, 2022
Copy link
Member

@ncalexan ncalexan left a comment

Choose a reason for hiding this comment

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

@minyung thanks for the patch!

I would prefer to not use the linker wrapper to do this and to handle the arguments in the plugin code, but I see that these arguments come from deep within Rust and its standard library. This is a sad situation. I will note that you can use Nightly Rust within rust-android-gradle, which shouldn't have this problem as of June 2021 or so.

I'm going to try to write a test for building against NDK 23 as part of landing this; we'll see how that goes.

@ncalexan
Copy link
Member

I did a little work on the test infrastructure and got a test looking healthy that verifies we can target NDK 21, 22, and 23. (The versions supported in Github Actions right now.)

I merged a slightly modified version of your patch. If CI looks green tomorrow (it's bed time here), I'll release a new version and then close this PR. Thanks again!

@ncalexan
Copy link
Member

I did a little work on the test infrastructure and got a test looking healthy that verifies we can target NDK 21, 22, and 23. (The versions supported in Github Actions right now.)

I merged a slightly modified version of your patch. If CI looks green tomorrow (it's bed time here), I'll release a new version and then close this PR. Thanks again!

The tests are green and 0.9.2 is live with this change. Let me know if it doesn't work for you, @minyung!

@ncalexan ncalexan closed this Feb 25, 2022
@minyung
Copy link
Contributor Author

minyung commented Feb 25, 2022

@ncalexan
I also didn't want to modify the linker wrapper code, but I couldn't see any other way, so I inevitably modified the linker wrapper code. 😢
It was confirmed that it works normally in version 0.9.2. Thank you so much for the quick patch. ❤️❤️❤️

Gstalker added a commit to Gstalker/Zygisk-Rust-ModuleTemplate that referenced this pull request Mar 17, 2022
ncalexan added a commit to ncalexan/rust-android-gradle that referenced this pull request May 6, 2022
…lla#89.

This applies the fix of mozilla#83 to linker argument @files.  Linker
argument files are used on (at least) Windows.
ncalexan added a commit to ncalexan/rust-android-gradle that referenced this pull request May 6, 2022
…lla#89.

This applies the fix of mozilla#83 to linker argument @files.  Linker
argument files are used on (at least) Windows.
ncalexan added a commit to ncalexan/rust-android-gradle that referenced this pull request May 6, 2022
…lla#89.

This applies the fix of mozilla#83 to linker argument @files.  Linker
argument files are used on (at least) Windows.
ncalexan added a commit that referenced this pull request May 6, 2022
This applies the fix of #83 to linker argument @files.  Linker
argument files are used on (at least) Windows.
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.

2 participants