-
-
Notifications
You must be signed in to change notification settings - Fork 899
Add rule for android native app(NDK) #7123
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
Conversation
- Add glue file and jni interface file to target;
- Generate android apk package at install stage;
- Use adb to run the android app.
Summary of ChangesHello @keosu, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances xmake's capabilities for Android C++ development by introducing a dedicated rule that automates the entire build, packaging, and deployment lifecycle. It integrates seamlessly with the Android SDK and NDK toolchains, abstracting away the complexities of APK creation, signing, and on-device execution, thereby providing a more efficient and developer-friendly experience. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request introduces a new xmake rule for building Android C++ applications, which includes functionality for packaging an APK and running it on a device. My review has identified a few areas for improvement to enhance the robustness, security, and correctness of the new rule. Key feedback includes fixing a critical bug related to a hardcoded architecture, addressing a security concern with a default keystore password, improving error handling, and correcting some typos and dead code. The suggested changes aim to make the new rule more reliable and secure for users.
|
你这里的用法是作为 android native app 来构建的。 那 install 的逻辑就不对,不应该是 打包 apk 而应该是 直接安装到设备。
|
|
另外,你现在的测试工程,这边签名有问题 |
|
还有,请在 tests 下提供一个完整的测试例子工程,不要有中文注释,不要有无关的东西,尽可能最精简的模版工程。 还有 rule 定义开头,加上完整的使用注释 |
Also, please provide a complete test example project under tests. No Chinese comments, no irrelevant things, and the most streamlined template project possible. There is also the beginning of the rule definition, plus complete usage comments |
|
Hi @waruqi , thanks for your review, I have updated the PR and resolved most issues, please help to review again. |
| -- define rule: build android native app with NDK | ||
| rule("android.native_app") | ||
| -- this rule only for android target | ||
| if is_plat("android") then |
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.
Did you test it? please call target:is_plat in script scope.
error: @programdir/rules/ndk/xmake.lua:24: attempt to call a nil value (global 'is_plat')
stack traceback:
[@programdir/rules/ndk/xmake.lua:24]: in main chunk
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.
That's weird, is_plat works well on my windows laptop and the ubuntu ci while target:is_plat not.
But this is tested as an external custom rule, does this matter? I don't know how to test within xmake project itself now, and the CI seems not triggered for my PR.
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.
you need to add a test.lua file in example project folder. you can see other tests example
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.
but there is not android ndk on ci now, maybe it cannot be tested.
|
Because there were too many changes required, I made some improvements based on your patch: #7139 |
Test project path: https://github.com/keosu/xmake/tree/dev/tests/projects/c%2B%2B/ndk_raylib
A standalone example repo: https://github.com/keosu/raycpp_android_xmake/tree/pr Andriod CI pass