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

Proposal: Port plugin to AndroidComponentsExtension #369

Closed

Conversation

kkris
Copy link
Contributor

@kkris kkris commented Nov 6, 2022

I took some time to debug #349 with AGP 7.3.x and I think the root of the problem is that AGP now ignores the addition of the source set. It seems one can set it fine, but it is just silently ignored. I verified this by setting some break points inside the AGP code during the merge resources task and the easylauncher source set is not there. Any other attempts to fix this failed.

I then stumbled over the AndroidComponentsExtension (https://developer.android.com/reference/tools/gradle-api/4.2/com/android/build/api/extension/AndroidComponentsExtension) which provides an API for what easylauncher actually wants to achieve: create a new source set, use a task to generate some new assets and hook task execution and source set merging into the build process.

This required some refactorings (summarized below), but now the implementation no longer relies on implicit assumptions (such as the name of the merge resources task). As far as I can tell, there are only two regressions for which I have not yet found a solution:

  • the project requires setting android.disableResourceValidation=true
  • the API does not allow to query the isDebuggable flag

Following is a brief summary of changes:

  • Move most parts of the processing into the task execution (instead of at configuration time)
    • Manifest parsing
    • Icon file gathering
  • Port to AndroidComponentsExtension API
    • Entry point: variant.artifact(...)
    • This tells AGP to execute this task which will generate some resources. The path handling and the task dependencies are automatically taken care of

The sample projects now work again with AGP 7.3.x. Was hoping to get some feedback on this approach and especially the two remaining issues before continuing.

@mateuszkwiecinski
Copy link
Member

mateuszkwiecinski commented Nov 12, 2022

Tremendous! 🎉 This addresses a long overdue migration I wanted to do, but waited until https://issuetracker.google.com/issues/197121905 and https://issuetracker.google.com/issues/170650362 are somehow addressed. But if the migration fixes the linked issue, then even better 🚀 🚀 🚀

I opened a #380 where I'll try to do a follow-up and implement missing stuff.

Regarding:

the project requires setting android.disableResourceValidation=true

I think that's fine 🤷 In the issue report, Google folks acknowledged it's an invalid behaviour, and mentioned this is a workaround, so I hope it'll be eventually addressed on AGP side. If I'm not mistaken, it's already fixed in latest AGP 8.0.0 alphas 👀

the API does not allow to query the isDebuggable flag

I was able to come up with a workaround, which heavily relies on current AGP implementation:

internal val Variant.isDebuggable
    get() = when (this) {
        is AnalyticsEnabledApplicationVariant -> delegate.isDebuggable
        is ComponentCreationConfig -> debuggable
        else -> false
    }

if that breaks, I'll figure something else. Although I'd expected a P1 issue will eventually be addressed at some point 🤷

Thanks again! Great idea on moving work into task execution rather than doing things in configuration time 🚀

@mateuszkwiecinski
Copy link
Member

Merged in #380, released in 6.0.0

@kkris
Copy link
Contributor Author

kkris commented Nov 12, 2022

You're welcome :)

Thank you for cleaning up and fixing the open issues! Glad to know that the current workarounds are known AGP bugs.

@kkris kkris deleted the port-to-android-components-extension branch November 12, 2022 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants