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

Use Kotlin instead of Groovy for Android build configuration #2902

Merged
merged 10 commits into from
Oct 10, 2024

Conversation

kaushalbx
Copy link
Contributor

Groovy to kotlin dsl migration, solving issue #2699

@kaushalbx
Copy link
Contributor Author

@louwers have fixed the error related to checkstyle.

@louwers louwers added build Related to build, configuration or CI/CD android labels Oct 7, 2024
@louwers louwers requested a review from artakka October 7, 2024 10:40
@louwers louwers linked an issue Oct 7, 2024 that may be closed by this pull request
@louwers
Copy link
Collaborator

louwers commented Oct 7, 2024

@adrian-cojocaru Could you check if the Vulkan validation layers still work with this?

@adrian-cojocaru
Copy link
Collaborator

Vulkan validation doesn't seem to be present in the build (the download never starts?).

@louwers
Copy link
Collaborator

louwers commented Oct 7, 2024

@adrian-cojocaru How did you enable it before?

@adrian-cojocaru
Copy link
Collaborator

It's enabled by default for vulkanDebug

@kaushalbx
Copy link
Contributor Author

Vulkan validation doesn't seem to be present in the build (the download never starts?).

@adrian-cojocaru It's present in the build at line no. 9 at 'platform/android/MapLibreAndroid/build.gradle.kts'

kotlin("android")
id("maplibre.download-vulkan-validation")
id("maplibre.gradle-checkstyle")

@kaushalbx
Copy link
Contributor Author

@louwers , @adrian-cojocaru I have made some changes for Vulkan validation, Plz check if it's working now.

@kaushalbx
Copy link
Contributor Author

@louwers , @adrian-cojocaru it has passed all the checks, please confirm if there is still an issue in Vulkan.

@adrian-cojocaru
Copy link
Collaborator

@kaushalbx From a quick look it seems that the unzip task never calls the download one. Running gradlew :MapLibreAndroid:unzip does work as intended but in Android Studio the download never starts.

@kaushalbx
Copy link
Contributor Author

@adrian-cojocaru now I am able to see unzipped file at 'platform/android/MapLibreAndroid/src/vulkanDebug/jniLibs'. This is what we are expecting?

@kaushalbx
Copy link
Contributor Author

kaushalbx commented Oct 7, 2024

@adrian-cojocaru when i ran the command gradlew :MapLibreAndroid:unzip it downloaded the zip file at 'platform/android/MapLibreAndroid/android-binaries-1.3.290.0.zip' and unzipped it at 'platform/android/MapLibreAndroid/src/vulkanDebug/jniLibs/android-binaries-1.3.290.0'.
I'm going to push the changes, I think this is the expected behaviour.

@kaushalbx
Copy link
Contributor Author

@louwers please run the checks.

@adrian-cojocaru
Copy link
Collaborator

The files should be in platform/android/MapLibreAndroid/src/vulkanDebug/jniLibs/ without the intermediate android-binaries-1.3.290.0. Everything else is ok.

@louwers louwers changed the title Use Kotlin instead of Groovy for Android build configuration #2699 Use Kotlin instead of Groovy for Android build configuration Oct 9, 2024
@louwers
Copy link
Collaborator

louwers commented Oct 9, 2024

Thanks for addressing the final comments @kaushalbx!

The Vulkan render tests started failing on CI at least, @adrian-cojocaru do you think this is an issue with the build config?

@louwers
Copy link
Collaborator

louwers commented Oct 9, 2024

The Vulkan render tests failed before, I hope they pass now...

Thanks for the reviews @adrian-cojocaru and @westnordost!

@louwers louwers enabled auto-merge (squash) October 10, 2024 08:07
@louwers louwers merged commit 8d284cf into maplibre:main Oct 10, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android build Related to build, configuration or CI/CD
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use Kotlin instead of Groovy for Android build configuration
4 participants