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

Support usage in projects using Gradle 8.10 by avoiding using Project.equals #257

Merged
merged 2 commits into from
Aug 23, 2024

Conversation

anthonycr
Copy link
Contributor

@anthonycr anthonycr commented Aug 20, 2024

I can't find anything in the Gradle 8.10 release notes or issue tracker, but we've found that the affected module detector (both versions 0.3.1 and 0.3.0) stops working on Gradle 8.10.

Using 0.3.1 we are seeing that no tasks will run and AMD always sees that no modules have changed. I downgraded our integration to 0.3.0 to obtain logs and work around #205, and saw that AMD is correctly identifying all the changed modules, but when it goes to identify whether those changed modules should be included in the build, it always returns false for every module.

I suspected that the cause was the use of Set<Project> and subsequent usage of Set.contains(Project), which would rely on the Project intrinsic equals implementation. Neither Project nor DefaultProject implement equals, and it seems like separate Project instances are getting created now in Gradle 8.10 (I'm not sure why). As a result, the default equals implementation is returning false when checking equivalent projects. I added this repository as an included build and updated the usage of Set<Project> to Map<String, Project> where String was the project path, and AMD worked as expected.

This MR updates the usage of Set<Project> to Map<ProjectPath, Project> where ProjectPath is a value class that wraps the path. It updates the tests to expect Map<ProjectPath, Project> instead of Set<Project>, but it does not update the Gradle version. Note: I didn't update the Gradle version so that we don't potentially lock out consumers on older Gradle versions from using AMD.

I can work on a repro project containing both Gradle 8.9 and 8.10, if needed, to demonstrate the issue.

… Project inequivalence in newer Gradle versions
@CLAassistant
Copy link

CLAassistant commented Aug 20, 2024

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@joshafeinberg
Copy link
Member

joshafeinberg commented Aug 20, 2024

Thanks for this, first thing I've found is the sample app's buildSrc needs a bit of an update in /sample/buildSrc/src/main/kotlin/com/dropbox/sample/tasks/AffectedTasksPlugin.kt. it references the map so it can't figure out the lambda

If you could push up a fix for that I can run the CI and get some tests going

@anthonycr
Copy link
Contributor Author

Thanks for this, first thing I've found is the sample app's buildSrc needs a bit of an update in /sample/buildSrc/src/main/kotlin/com/dropbox/sample/tasks/AffectedTasksPlugin.kt. it references the map so it can't figure out the lambda

If you could push up a fix for that I can run the CI and get some tests going

Oops! I didn't notice that the sample was being configured separately from the plugin 😅 Updated the sample app to compile.

@joshafeinberg joshafeinberg merged commit d89407b into dropbox:main Aug 23, 2024
4 checks passed
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.

3 participants