diff --git a/docs/newdocs/phases.md b/docs/newdocs/phases.md index 82cbc938..d4cb088b 100644 --- a/docs/newdocs/phases.md +++ b/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 ``` diff --git a/rules/common/private/utils.bzl b/rules/common/private/utils.bzl index b2f044d9..a2df3166 100644 --- a/rules/common/private/utils.bzl +++ b/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], diff --git a/rules/private/phases.bzl b/rules/private/phases.bzl index a5167edd..5d0d02a2 100644 --- a/rules/private/phases.bzl +++ b/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 diff --git a/rules/private/phases/phase_binary_launcher.bzl b/rules/private/phases/phase_binary_launcher.bzl index 5d969cea..256664a4 100644 --- a/rules/private/phases/phase_binary_launcher.bzl +++ b/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( diff --git a/rules/private/phases/phase_javainfo.bzl b/rules/private/phases/phase_javainfo.bzl index c9d19199..4b2c11ad 100644 --- a/rules/private/phases/phase_javainfo.bzl +++ b/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, ) diff --git a/rules/private/phases/phase_library_defaultinfo.bzl b/rules/private/phases/phase_library_defaultinfo.bzl index 3a3ca8bf..029dfdfc 100644 --- a/rules/private/phases/phase_library_defaultinfo.bzl +++ b/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), )) diff --git a/rules/private/phases/phase_outputgroupinfo.bzl b/rules/private/phases/phase_outputgroupinfo.bzl new file mode 100644 index 00000000..5a046d8e --- /dev/null +++ b/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)) diff --git a/rules/private/phases/phase_singlejar.bzl b/rules/private/phases/phase_singlejar.bzl index b12d6b50..4bcbc1cf 100644 --- a/rules/private/phases/phase_singlejar.bzl +++ b/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) diff --git a/rules/private/phases/phase_test_launcher.bzl b/rules/private/phases/phase_test_launcher.bzl index 11854c69..7af3a461 100644 --- a/rules/private/phases/phase_test_launcher.bzl +++ b/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 diff --git a/rules/private/phases/phase_zinc_depscheck.bzl b/rules/private/phases/phase_zinc_depscheck.bzl index bd429bbd..ac12a77b 100644 --- a/rules/private/phases/phase_zinc_depscheck.bzl +++ b/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 diff --git a/rules/scala.bzl b/rules/scala.bzl index c2b8d8a8..59740346 100644 --- a/rules/scala.bzl +++ b/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 diff --git a/rules/scala/private/provider.bzl b/rules/scala/private/provider.bzl index f377e308..ea82132a 100644 --- a/rules/scala/private/provider.bzl +++ b/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), ], ), diff --git a/src/main/scala/higherkindness/rules_scala/common/worker/WorkerMain.scala b/src/main/scala/higherkindness/rules_scala/common/worker/WorkerMain.scala index 8adf48ba..960e4a44 100644 --- a/src/main/scala/higherkindness/rules_scala/common/worker/WorkerMain.scala +++ b/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,14 +143,15 @@ 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" + @@ -152,9 +159,9 @@ abstract class WorkerMain[S](stdin: InputStream = System.in, stdout: PrintStream ) // 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 diff --git a/tests/dependencies/validation_action/BUILD.bazel b/tests/dependencies/validation_action/BUILD.bazel new file mode 100644 index 00000000..bc597af7 --- /dev/null +++ b/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"], +) diff --git a/tests/dependencies/validation_action/test b/tests/dependencies/validation_action/test new file mode 100755 index 00000000..24729326 --- /dev/null +++ b/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