Skip to content

Conversation

@jeremylong
Copy link
Contributor

No description provided.

@jeremylong jeremylong requested a review from Copilot October 11, 2025 21:02
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR migrates the OWASP Dependency Check Gradle plugin from legacy Gradle APIs to the modern Gradle Property API. The migration improves type safety and enables proper configuration caching support by replacing direct field access with Property objects.

  • Converts all configuration fields from primitive types to Property/ListProperty/DirectoryProperty objects
  • Updates all extension classes to use proper Gradle Property API patterns with getters/setters
  • Modifies task and configuration code to use .get() and .getOrNull() methods for property access

Reviewed Changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
Extension classes Convert fields to Property objects with proper constructors and accessor methods
Task classes Update property access to use .get() and .getOrNull() methods
Test specifications Update assertions to use Property API accessor methods
Main plugin class Update extension creation to pass ObjectFactory
README.md Minor documentation update with tracking pixel

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@jeremylong jeremylong merged commit 4f787ee into main Oct 12, 2025
3 checks passed
@jeremylong jeremylong deleted the propertyAPI branch October 12, 2025 20:53
@chadlwilson
Copy link
Collaborator

FWIW - this was (intentionally or not) a minor API breaking change, at least for groovy. Posting here in case anyone else comes across it

No longer works:

data.directory = layout.buildDirectory.dir("dependency-check-data").get()
> Cannot set the value of property 'directory' of type java.lang.String using a provider of type org.gradle.api.file.Directory.

Now it needs to actually be a string as it's not able to rely on Gradle to co-erce types for some reason; probably due to directly calling objects.property

dependencyCheck {
  data.directory = layout.buildDirectory.dir("dependency-check-data").get().asFile.path
}

@annsj102
Copy link

Has something been changed in regards to the scope of the scan?

With the update to 12.1.7 I have several projects that get failing checks due to a plugin classpath dependency. Is this intended? And is it possible to configure scope?

@jeremylong
Copy link
Contributor Author

@annsj102 can you open a seperate issue with more details?

@annsj102
Copy link

@annsj102 can you open a seperate issue with more details?

Done
#479

@Vampire
Copy link
Contributor

Vampire commented Oct 27, 2025

Doing majorly API breaking changes like this PR in minor version is quite unnice. :-/

@jeremylong
Copy link
Contributor Author

@Vampire sorry about this. In my testing I never saw an issue (which just means I need to improve my testing).

@chadlwilson
Copy link
Collaborator

Unfortunately (and perhaps obviously) the automated unit and integration tests for this plugin are pretty weak. Could probably do with help from the community.

@Vampire
Copy link
Contributor

Vampire commented Oct 27, 2025

Well, even if it would work the same in Groovy DSL and Kotlin DSL build scripts, even then it would be a breaking change if you change signatures / types.
Remember that there could be plugins integrating with your plugin like convention plugins which might be compiled Java classes or any other JVM language so that even if the changes would have been source-compatible (which they were not), it would be a breaking change. ;-)

@chadlwilson
Copy link
Collaborator

chadlwilson commented Oct 27, 2025

No-one is disagreeing that it was (unintentionally) a breaking change though. But also neither does this project promise a stable API for convention plugins that I have seen. Best to not infer promises that were never made - instead be prepared to adapt - or contribute.

Whether or not a given project has capacity to properly detect possible breaking changes, carefully manage them and batch them together etc etc is a function of the resources and community that project has to manage the codebase and user base it has. Not everyone is a Gradle expert, and as you know, Gradle itself makes so many breaking changes between releases that it's very difficult to keep up even as a user - let alone a plugin.

PRs welcome to improve the situation.

@Vampire
Copy link
Contributor

Vampire commented Oct 27, 2025

As you I do contribute PRs. :-)
But there is not much to improve except reverting the changes and rereleasing them as a major version change.
I also did no suggest to do that, just to keep in mind that every change in public signature is a breaking change just like for any library.
Of course you are not expected to not do breaking changes, especially as you nowhere suggest any backwards compatibility guarantees or semantic version semantics.
Just wanted to clarify that even if something would be a source-compatible change in a build script it can still break downstream users in a breaking way, so that if you consider not to do a breaking change that you have that in the back of your mind. :-)

Gradle itself makes so many breaking changes between releases

But always only in major versions and with a previous deprecation cycle :-) (accidental breaking changes aside of course :-D )

@chadlwilson
Copy link
Collaborator

Yup, appreciate that. I only posted here originally because i was surprised (as a user downstream) that this was broken in a patch release.

Gradle itself makes so many breaking changes between releases

But always only in major versions and with a previous deprecation cycle :-) (accidental breaking changes aside of course :-D )

You're missing the point.

The point is that even if you bump versions and communicate breaking changes (as Gradle mostly so - except when they don't - which happens often) there is still a cost to the sheer rate of change they are pushing.

And that cost has to be borne by users and plugin authors, regardless of their resources. Managing breaking changes carefully, batching and coordinating etc costs resources these projects do not have. For simplicity, this project does not even version independently of ODC proper, adding further complexity - and upstream is also not a strict semver.

What I am trying to say is that we are in this situation partly because of the cumulative effect of Gradle's decisions to break things and churn their API over the years. Not complaining, just pointing out the challenge this causes.

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.

5 participants