- Notifications
You must be signed in to change notification settings - Fork914
Fix #8764 Gradle 9 ProjectDependency resolution to other project#8797
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
| Function<ProjectDependency,Project>projDependencyToProject = | ||
| sinceGradleOrDefault( | ||
| "9.0", | ||
| () -> { | ||
| MethodgetPath =ProjectDependency.class.getMethod("getPath"); | ||
| returndep -> { | ||
| try { | ||
| Stringpath = (String)getPath.invoke(dep); | ||
| returnproject.findProject(path); | ||
| }catch (ReflectiveOperationExceptione) { | ||
| thrownewUnsupportedOperationException(e); | ||
| } | ||
| }; | ||
| }, | ||
| () ->ProjectDependency::getDependencyProject);// removed in Gradle 9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
IMO: this would be more future proof if we would call the new method directly and use reflection for the removed method. (basically the other way around)
@lkishalmi@sdedic do you think we can bump the gradle tooling version of this module so that we can callgetPath() directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Gradle >= 8.11 but <9 should have both APIs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I have an alternative branch where I think I've done the necessary work to bump this module up to Gradle 8.11.1. Tested locally on some personal Gradle projects using 7.5.x, 8.11-14, and 9.0.0 and they all open w/out error notifications. In the process there was a fun roadbump that turns out to be a JDK bug re: method references under certain conditions.
The 8.11.1 upgrade has several more required changes compared to this spot fix. LMK how you'd like me to proceed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@wrprice sounds great! We could try to get this PR here in first, then see if we can bump the tooling version.
Last time I tried I could get a hello world project using gradle 5.x to build, but I believe 4 didn't work anymore. But I personally don't use gradle all that much, I let others comment on the versions question. NB generally tries to support lower versions if it is still reasonably maintainable while keeping up with latest releases.
The 8.11.1 upgrade has several more required changes compared to this spot fix. LMK how you'd like me to proceed.
Once this PR here is merged, we could certainly take a look at the version bump proposal, unless there is a showstopper I am not aware of (cc@lkishalmi )
mbien commentedSep 10, 2025
looks like the paperwork check doesn't like the commit header. The reason for that is because github added the Assuming your local You can see if it is worked by looking at the patch file for this PR: |
mbien left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
changes look good to me, CI is happy too
d10bf81 intoapache:masterUh oh!
There was an error while loading.Please reload this page.
mbien commentedSep 10, 2025
@wrprice thanks for the fix and congrats on your first contribution! |
Uh oh!
There was an error while loading.Please reload this page.
In multi-project Gradle builds where one project depends on another project, for example:
The dependency is represented in the Gradle API as a
ProjectDependencyobject, butProjectDependency::getDependencyProjectwas deprecated, and then removed in Gradle 9.0.0. When using Gradle >= 9, it now requires two steps to resolve target of the dependency:ProjectDependency::getPath(added in Gradle 8.11)Project::findProject(String path)fixes#8764
^Add meaningful description above
Click to collapse/expand PR instructions
By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -
Please make sure (eg.
git log) that all commits have a valid name and email address for you in the Author field.If you're a first time contributor, see the Contributing guidelines for more information.
If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.
PR approval and merge checklist:
If this PR targets the delivery branch:don't merge. (full wiki article)