fix(gradle): Refactored init-script-gradle.ftl to add support for configuration cache in gradle projects. (IDETECT-4812) #1531
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the Gradle Native Inspector init script to support Gradle's configuration cache by moving all Project API usage to the configuration phase, ensuring compatibility with Gradle 6.x+ and future-proofing for Gradle 9+.
Key changes:
- Moved all project property access and task configuration to the
afterEvaluateconfiguration phase - Replaced runtime Project API calls with pre-captured values to avoid configuration cache violations
- Added comprehensive error handling with try/catch blocks throughout the script
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| File projectFile = new File(projectFilePathConfig) | ||
| if (projectFile.exists()) { | ||
| projectFile.delete() | ||
| } | ||
| projectFile.createNewFile() |
There was a problem hiding this comment.
Creating and deleting files during the configuration phase can cause issues with configuration cache and parallel builds. File operations should be moved to task execution phase (doFirst/doLast) to avoid configuration cache violations and ensure proper build isolation.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
This is a known issue. Even though creating file should be part of execution phase but then setting configuration objects like outputFile to that created file, when done should be part of configuration phase. I would like to keep this as it is.
|
|
||
| // Set the configurations at configuration time if possible | ||
| if (!selectedConfigs.isEmpty()) { | ||
| dependenciesTask.configurations = selectedConfigs |
There was a problem hiding this comment.
Setting task properties directly with Configuration objects can cause serialization issues with configuration cache. Consider using configuration names and resolving configurations during task execution instead.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Setting configuration object properties during configuration phase is not causing serialization issues as tested with multiple projects. I assume this is what Gradle wants for now.
| if (shouldIncludeProject) { | ||
| // Create output file directly during configuration | ||
| File projectFile = new File(projectFilePathConfig) | ||
| if (projectFile.exists()) { | ||
| projectFile.delete() | ||
| } | ||
| projectFile.createNewFile() | ||
|
|
||
| // Set the output file during configuration phase | ||
| try { | ||
| dependenciesTask.outputFile = projectFile | ||
| println "Set output file during configuration to " + projectFile.getAbsolutePath() | ||
| } catch (Exception e) { | ||
| println "Could not set outputFile property during configuration: " + e.message | ||
| e.printStackTrace() | ||
| } | ||
| } | ||
|
|
||
| if(dependencyTask.metaClass.respondsTo(dependencyTask,"setOutputFile")) { | ||
| println "Updating output file for task to "+projectFile.getAbsolutePath() | ||
| // modify the output file | ||
| setOutputFile(projectFile) | ||
| } else { | ||
| println "Could not find method 'setOutputFile'" | ||
| dependenciesTask.doFirst { | ||
| try { | ||
| if (extractionDir == null) { | ||
| throw new IllegalStateException("GRADLEEXTRACTIONDIR system property is not set") | ||
| } | ||
|
|
There was a problem hiding this comment.
Setting the outputFile property with a File object created during configuration phase violates configuration cache requirements. The file creation should be deferred to task execution time.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Already explained in the above comments.
| println "ERROR in computeProjectFilePath: " + e.message | ||
| e.printStackTrace() | ||
| // Do nothing else |
There was a problem hiding this comment.
[nitpick] The comment '// Do nothing else' after error handling is unclear. Consider either removing the comment or explaining what the expected behavior should be when an error occurs.
| println "ERROR in computeProjectFilePath: " + e.message | |
| e.printStackTrace() | |
| // Do nothing else | |
| // Return null to indicate failure | |
| return null |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
If the gradlew executable fails to run, Detect still runs as per the old code, falling back to the gradle project inspector Hence we do nothing.
| println "ERROR in getFilteredConfigurationNames: " + e.message | ||
| e.printStackTrace() | ||
| // Do nothing else |
There was a problem hiding this comment.
[nitpick] Same unclear comment as above. The error handling should either return a default value or rethrow the exception rather than silently continuing with undefined behavior.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Already explained in the above comments.
Prep doc for 11.0.0 Fix Nuget note formatting
# Conflicts: # shared-version.properties
…ependency graph for a Go project (#1529) * Ignore the toolchain module in the go mod graph output because it is part of the Go internal tooling. * Address review comments: use constants and add comment for clarity
* Ignore the toolchain module in the go mod graph output because it is part of the Go internal tooling. * Address review comments: use constants and add comment for clarity * Add release note * Update release note
…1468) * WIP * file encoding temp fix * Try windows-1252 when reading file to string * Remove debugging pieces * Check if decoded content is precomposed or decomposed * Use windows 1252 by default on created results file * Latest fix + debug logs * Fix typo and add additional logs * Clean up MR for review * Add releaase note * Update release note wording * Address review comment: Move the normalization outside try/catch so it's more clear what the try/catch is targeting.
…figuration cache in gradle projects. (IDETECT-4812)
bd-samratmuk
changed the title
bd-samratmuk
requested review from
zahidblackduck,
shantyk,
devmehtabd and
dterrybd
Issue:
Details
Key changes:
Configuration Cache Compatibility:
The script now ensures all Project API usage (e.g., reading properties, setting task fields) happens during the configuration phase, making it compatible with Gradle's configuration cache (introduced in 6.x, required in 9.x+).
Task Configuration Refactor:
All logic that interacts with project properties, configurations, and output files is moved to the configuration phase (afterEvaluate), avoiding runtime access to forbidden APIs.
Backward Compatibility:
The script uses standard Gradle APIs and avoids advanced features, so it should work with older Gradle versions (pre-6.x) as well.
Error Handling:
Extensive try/catch blocks are added to log errors and prevent build failures due to unexpected issues.
Metadata and Output File Handling:
Output files and metadata are created and set during configuration, not execution, ensuring compliance with configuration cache requirements.
Configuration Filtering:
Configuration filtering is done by name, not by passing configuration objects, to avoid serialization issues with the cache.
No Breaking Changes:
The overall workflow and output remain the same, so existing integrations should not break.
Gradle Best Practices:
The script follows Gradle best practices for cacheable builds and future-proofs the inspector for Gradle 9+.