Stop build if ANDROID_NDK_ROOT is not defined for android lib #520

thunderbiscuit · 2024-04-30T14:41:51Z

This PR simplifies the Android plugin and forces users to have the ANDROID_NDK_ROOT environment variable defined before attempting to build the Android library. We've had reports that build would fail but devs would not know why, and this was because their ANDROID_SDK_ROOT was not defined, and the plugin assumed it was. This change now makes the plugin throw. Info on how to build and solve the issue is in the readme.

Fixes #295.

Summary by CodeRabbit

Documentation
- Updated setup instructions for ANDROID_SDK_ROOT and ANDROID_NDK_ROOT on macOS and Linux in the README.
Bug Fixes
- Modified build process to halt if ANDROID_NDK_ROOT is not set, ensuring correct environment setup.
Refactor
- Removed an unnecessary empty line in Enums.kt to improve code cleanliness.

coderabbitai · 2024-04-30T14:41:59Z

Walkthrough

The modifications primarily focus on enhancing the configuration of environment variables for the Android build process in the bdk-android project. Changes include path updates for ANDROID_SDK_ROOT and ANDROID_NDK_ROOT, and a new error check to ensure these variables are set, improving the build's robustness.

Changes

File Path	Change Summary
`bdk-android/README.md`	Updated paths for `ANDROID_SDK_ROOT` and `ANDROID_NDK_ROOT` on macOS and Linux.
`.../plugins/src/main/kotlin/org/bitcoindevkit/...`	Added error check for unset `ANDROID_SDK_ROOT` or `ANDROID_NDK_ROOT`.
`.../plugins/src/main/kotlin/org/bitcoindevkit/...`	Minor cleanup by removing an empty line.

Assessment against linked issues

Objective	Addressed	Explanation
Add check for missing environment variables in Android plugin (#295)	✅

The changes successfully address the primary objective of issue #295 by implementing a check that halts the build process if ANDROID_SDK_ROOT or ANDROID_NDK_ROOT is not set, thereby ensuring that necessary environment variables are present before proceeding.

Recent Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between f31678b and d4736a6.

Files selected for processing (3)

bdk-android/README.md (1 hunks)
bdk-android/plugins/src/main/kotlin/org/bitcoindevkit/plugins/Enums.kt (1 hunks)
bdk-android/plugins/src/main/kotlin/org/bitcoindevkit/plugins/UniFfiAndroidPlugin.kt (4 hunks)

Files skipped from review as they are similar to previous changes (2)

bdk-android/plugins/src/main/kotlin/org/bitcoindevkit/plugins/Enums.kt
bdk-android/plugins/src/main/kotlin/org/bitcoindevkit/plugins/UniFfiAndroidPlugin.kt

Additional Context Used

LanguageTool (20)

bdk-android/README.md (20)

Near line 1: Possible spelling mistake found.
Context: # bdk-android This project builds an .aar package for...

Near line 2: Possible spelling mistake found.
Context: # bdk-android This project builds an .aar package for the Android platform that p...

Near line 2: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...r package for the Android platform that provide Kotlin language bindings for the [bdk...

Near line 2: Possible spelling mistake found.
Context: ...rovide Kotlin language bindings for the [bdk] library. The Kotlin language bindings...

Near line 2: Possible spelling mistake found.
Context: ...in language bindings are created by the [bdk-ffi] project which is included in the root...

Near line 2: Possible missing comma found.
Context: ...bindings are created by the [bdk-ffi] project which is included in the root of this r...

Near line 4: This phrase is duplicated. You should probably use “to Use” only once.
Context: ...in the root of this repository. ## How to Use To use the Kotlin language bindings for [bdk...

Near line 5: Possible spelling mistake found.
Context: ...To use the Kotlin language bindings for [bdk] in your Android project add the follo...

Near line 5: Possible missing comma found.
Context: ...ge bindings for [bdk] in your Android project add the following to your gradle depend...

Near line 5: Possible spelling mistake found.
Context: ...droid project add the following to your gradle dependencies: ```kotlin repositories { ...

Near line 17: Possible spelling mistake found.
Context: ...elease, specify the snapshot repository url in the repositories block and use the...

Near line 29: Possible spelling mistake found.
Context: ...PSHOT>") } ``` ### Example Projects * [bdk-kotlin-example-wallet](https://github.com/bitcoindevkit/bdk-k...

Near line 40: Possible spelling mistake found.
Context: ...kit/bdk-ffi ``` 2. Follow the "General" bdk-ffi ["Getting Started (Developer)"] instruc...

Near line 62: Possible spelling mistake found.
Context: ...SDK_ROOT/ndk/25.2.9519653 7. Build kotlin bindings sh # build Android librar...

Near line 79: Possible spelling mistake found.
Context: ...u do wish to sign them, simply set your ~/.gradle/gradle.properties signing key values l...

Near line 85: Possible spelling mistake found.
Context: ...YOUR_GNUPG_PASSPHRASE> ``` and use the publishToMavenLocal task without the `localBuild` flag: ``...

Near line 85: Possible spelling mistake found.
Context: ... publishToMavenLocal task without the localBuild flag: ```shell ./gradlew publishToMave...

Near line 92: Possible spelling mistake found.
Context: ...ght not have the JNA dependency on your classpath. The exception thrown will be ```shell ...

Near line 105: Possible spelling mistake found.
Context: ...by default. This will not work with the bdk-android library, as we do not support 32-bit ar...

Near line 105: Possible spelling mistake found.
Context: ...install an x86_64 emulator to work with bdk-android. [bdk]: https://github.com/bitcoinde...

Additional comments not posted (1)

bdk-android/README.md (1)

54-58: Update the ANDROID_SDK_ROOT and ANDROID_NDK_ROOT environment variable setup instructions for macOS and Linux.

The updated paths for macOS and the explicit inclusion of the NDK version enhance clarity and ensure developers set up their environment correctly. This change aligns with the PR's objective to make the build process more robust by enforcing environment variable checks.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Note: Auto-reply has been disabled for this repository by the repository owner. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

reez · 2024-04-30T14:47:23Z

bdk-android/plugins/src/main/kotlin/org/bitcoindevkit/plugins/UniFfiAndroidPlugin.kt

-                environment(
-                    Pair("ANDROID_NDK_ROOT", "${System.getenv("ANDROID_SDK_ROOT")}/ndk-bundle")
-                )
+                throw IllegalStateException("ANDROID_NDK_ROOT environment variable is not set; cannot build library")


Interesting, good to know how you stop the build here

Yeah and I can actually check for it a bit earlier and remove duplicated code. Pushed a fix just now.

reez

ACK d4736a6

thunderbiscuit changed the title ~~Stop build if android ndk root is not defined for android lib~~ Stop build if ANDROID_NDK_ROOT is not defined for android lib Apr 30, 2024

reez reviewed Apr 30, 2024

View reviewed changes

build: stop build if android ndk root is not defined for android lib

d4736a6

thunderbiscuit force-pushed the feature/variable-check branch from 0d0cf3f to d4736a6 Compare April 30, 2024 14:53

reez approved these changes Apr 30, 2024

View reviewed changes

thunderbiscuit merged commit d4736a6 into bitcoindevkit:master Apr 30, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stop build if ANDROID_NDK_ROOT is not defined for android lib #520

Stop build if ANDROID_NDK_ROOT is not defined for android lib #520

thunderbiscuit commented Apr 30, 2024 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Apr 30, 2024 •

edited

Loading

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

Documentation and Community

reez Apr 30, 2024

thunderbiscuit Apr 30, 2024

reez left a comment •

edited

Loading

Stop build if ANDROID_NDK_ROOT is not defined for android lib #520

Stop build if ANDROID_NDK_ROOT is not defined for android lib #520

Conversation

thunderbiscuit commented Apr 30, 2024 • edited by coderabbitai bot Loading

Summary by CodeRabbit

coderabbitai bot commented Apr 30, 2024 • edited Loading

Walkthrough

Changes

Assessment against linked issues

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

Documentation and Community

reez Apr 30, 2024

Choose a reason for hiding this comment

thunderbiscuit Apr 30, 2024

Choose a reason for hiding this comment

reez left a comment • edited Loading

Choose a reason for hiding this comment

thunderbiscuit commented Apr 30, 2024 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Apr 30, 2024 •

edited

Loading

CodeRabbit Configration File (`.coderabbit.yaml`)

reez left a comment •

edited

Loading