Fix #8764 Gradle 9 ProjectDependency resolution to other project #8797
Conversation
mbien
added
Gradle
| Function<ProjectDependency, Project> projDependencyToProject = | ||
| sinceGradleOrDefault( | ||
| "9.0", | ||
| () -> { | ||
| Method getPath = ProjectDependency.class.getMethod("getPath"); | ||
| return dep -> { | ||
| try { | ||
| String path = (String) getPath.invoke(dep); | ||
| return project.findProject(path); | ||
| } catch (ReflectiveOperationException e) { | ||
| throw new UnsupportedOperationException(e); | ||
| } | ||
| }; | ||
| }, | ||
| () -> ProjectDependency::getDependencyProject); // removed in Gradle 9 |
There was a problem hiding this comment.
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 call getPath() directly?
There was a problem hiding this comment.
Gradle >= 8.11 but <9 should have both APIs
There was a problem hiding this comment.
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.
@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 )
|
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
mbien
left a comment
There was a problem hiding this comment.
changes look good to me, CI is happy too
|
@wrprice thanks for the fix and congrats on your first contribution! |
2 participants
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)