Only write dependency-graph on build success

Implementation uses "RunWork" build operation on Gradle 8.+, and "buildFinished" on older versions. Fixes #115
gradle · Feb 13, 2024 · 9e5f3a4 · 9e5f3a4
1 parent 13e9c3a
commit 9e5f3a4
Show file tree

Hide file tree

Showing 5 changed files with 85 additions and 10 deletions.
diff --git a/plugin-test/src/test/groovy/org/gradle/github/dependencygraph/BaseExtractorTest.groovy b/plugin-test/src/test/groovy/org/gradle/github/dependencygraph/BaseExtractorTest.groovy
@@ -99,6 +99,16 @@ abstract class BaseExtractorTest extends Specification {
         return result
     }
 
+    protected BuildResult runAndFail() {
+        return runAndFail("ForceDependencyResolutionPlugin_resolveAllDependencies")
+    }
+
+    protected BuildResult runAndFail(String... names) {
+        executer.withTasks(names)
+        result = getExecuter().runWithFailure()
+        return result
+    }
+
     @CompileDynamic
     protected void applyDependencyGraphPlugin() {
         File pluginJar = TEST_CONFIG.asFile("extractorPlugin.jar.path")

diff --git a/...st/src/test/groovy/org/gradle/github/dependencygraph/DependencyExtractorConfigTest.groovy b/...st/src/test/groovy/org/gradle/github/dependencygraph/DependencyExtractorConfigTest.groovy
@@ -52,6 +52,17 @@ class DependencyExtractorConfigTest extends BaseExtractorTest {
         assert job.id == environmentVars.jobId
     }
 
+    def "fails gracefully if configuration values not set"() {
+        when:
+        def envVars = environmentVars.asEnvironmentMap()
+        envVars.remove("GITHUB_DEPENDENCY_GRAPH_JOB_CORRELATOR")
+        executer.withEnvironmentVars(envVars)
+        def result = executer.runWithFailure()
+
+        then:
+        result.output.contains("> The configuration parameter 'GITHUB_DEPENDENCY_GRAPH_JOB_CORRELATOR' must be set")
+    }
+
     @IgnoreIf({
         // There is an issue where BuildService is closed too early in Gradle 8.0,
         // resulting in empty dependency graph.
@@ -88,14 +99,38 @@ class DependencyExtractorConfigTest extends BaseExtractorTest {
         !dependencyGraphFile.exists()
     }
 
-    def "fails gracefully if configuration values not set"() {
+    def "does not generate dependency-graph on configuration failure"() {
+        given:
+        buildFile << """
+        throw new RuntimeException("Failure during configuration")
+        """
+
         when:
-        def envVars = environmentVars.asEnvironmentMap()
-        envVars.remove("GITHUB_DEPENDENCY_GRAPH_JOB_CORRELATOR")
-        executer.withEnvironmentVars(envVars)
-        def result = executer.runWithFailure()
+        def buildResult = runAndFail()
 
         then:
-        result.output.contains("> The configuration parameter 'GITHUB_DEPENDENCY_GRAPH_JOB_CORRELATOR' must be set")
+        buildResult.output.contains("Gradle Build did not complete successfully: Dependency Graph file will not be generated.")
+        !dependencyGraphFile.exists()
+    }
+
+    def "does not generate dependency-graph on task failure"() {
+        given:
+        buildFile << """
+            tasks.register("taskThatSucceeds") {
+              doLast {}
+            }
+            tasks.register("taskThatFails") {
+              doLast {
+                throw new RuntimeException("Failure in task")
+              }
+            }
+        """
+
+        when:
+        def buildResult = runAndFail("taskThatSucceeds", "taskThatFails")
+
+        then:
+        buildResult.output.contains("Gradle Build did not complete successfully: Dependency Graph file will not be generated.")
+        !dependencyGraphFile.exists()
     }
 }
diff --git a/plugin/src/main/kotlin/org/gradle/dependencygraph/AbstractDependencyExtractorPlugin.kt b/plugin/src/main/kotlin/org/gradle/dependencygraph/AbstractDependencyExtractorPlugin.kt
@@ -89,9 +89,10 @@ abstract class AbstractDependencyExtractorPlugin : Plugin<Gradle> {
                 extractorServiceProvider: Provider<out DependencyExtractor>
             ) {
                 gradle.buildFinished {
-                    extractorServiceProvider.get().close()
-                    gradle.buildOperationListenerManager
-                        .removeListener(extractorServiceProvider.get())
+                    val extractorService = extractorServiceProvider.get()
+                    extractorService.handleBuildCompletion(it.failure)
+                    extractorService.close()
+                    gradle.buildOperationListenerManager.removeListener(extractorService)
                 }
             }
         }

diff --git a/plugin/src/main/kotlin/org/gradle/dependencygraph/extractor/DependencyExtractor.kt b/plugin/src/main/kotlin/org/gradle/dependencygraph/extractor/DependencyExtractor.kt
@@ -10,7 +10,7 @@ import org.gradle.api.logging.Logging
 import org.gradle.dependencygraph.DependencyGraphRenderer
 import org.gradle.dependencygraph.model.*
 import org.gradle.dependencygraph.model.DependencyScope.*
-import org.gradle.dependencygraph.util.*
+import org.gradle.dependencygraph.util.PluginParameters
 import org.gradle.initialization.EvaluateSettingsBuildOperationType
 import org.gradle.initialization.LoadProjectsBuildOperationType
 import org.gradle.internal.exceptions.DefaultMultiCauseException
@@ -38,6 +38,8 @@ abstract class DependencyExtractor :
     private val pluginParameters = PluginParameters()
 
     private var settingsEvaluated = false
+    private var buildCompleted = false
+    private var buildFailed = false
 
     private val resolvedConfigurations = Collections.synchronizedList(mutableListOf<ResolvedConfiguration>())
 
@@ -70,6 +72,7 @@ abstract class DependencyExtractor :
     }
 
     override fun finished(buildOperation: BuildOperationDescriptor, finishEvent: OperationFinishEvent) {
+
         handleBuildOperationType<
             ResolveConfigurationDependenciesBuildOperationType.Details,
             ResolveConfigurationDependenciesBuildOperationType.Result
@@ -341,6 +344,13 @@ abstract class DependencyExtractor :
         )
     }
 
+    fun handleBuildCompletion(failure: Throwable?) {
+        buildCompleted = true
+        if (failure != null) {
+            buildFailed = true
+        }
+    }
+
     override fun close() {
         if (thrownExceptions.isNotEmpty()) {
             throw DefaultMultiCauseException(
@@ -358,6 +368,13 @@ abstract class DependencyExtractor :
             )
             return
         }
+        // Do not write an incomplete graph when build didn't complete successfully
+        if (!buildCompleted || buildFailed) {
+            LOGGER.lifecycle(
+                "Gradle Build did not complete successfully: Dependency Graph file will not be generated."
+            )
+            return
+        }
         try {
             writeDependencyGraph()
         } catch (e: RuntimeException) {

diff --git a/...n/src/main/kotlin/org/gradle/dependencygraph/extractor/DependencyExtractorBuildService.kt b/...n/src/main/kotlin/org/gradle/dependencygraph/extractor/DependencyExtractorBuildService.kt
@@ -3,6 +3,9 @@ package org.gradle.dependencygraph.extractor
 import org.gradle.api.provider.Property
 import org.gradle.api.services.BuildService
 import org.gradle.api.services.BuildServiceParameters
+import org.gradle.internal.operations.BuildOperationCategory
+import org.gradle.internal.operations.BuildOperationDescriptor
+import org.gradle.internal.operations.OperationFinishEvent
 
 abstract class DependencyExtractorBuildService :
     DependencyExtractor(),
@@ -16,4 +19,13 @@ abstract class DependencyExtractorBuildService :
     override fun getRendererClassName(): String {
         return parameters.rendererClassName.get()
     }
+
+    override fun finished(buildOperation: BuildOperationDescriptor, finishEvent: OperationFinishEvent) {
+        super.finished(buildOperation, finishEvent)
+
+        // Track build completion without 'buildFinished'
+        if (buildOperation.metadata == BuildOperationCategory.RUN_WORK) {
+            handleBuildCompletion(finishEvent.failure)
+        }
+    }
 }