Do dependency checking via a validation action

docs/newdocs/phases.md

@@ -15,6 +15,7 @@ def _scala_binary_implementation(ctx):
         ("ijinfo", _phase_ijinfo),
         ("binary_deployjar", _phase_binary_deployjar),
         ("binary_launcher", _phase_binary_launcher),
+        ("outputgroupinfo", _phase_outputgroupinfo),
         ("coda", _phase_coda),
     ]).coda
 ```

rules/common/private/utils.bzl

@@ -162,7 +162,6 @@ def action_singlejar(
         ctx,
         inputs,
         output,
-        phantom_inputs = depset(),
         main_class = None,
         progress_message = None,
         resources = {},
@@ -174,8 +173,6 @@ def action_singlejar(
 
     if type(inputs) == "list":
         inputs = depset(inputs)
-    if type(phantom_inputs) == "list":
-        phantom_inputs = depset(phantom_inputs)
 
     args = ctx.actions.args()
     args.add("--exclude_build_data")
@@ -190,7 +187,7 @@ def action_singlejar(
         args.set_param_file_format("multiline")
         args.use_param_file("@%s", use_always = True)
 
-    all_inputs = depset(resources.values(), transitive = [inputs, phantom_inputs])
+    all_inputs = depset(resources.values(), transitive = [inputs])
 
     ctx.actions.run(
         arguments = [args],

rules/private/phases.bzl

@@ -9,6 +9,7 @@ load(":phases/phase_ijinfo.bzl", _phase_ijinfo = "phase_ijinfo")
 load(":phases/phase_javainfo.bzl", _phase_javainfo = "phase_javainfo")
 load(":phases/phase_library_defaultinfo.bzl", _phase_library_defaultinfo = "phase_library_defaultinfo")
 load(":phases/phase_noop.bzl", _phase_noop = "phase_noop")
+load(":phases/phase_outputgroupinfo.bzl", _phase_outputgroupinfo = "phase_outputgroupinfo")
 load(":phases/phase_resources.bzl", _phase_resources = "phase_resources")
 load(":phases/phase_scalafmt_nondefault_outputs.bzl", _phase_scalafmt_nondefault_outputs = "phase_scalafmt_nondefault_outputs")
 load(":phases/phase_semanticdb.bzl", _phase_semanticdb = "phase_semanticdb")
@@ -41,6 +42,8 @@ phase_library_defaultinfo = _phase_library_defaultinfo
 
 phase_noop = _phase_noop
 
+phase_outputgroupinfo = _phase_outputgroupinfo
+
 phase_resources = _phase_resources
 
 phase_scalafmt_nondefault_outputs = _phase_scalafmt_nondefault_outputs

rules/private/phases/phase_binary_launcher.bzl

@@ -30,7 +30,7 @@ def phase_binary_launcher(ctx, g):
 
     g.out.providers.append(DefaultInfo(
         executable = ctx.outputs.bin,
-        files = depset([ctx.outputs.bin, ctx.outputs.jar]),
+        files = depset([ctx.outputs.bin, ctx.outputs.jar] + g.semanticdb.outputs),
         runfiles = ctx.runfiles(
             files = inputs + files,
             transitive_files = depset(

rules/private/phases/phase_javainfo.bzl

@@ -13,7 +13,6 @@ load(
 # PHASE: javainfo
 #
 # Builds up the JavaInfo provider. And the ScalaInfo, while we're at it.
-# And DefaultInfo.
 #
 
 def phase_javainfo(ctx, g):
@@ -61,18 +60,10 @@ def phase_javainfo(ctx, g):
         scala_configuration = g.init.scala_configuration,
     )
 
-    output_group_info = OutputGroupInfo(
-        **g.out.output_groups
-    )
-
-    g.out.providers.extend([
-        output_group_info,
-        java_info,
-        scala_info,
-    ])
+    g.out.providers.append(java_info)
+    g.out.providers.append(scala_info)
 
     return struct(
         java_info = java_info,
-        output_group_info = output_group_info,
         scala_info = scala_info,
     )

rules/private/phases/phase_library_defaultinfo.bzl

@@ -6,5 +6,5 @@
 
 def phase_library_defaultinfo(ctx, g):
     g.out.providers.append(DefaultInfo(
-        files = depset([ctx.outputs.jar]),
+        files = depset([ctx.outputs.jar] + g.semanticdb.outputs),
     ))

rules/private/phases/phase_outputgroupinfo.bzl

@@ -0,0 +1,8 @@
+#
+# PHASE: outputgroupinfo
+#
+# Generates the `OutputGroupInfo` provider.
+#
+
+def phase_outputgroupinfo(ctx, g):
+    g.out.providers.append(OutputGroupInfo(**g.out.output_groups))

rules/private/phases/phase_singlejar.bzl

@@ -21,12 +21,9 @@ def phase_singlejar(ctx, g):
     # cause the build to fail, cleanly, if any declared outputs are
     # missing from previous phases.
     inputs = [f for f in ctx.files.resource_jars if f.extension.lower() in ["jar"]]
-    phantom_inputs = []
     for v in [getattr(g, k) for k in dir(g) if k not in ["to_json", "to_proto"]]:
         if hasattr(v, "jar"):
             jar = getattr(v, "jar")
             inputs.append(jar)
-        if hasattr(v, "outputs"):
-            phantom_inputs.extend(getattr(v, "outputs"))
 
-    _action_singlejar(ctx, inputs, ctx.outputs.jar, phantom_inputs)
+    _action_singlejar(ctx, inputs, ctx.outputs.jar)

rules/private/phases/phase_test_launcher.bzl

@@ -2,12 +2,7 @@ load(
     "@rules_scala_annex//rules/private:coverage_replacements_provider.bzl",
     _coverage_replacements_provider = "coverage_replacements_provider",
 )
-load(
-    "//rules/common:private/utils.bzl",
-    _action_singlejar = "action_singlejar",
-    _collect = "collect",
-    _write_launcher = "write_launcher",
-)
+load("//rules/common:private/utils.bzl", _collect = "collect", _write_launcher = "write_launcher")
 
 #
 # PHASE: test_launcher

rules/private/phases/phase_zinc_depscheck.bzl

@@ -20,11 +20,10 @@ def phase_zinc_depscheck(ctx, g):
         return
 
     deps_configuration = ctx.attr.scala[_DepsConfiguration]
-
-    deps_checks = {}
     labeled_jar_groups = depset(transitive = [dep[_LabeledJars].values for dep in ctx.attr.deps])
-
     worker_inputs, _, worker_input_manifests = ctx.resolve_command(tools = [deps_configuration.worker])
+    outputs = []
+
     for name in ("direct", "used"):
         deps_check = ctx.actions.declare_file("{}/depscheck_{}.success".format(ctx.label.name, name))
         deps_args = ctx.actions.args()
@@ -61,21 +60,16 @@ def phase_zinc_depscheck(ctx, g):
             ),
             arguments = [deps_args],
         )
-        deps_checks[name] = deps_check
 
-    outputs = []
-    if deps_configuration.direct == "error":
-        outputs.append(deps_checks["direct"])
-    if deps_configuration.used == "error":
-        outputs.append(deps_checks["used"])
+        if getattr(deps_configuration, name) == "error":
+            outputs.append(deps_check)
 
-    g.out.output_groups["depscheck"] = depset(outputs)
+    if "_validation" in g.out.output_groups:
+        validation_transitive = [g.out.output_groups["_validation"]]
+    else:
+        validation_transitive = None
 
-    return struct(
-        checks = deps_checks,
-        outputs = outputs,
-        toolchain = deps_configuration,
-    )
+    g.out.output_groups["_validation"] = depset(outputs, transitive = validation_transitive)
 
 # If you use avoid using map_each, then labels are converted to their apparent repo name rather than
 # their canonical repo name. The apparent repo name is the human readable one that we want for use

rules/scala.bzl

@@ -29,7 +29,9 @@ load(
     _phase_javainfo = "phase_javainfo",
     _phase_library_defaultinfo = "phase_library_defaultinfo",
     _phase_noop = "phase_noop",
+    _phase_outputgroupinfo = "phase_outputgroupinfo",
     _phase_resources = "phase_resources",
+    _phase_semanticdb = "phase_semanticdb",
     _phase_singlejar = "phase_singlejar",
     _phase_test_launcher = "phase_test_launcher",
     _run_phases = "run_phases",
@@ -211,11 +213,13 @@ def _scala_library_implementation(ctx):
         ("resources", _phase_resources),
         ("classpaths", _phase_classpaths),
         ("javainfo", _phase_javainfo),
+        ("semanticdb", _phase_semanticdb),
         ("compile", _phase_noop),
         ("singlejar", _phase_singlejar),
         ("coverage", _phase_coverage_jacoco),
         ("ijinfo", _phase_ijinfo),
         ("library_defaultinfo", _phase_library_defaultinfo),
+        ("outputgroupinfo", _phase_outputgroupinfo),
         ("coda", _phase_coda),
     ]).coda
 
@@ -224,12 +228,14 @@ def _scala_binary_implementation(ctx):
         ("resources", _phase_resources),
         ("classpaths", _phase_classpaths),
         ("javainfo", _phase_javainfo),
+        ("semanticdb", _phase_semanticdb),
         ("compile", _phase_noop),
         ("singlejar", _phase_singlejar),
         ("coverage", _phase_coverage_jacoco),
         ("ijinfo", _phase_ijinfo),
         ("binary_deployjar", _phase_binary_deployjar),
         ("binary_launcher", _phase_binary_launcher),
+        ("outputgroupinfo", _phase_outputgroupinfo),
         ("coda", _phase_coda),
     ]).coda
 
@@ -238,11 +244,13 @@ def _scala_test_implementation(ctx):
         ("resources", _phase_resources),
         ("classpaths", _phase_classpaths),
         ("javainfo", _phase_javainfo),
+        ("semanticdb", _phase_semanticdb),
         ("compile", _phase_noop),
         ("singlejar", _phase_singlejar),
         ("coverage", _phase_coverage_jacoco),
         ("ijinfo", _phase_ijinfo),
         ("test_launcher", _phase_test_launcher),
+        ("outputgroupinfo", _phase_outputgroupinfo),
         ("coda", _phase_coda),
     ]).coda
 

rules/scala/private/provider.bzl

@@ -9,7 +9,6 @@ load(
 load(
     "//rules/private:phases.bzl",
     _phase_bootstrap_compile = "phase_bootstrap_compile",
-    _phase_semanticdb = "phase_semanticdb",
     _phase_zinc_compile = "phase_zinc_compile",
     _phase_zinc_depscheck = "phase_zinc_depscheck",
 )
@@ -60,7 +59,6 @@ def configure_zinc_scala_implementation(ctx):
         _ScalaRulePhase(
             phases = [
                 ("=", "compile", "compile", _phase_zinc_compile),
-                ("-", "compile", "semanticdb", _phase_semanticdb),
                 ("+", "compile", "depscheck", _phase_zinc_depscheck),
             ],
         ),

src/main/scala/higherkindness/rules_scala/common/worker/WorkerMain.scala

@@ -117,12 +117,18 @@ abstract class WorkerMain[S](stdin: InputStream = System.in, stdout: PrintStream
             // close them after the async work in the Future is all done.
             // If we do something synchronous with Using, then there's a race condition where the
             // streams can get closed before the Future is completed.
-            var outStream: ByteArrayOutputStream = null
-            var out: PrintStream = null
+            var maybeOutStream: Option[ByteArrayOutputStream] = None
+            var maybeOut: Option[PrintStream] = None
+
+            def flushOut(): Unit = {
+              maybeOut.map(_.flush())
+            }
 
             val workTask = CancellableTask {
-              outStream = new ByteArrayOutputStream
-              out = new PrintStream(outStream)
+              val outStream = new ByteArrayOutputStream()
+              val out = new PrintStream(outStream)
+              maybeOutStream = Some(outStream)
+              maybeOut = Some(out)
               try {
                 work(ctx, args, out, sandboxDir, verbosity)
                 0
@@ -137,24 +143,25 @@ abstract class WorkerMain[S](stdin: InputStream = System.in, stdout: PrintStream
               .andThen {
                 // Work task succeeded or failed in an expected way
                 case Success(code) =>
-                  out.flush()
-                  writeResponse(requestId, Some(outStream), Some(code))
+                  flushOut()
+                  writeResponse(requestId, maybeOutStream, Some(code))
                   logVerbose(s"WorkResponse $requestId sent with code $code")
 
                 case Failure(e: ExecutionException) =>
                   e.getCause() match {
                     // Task successfully cancelled
                     case cancelError: InterruptedException =>
+                      flushOut()
                       writeResponse(requestId, None, None, wasCancelled = true)
                       logVerbose(
                         s"Cancellation WorkResponse sent for request id: $requestId in response to an" +
                           " InterruptedException",
                       )
                     // Work task threw a non-fatal error
                     case e =>
-                      e.printStackTrace(out)
-                      out.flush()
-                      writeResponse(requestId, Some(outStream), Some(-1))
+                      maybeOut.map(e.printStackTrace(_))
+                      flushOut()
+                      writeResponse(requestId, maybeOutStream, Some(-1))
                       logVerbose(
                         "Encountered an uncaught exception that was wrapped in an ExecutionException while" +
                           s" proccessing the Future for WorkRequest $requestId. This usually means a non-fatal" +
@@ -165,6 +172,7 @@ abstract class WorkerMain[S](stdin: InputStream = System.in, stdout: PrintStream
 
                 // Task successfully cancelled
                 case Failure(e: CancellationException) =>
+                  flushOut()
                   writeResponse(requestId, None, None, wasCancelled = true)
                   logVerbose(
                     s"Cancellation WorkResponse sent for request id: $requestId in response to a" +
@@ -173,15 +181,15 @@ abstract class WorkerMain[S](stdin: InputStream = System.in, stdout: PrintStream
 
                 // Work task threw an uncaught exception
                 case Failure(e) =>
-                  e.printStackTrace(out)
-                  out.flush()
-                  writeResponse(requestId, Some(outStream), Some(-1))
+                  maybeOut.map(e.printStackTrace(_))
+                  flushOut()
+                  writeResponse(requestId, maybeOutStream, Some(-1))
                   logVerbose(s"Uncaught exception in Future while proccessing WorkRequest $requestId:")
                   e.printStackTrace(System.err)
               }(scala.concurrent.ExecutionContext.global)
               .andThen { case _ =>
-                out.close()
-                outStream.close()
+                maybeOut.map(_.close())
+                maybeOutStream.map(_.close())
               }(scala.concurrent.ExecutionContext.global)
 
             // putIfAbsent will return a non-null value if there was already a value in the map

tests/dependencies/validation_action/BUILD.bazel

@@ -0,0 +1,7 @@
+load("@rules_scala_annex//rules:scala.bzl", "scala_library")
+
+scala_library(
+    name = "validation_action",
+    scala = "//scala:2_13",
+    tags = ["manual"],
+)

tests/dependencies/validation_action/test

@@ -0,0 +1,24 @@
+#!/bin/bash -e
+. "$(dirname "$0")"/../../common.sh
+
+scalacheckdeps_outputs="$(
+    bazel aquery 'mnemonic("^ScalaCheckDeps$", //dependencies/validation_action)' |
+        grep -oP '(?<=^  Outputs: \[).*(?=\]$)' |
+        sed 's/, /\n/g' |
+        sort -u
+)"
+
+action_inputs="$(
+    bazel aquery '//dependencies/validation_action' |
+        grep -oP '(?<=^  Inputs: \[).*(?=\]$)' |
+        sed 's/, /\n/g' |
+        sort -u
+)"
+
+common_files="$(comm -12 <(echo "$scalacheckdeps_outputs") <(echo "$action_inputs"))"
+
+if [ -n "$common_files" ]; then
+    echo "Found some ScalaCheckDeps outputs that are the inputs to other actons:"
+    echo "$common_files"
+    exit 1
+fi