Bugfix/gradle fixes tweaks#698
Conversation
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the library copying and XML generation logic in build.gradle. It splits the monolithic copyProjectLibs task into modular, declarative Copy tasks (copyProjectBaseLibraries, copyProjectLibraries, and copyProjectTestData). It also introduces a recursive dependency resolver (buildDependencyResolver) to dynamically resolve transitive dependencies for createProjectXml. The reviewer feedback highlights several key improvement opportunities: avoiding unnecessary disk I/O in file copying tasks by using details.name instead of details.file.name, optimizing the recursive dependency collection to use an accumulator instead of list concatenation, preventing potential malformed XML when jmeJarFiles is empty, and skipping XML generation for self-resolving dependencies that yield no artifacts.
| libraryWriter << '<!DOCTYPE library PUBLIC "-//NetBeans//DTD Library Declaration 1.0//EN" "http://www.netbeans.org/dtds/library-declaration-1_0.dtd">' << eol | ||
| libraryXml.library(version: "1.0", encoding: "UTF-8") { | ||
| makeName(libraryXml, "${dep.name}") | ||
| makeName(libraryXml, "${name}") |
There was a problem hiding this comment.
For third-party libraries (where jmeJarFiles is empty), accessing jmeJarFiles[0] on line 489 will return null, resulting in an invalid classpath entry: <resource>jar:nbinst://com.jme3.gde.project.baselibs/libs/null!/</resource>.\n\nTo fix this, the classpath resource for jmeJarFiles[0] should be wrapped in a check to ensure jmeJarFiles is not empty:\n\ngroovy\nif (!jmeJarFiles.isEmpty()) {\n resource "jar:nbinst://com.jme3.gde.project.baselibs/libs/${jmeJarFiles[0]}!/"\n}\n\n\nSince line 489 is outside the modified diff hunk, this cannot be applied via a direct code suggestion, but it should be addressed to avoid generating malformed XML files.
There was a problem hiding this comment.
I think this was always the case, I'm sure it is fine
Phew, I think finally done. Of course not compatible with Gradle 10 but will do for now. Mostly ChatGPTd it.
Resolves #696