fix: mark tasks as cacheable

[jeremylong]

resolves #100

This should only be merged after #476.

[chadlwilson]

Honestly, this is a bit scary. Can you please hold off?

1. The outputDirectory it uses by default is not one that is exclusively owned by the plugin; it is overlapping with the standard directories gradle ReportingExtension plugins use (layout.buildDirectory.dir('reports') including Gradle's own problem report.

   Caching disabled for task ':dependencyCheckAggregate' because:
     Gradle does not know how file 'target/reports/tests' was created (output property 'outputDir'). Task output caching requires exclusive access to output paths to guarantee correctness (i.e. multiple tasks are not allowed to produce output in the same location).
   

   $ tree target/reports | head -10
   target/reports
   ├── dependency-check-report.html
   ├── problems
   │   └── problems-report.html
   └── tests
       └── allTests
           ├── classes
           │   ├── com.thoughtworks.go.agent.AgentControllerTest.html
   

   The default configuration of the plugin should not clash with standard locations by default (or it should define only OutputFiles inside reports rather than the entire dir). The caching of outputs isn't a big deal as other plugins aren't so likely to depend on dep check's output, however that's not necesarily true.

2. There is also a big risk of ending up with stale results if you do this stuff incorrectly. In particular, the way this plugin resolves configurations and depends on other projects is unsafe, and may lead to it not re-running with another proejct's configuration is changed.

   e.g in my project I changed a dependency in a configuration for a subproject and re-ran.

   > Task :dependencyCheckAggregate UP-TO-DATE
   Custom actions are attached to task ':dependencyCheckAggregate'.
   Caching disabled for task ':dependencyCheckAggregate' because:
     Gradle does not know how file 'target/reports/tests' was created (output property 'outputDir'). Task output caching requires exclusive access to output paths to guarantee correctness (i.e. multiple tasks are not allowed to produce output in the same location).
   Skipping task ':dependencyCheckAggregate' as it is up-to-date.
   

   This is incorrect, dep check MUST re-run when relevant non-skipped input configurations have changed - however it does not model its inputs or dependent projects correctly.

3. Generally speaking the plugin probably needs to re-run (not have results cached) anyway, because it needs to check remote data sources such as NVD etc which may have changed since the last run. This isn't the type of plugin that suits the use of task caching unless you do something really sophisticated to link the cache expiries of all the analyzers.

[chadlwilson]

I think the OP in #100 (comment) is highlighting what I mention in #3) by suggesting that the "update" bits would need to be factored out into a separate task which can basically run every time to check NVD/OssIndex etc.

If those don't lead to a change in the dataDirectory contents, and there are no changed configurations, or files matching the scanSet, then theoretically you dont need to re-run the analyze.

Unfortunately, the way the engine is set up and the way many analyzers are "online" (not cacheable), I think it probably makes it a bit difficult to do this task/build caching in a predictable manner and to be useful?

Conceptually, you'd want every analyzer to be marked as to whether it depends on a "mutable online source" and then for the up-to-date-ness of the Gradle tasks to depend on the analyzers enabled.

[jeremylong]

This PR and #474 are not going to be merged without a lot more review, testing, and consideration of consequences.

Regarding point 3 above - I understand the need/desire to re-run the task if a day or more has passed and nothing has changed. However, if I run a build back-to-back that invokes the task, I believe it is fair to expect the task to be skipped if none of the inputs or outputs have been changed.

[chadlwilson]

Ok, cool - I thouhht perhaps you wanted to get it into the fix release today 😅

But yes I agree it'd be nice to define some semantics on up-to-date-ness here that are cache-aware (for NVD) and allow the user to, say, opt-out of rechecking certain online sources more frequently than X minutes even if they don't support caching of their results

[chadlwilson]

I guess you could try and make this dynamic based on information using custom https://docs.gradle.org/current/javadoc/org/gradle/api/tasks/TaskOutputs.html if the engine configuration can answer some questions somehow.

Like in pseudo-code:

outputs.cacheIf("All enabled analyzers are cacheable") { task ->
  task.forceCachingOfOnlineAnalyzers.get() || task.engine.allEnabledAnalyzersAreCacheable()
}

outputs.upToDateWhen { task ->
  !task.engine.needsDataSourceCacheRefresh()
}