Fix intermittent build failure, ensuring docker test output is still …

…usable (#4692)

This PR reverts #4254, and rewrites the changes to now only support the
use case that it was written for, but also avoid the gradle artifact
issue that it appears to have introduced. The patch works by both using
dependsOn and finalizedBy to set up two different downstream Sync tasks
that do the same thing - but only one runs on success, and only one
runs on failure. 

Fixes #4691

buildSrc/src/main/groovy/Docker.groovy

@@ -302,19 +302,8 @@ class Docker {
             // Single task with explicit inputs and outputs, to let gradle detect if it is up to date, and let docker
             // cache what it can.
 
-            // Sync outputs to the desired location
-            def syncAfterBuildAndRun = project.tasks.register("${taskName}Sync", Sync) { sync ->
-                sync.with {
-                    // run the provided closure first
-                    cfg.copyOut.execute(sync)
-
-                    // then set the from location
-                    from dockerCopyLocation
-                }
-            }
-
             // Note that if "showLogsOnSuccess" is true, we don't run this way, since that would omit logs when cached.
-            return project.tasks.register(taskName, CombinedDockerRunTask) { cacheableDockerTask ->
+            def buildAndRun = project.tasks.register("${taskName}Run", CombinedDockerRunTask) { cacheableDockerTask ->
                 cacheableDockerTask.with {
                     // mark inputs, depend on dockerfile task and input sync task
                     inputs.files(makeImage.get().outputs.files)
@@ -345,7 +334,36 @@ class Docker {
 
                     remotePath.set(cfg.containerOutPath)
                     outputDir.set(project.file(dockerCopyLocation))
-                    finalizedBy syncAfterBuildAndRun
+                }
+            }
+
+            // Handle copying failure. This is now distinct from the "actual" Sync task that depends directly
+            // on the CombinedDockerRunTask.
+            def syncAfterFail = project.tasks.register("${taskName}SyncAfterFail", Sync) { sync ->
+                sync.with {
+                    // run the provided closure first
+                    cfg.copyOut.execute(sync)
+
+                    // then set the from location
+                    from dockerCopyLocation
+
+                    onlyIf { buildAndRun.get().state.failure != null }
+                }
+            }
+            buildAndRun.configure {t ->
+                t.finalizedBy syncAfterFail
+            }
+
+            // Sync outputs to the desired location
+            return project.tasks.register(taskName, Sync) { sync ->
+                sync.with {
+                    dependsOn buildAndRun
+
+                    // run the provided closure first
+                    cfg.copyOut.execute(sync)
+
+                    // then set the from location
+                    from dockerCopyLocation
                 }
             }
         }

buildSrc/src/main/groovy/io/deephaven/tools/docker/CombinedDockerRunTask.groovy

@@ -111,7 +111,6 @@ class CombinedDockerRunTask extends AbstractDockerRemoteApiTask {
                     throw new GradleException("${failedMessage}, check logs for details")
                 }
             }
-
         } finally {
             if (containerId == null) {
                 return;