Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sends verbosity from the worker protocol to the worker and enable Java toolchain multiplex sandboxing #55

Merged
merged 3 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion rules/scala_proto/private/ScalaProtoWorker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ object ScalaProtoWorker extends WorkerMain[Unit] {

override def init(args: Option[Array[String]]): Unit = ()

protected def work(ctx: Unit, args: Array[String], out: PrintStream, workDir: Path): Unit = {
protected def work(ctx: Unit, args: Array[String], out: PrintStream, workDir: Path, verbosity: Int): Unit = {
val workRequest = ScalaProtoRequest(workDir, ArgsUtil.parseArgsOrFailSafe(args, argParser, out))
InterruptUtil.throwIfInterrupted()

Expand Down
2 changes: 1 addition & 1 deletion rules/scalafmt/scalafmt/ScalafmtRunner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ object ScalafmtRunner extends WorkerMain[Unit] {

protected[this] def init(args: Option[Array[String]]): Unit = {}

protected[this] def work(worker: Unit, args: Array[String], out: PrintStream, workDir: Path): Unit = {
protected[this] def work(worker: Unit, args: Array[String], out: PrintStream, workDir: Path, verbosity: Int): Unit = {
val workRequest = ScalafmtRequest(workDir, ArgsUtil.parseArgsOrFailSafe(args, argParser, out))
InterruptUtil.throwIfInterrupted()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ abstract class WorkerMain[S](stdin: InputStream = System.in, stdout: PrintStream

protected[this] def init(args: Option[Array[String]]): S

protected[this] def work(ctx: S, args: Array[String], out: PrintStream, workDir: Path): Unit
protected[this] def work(ctx: S, args: Array[String], out: PrintStream, workDir: Path, verbosity: Int): Unit

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're getting the arguments and verbosity directly from the work request, should we just pass that to the worker directly instead of copying so many of its fields?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we should because this does create a nice interface all the workers must adhere to. It does sometimes mean updates when the protocol changes. Good news is that tends to happen rather infrequently.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I can see how this forces worker implementers to think about certain things. Nevermind, then.


protected[this] var isWorker = false

Expand Down Expand Up @@ -101,6 +101,7 @@ abstract class WorkerMain[S](stdin: InputStream = System.in, stdout: PrintStream
} else {
val args = request.getArgumentsList.toArray(Array.empty[String])
val sandboxDir = Path.of(request.getSandboxDir())
val verbosity = request.getVerbosity()
System.err.println(s"WorkRequest $requestId received with args: ${request.getArgumentsList}")

// We go through this hullabaloo with output streams being defined out here, so we can
Expand All @@ -114,7 +115,7 @@ abstract class WorkerMain[S](stdin: InputStream = System.in, stdout: PrintStream
outStream = new ByteArrayOutputStream
out = new PrintStream(outStream)
try {
work(ctx, args, out, sandboxDir)
work(ctx, args, out, sandboxDir, verbosity)
0
} catch {
case e @ AnnexWorkerError(code, _, _) =>
Expand Down Expand Up @@ -210,6 +211,7 @@ abstract class WorkerMain[S](stdin: InputStream = System.in, stdout: PrintStream
args.toArray,
out,
workDir = Path.of(""),
verbosity = 0,
)
} catch {
// This error means the work function encountered an error that we want to not be caught
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ import java.nio.file.Path

object BloopRunner extends WorkerMain[Unit] {
override def init(args: Option[Array[String]]): Unit = ()
override def work(ctx: Unit, args: Array[String], out: PrintStream, workDir: Path): Unit = Bloop
override def work(ctx: Unit, args: Array[String], out: PrintStream, workDir: Path, verbosity: Int): Unit = Bloop
}
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ object DepsRunner extends WorkerMain[Unit] {

override def init(args: Option[Array[String]]): Unit = ()

override def work(ctx: Unit, args: Array[String], out: PrintStream, workDir: Path): Unit = {
override def work(ctx: Unit, args: Array[String], out: PrintStream, workDir: Path, verbosity: Int): Unit = {
val workRequest = DepsRunnerRequest(workDir, ArgsUtil.parseArgsOrFailSafe(args, argParser, out))
InterruptUtil.throwIfInterrupted()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ object JacocoInstrumenter extends WorkerMain[Unit] {

override def init(args: Option[Array[String]]): Unit = ()

override def work(ctx: Unit, args: Array[String], out: PrintStream, workDir: Path): Unit = {
override def work(ctx: Unit, args: Array[String], out: PrintStream, workDir: Path, verbosity: Int): Unit = {
val workRequest = JacocoRequest(workDir, ArgsUtil.parseArgsOrFailSafe(args, argParser, out))

val jacoco = new Instrumenter(new OfflineInstrumentationAccessGenerator)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ object ZincRunner extends WorkerMain[ZincRunnerWorkerConfig] {
args: Array[String],
out: PrintStream,
workDir: Path,
verbosity: Int,
): Unit = {
val workRequest = CommonArguments(ArgsUtil.parseArgsOrFailSafe(args, parser, out), workDir)
InterruptUtil.throwIfInterrupted()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ object DocRunner extends WorkerMain[Unit] {

override def init(args: Option[Array[String]]): Unit = ()

override def work(ctx: Unit, args: Array[String], out: PrintStream, workDir: Path): Unit = {
override def work(ctx: Unit, args: Array[String], out: PrintStream, workDir: Path, verbosity: Int): Unit = {
val workRequest = DocRequest(workDir, ArgsUtil.parseArgsOrFailSafe(args, argParser, out))
InterruptUtil.throwIfInterrupted()

Expand Down
2 changes: 1 addition & 1 deletion tests/cancellation/RunnerForCancelSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class RunnerForCancelSpec(stdin: InputStream, stdout: PrintStream)

override def init(args: Option[Array[String]]): Unit = ()

override def work(ctx: Unit, args: Array[String], out: PrintStream, workDir: Path): Unit = {
override def work(ctx: Unit, args: Array[String], out: PrintStream, workDir: Path, verbosity: Int): Unit = {
var interrupted = false
var i = 0

Expand Down
2 changes: 1 addition & 1 deletion tests/worker-error/RunnerThatThrowsError.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class RunnerThatThrowsError(stdin: InputStream, stdout: PrintStream)

override def init(args: Option[Array[String]]): Unit = ()

override def work(ctx: Unit, args: Array[String], out: PrintStream, workDir: Path): Unit = {
override def work(ctx: Unit, args: Array[String], out: PrintStream, workDir: Path, verbosity: Int): Unit = {
throw new Error()
}
}
2 changes: 1 addition & 1 deletion tests/worker-error/RunnerThatThrowsException.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class RunnerThatThrowsException(stdin: InputStream, stdout: PrintStream)

override def init(args: Option[Array[String]]): Unit = ()

override def work(ctx: Unit, args: Array[String], out: PrintStream, workDir: Path): Unit = {
override def work(ctx: Unit, args: Array[String], out: PrintStream, workDir: Path, verbosity: Int): Unit = {
throw new Exception()
}
}
2 changes: 1 addition & 1 deletion tests/worker-error/RunnerThatThrowsFatalError.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class RunnerThatThrowsFatalError(stdin: InputStream, stdout: PrintStream)

override def init(args: Option[Array[String]]): Unit = ()

override def work(ctx: Unit, args: Array[String], out: PrintStream, workDir: Path): Unit = {
override def work(ctx: Unit, args: Array[String], out: PrintStream, workDir: Path, verbosity: Int): Unit = {
throw new OutOfMemoryError()
}
}
20 changes: 20 additions & 0 deletions tests/worker-verbosity/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
load("@rules_scala_annex//rules:scala.bzl", "scala_binary")
load("verbosity_spec_worker_run.bzl", "verbosity_spec_worker_run")

scala_binary(
name = "verbosity-spec-worker",
srcs = [
"RunnerThatPrintsVerbosity.scala",
],
scala = "//scala:2_13",
tags = ["manual"],
deps = [
"@rules_scala_annex//src/main/scala/higherkindness/rules_scala/common/sandbox",
"@rules_scala_annex//src/main/scala/higherkindness/rules_scala/common/worker",
],
)

verbosity_spec_worker_run(
name = "verbosity-spec-target",
verbosity_spec_worker = ":verbosity-spec-worker",
)
15 changes: 15 additions & 0 deletions tests/worker-verbosity/RunnerThatPrintsVerbosity.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package anx.cancellation

import higherkindness.rules_scala.common.worker.WorkerMain
import higherkindness.rules_scala.common.sandbox.SandboxUtil

import java.io.{InputStream, PrintStream}
import java.nio.file.{Files, Path, Paths}

object RunnerThatPrintsVerbosity extends WorkerMain[Unit] {
override def init(args: Option[Array[String]]): Unit = ()
override def work(ctx: Unit, args: Array[String], out: PrintStream, workDir: Path, verbosity: Int): Unit = {
out.println(s"Verbosity: ${verbosity}")
Files.createFile(SandboxUtil.getSandboxPath(workDir, Paths.get(args(0))))
}
}
12 changes: 12 additions & 0 deletions tests/worker-verbosity/test
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#!/bin/bash -e
. "$(dirname "$0")"/../common.sh

# We use modify_execution_info, nouse_action_cache, and bazel shutdown here
# in order to prevent the disk cache, skyframe cache, and persistent action cache
# from being used for the verbosity spec worker actions and thus getting the
# verbosity we want getting printed. The alternative is to bazel clean, which
# takes much longer.
bazel shutdown
bazel build --modify_execution_info="VerbositySpecWorkerRun=+no-cache" --nouse_action_cache :verbosity-spec-target |& grep -q "Verbosity: 0"
bazel shutdown
bazel build --modify_execution_info="VerbositySpecWorkerRun=+no-cache" --nouse_action_cache --worker_verbose :verbosity-spec-target |& grep -q "Verbosity: 10"
39 changes: 39 additions & 0 deletions tests/worker-verbosity/verbosity_spec_worker_run.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
def _impl(ctx):
foo_file = ctx.actions.declare_file("foo.txt")
outputs = [foo_file]

args = ctx.actions.args()
args.add(foo_file)
args.set_param_file_format("multiline")
args.use_param_file("@%s", use_always = True)

ctx.actions.run(
outputs = outputs,
arguments = [args],
mnemonic = "VerbositySpecWorkerRun",
execution_requirements = {
"supports-multiplex-workers": "1",
"supports-workers": "1",
"supports-multiplex-sandboxing": "1",
"supports-worker-cancellation": "1",
},
progress_message = "Running verbosity spec worker %{label}",
executable = ctx.executable.verbosity_spec_worker,
)

return [
DefaultInfo(files = depset(outputs)),
]

verbosity_spec_worker_run = rule(
implementation = _impl,
doc = "Runs a worker that prints the verbosity level it received from the work request",
attrs = {
"verbosity_spec_worker": attr.label(
executable = True,
cfg = "host",
allow_files = True,
default = Label(":verbosity-spec-worker"),
),
},
)
Loading