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

Made isRequiredBy compatible with composite gradle builds #460

Merged
merged 2 commits into from
Oct 10, 2024

Conversation

jamesbassett
Copy link
Contributor

@jamesbassett jamesbassett commented Oct 9, 2024

This change makes it possible to use isRequiredBy with a composite gradle build (attempting to do so currently fails with "Cannot use shouldRunAfter to reference tasks from another build").

See #459 for details.

I have tested this in my reproducer repo by publishing the plugin locally

  • using both variants of isRequiredBy, i.e. supplying either
    • tasks.getByName("test")
    • tasks.named<Test>("test")
  • with a composite build and a normal build

When testing the eager variant (getByName()) I encountered build failures on the toolchain config saying I couldn't modify it. I then realised that my use of task.taskDependencies.getDependencies() was eagerly creating tasks and resolving dependencies. So I changed it to use a TaskDependency proxy that allows the filtering to occur lazily, and the issues went away.

I've also tested it in a private repo that makes use of a multi-module composite build. It was here that I discovered I needed to make the filtering logic smarter and use the task's rootProject.name, as the included build name is always the root project.

In terms of configuration avoidance, I've done build scans and I don't see any tasks "Created during configuration" as a result of the plugin. In my reproducer repo I seem to get 5 as a result of using a composite build (0 when changing it to a normal multi module build), but notice no difference as a result of this plugin. It goes up to 6 if I eagerly fetch the task, i.e. tasks.getByName("test").

Compatibility-wise I don't think I'm using anything risky/new/breaking from the Gradle API.

I've also minimised the risk of this change breaking anything by only attempting to filter if there are included builds (otherwise it retains the original logic).

@augi augi enabled auto-merge (squash) October 10, 2024 11:09
@augi augi merged commit ccd287f into avast:main Oct 10, 2024
1 check passed
@augi
Copy link
Member

augi commented Oct 10, 2024

Thank you very much for your contribution! 👍 Released as 0.17.9 🚀

@jamesbassett jamesbassett deleted the composite_builds_fix branch October 10, 2024 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants