search

bugsnag / bugsnag-android-gradle-plugin

Gradle plugin for BugSnag which uploads ProGuard, DexGuard and NDK mapping files, and sends build notifications
https://docs.bugsnag.com/build-integrations/gradle/
MIT License
70 stars 38 forks source link

Fix NdkToolchain.baseDir configuration cache #491

Closed ZacSweers closed 1 year ago

ZacSweers commented 1 year ago

This property should be a DirectoryProperty rather than Property<File> in order to be compatible with configuration cache. This also allows for it to be more chainable with providers of this from android components.

Goal

See above

Design

See above

Changeset

See above

Testing

Manually tested in our project via maven local.

ZacSweers commented 1 year ago

Testing this reveals a separate issue though, which is that it's a required property for NdkToolchain and tasks to be configured, even if they aren't run. This presents a problem for consumers that don't have the NDK installed (even if they don't use it), as the property configuration then fails. There is a flag for this via the upload task control, but it's checked eagerly and hard to disable. I could take a crack at improving it here (i.e. make the baseDir property optional), or leave that for a followup.

johnkiely1 commented 1 year ago

Thanks for the PR @ZacSweers, we will review as soon as we can.

lemnik commented 1 year ago

Hi @ZacSweers, thanks so much for this PR and pointing out the eager-config issue as well. Would you suggest making the NdkToolchain references a Provider instead of a hard-reference (specifically for tasks register functions)?

ZacSweers commented 1 year ago

I'm not sure if that would help. I think where you'd want to end up is probably somewhere where you register the tasks regardless but make them no-op (± a warning log) or error at task-action-time if no NDK is found

lemnik commented 1 year ago

@ZacSweers I've moved this commit over to: https://github.com/bugsnag/bugsnag-android-gradle-plugin/pull/499 so I'm closing this PR.