Add real multiplex support #481
Conversation
Add new command structure to used multiplxing properly. Fix tests to work using bootstrap.
Add kt_bootstrap_binary Make sure PersistentWorker reports captured output
Add exporting a PrintStream to WorkerContext.Logging to integrate with KotlinBuilder Split mergejdeps and build into separate binaries. Remove BazelWorker and thank it for it's service.
Run buildifier
There was a problem hiding this comment.
Worth including a link to multiplexed workers in the PR description: https://docs.bazel.build/versions/master/multiplex-worker.html
Also, should we have an option to disable/enable them? I think this can be done simply my making "supports_multiplexed_worker" attribute conditional (on some flag in the tool chain?).... I know they had some attempts at fixing https://docs.bazel.build/versions/master/multiplex-worker.html#warning-about-rare-bug not sure if its still a thing or not but caution maybe?
src/main/kotlin/io/bazel/kotlin/builder/KotlinBuilderComponent.java
Outdated
Show resolved
Hide resolved
src/main/kotlin/io/bazel/kotlin/builder/KotlinBuilderComponent.java
Outdated
Show resolved
Hide resolved
| @@ -28,6 +28,7 @@ java_library( | |||
| "//src/main/protobuf:deps_java_proto", | |||
| "//src/main/protobuf:kotlin_model_java_proto", | |||
| "//src/main/protobuf:worker_protocol_java_proto", | |||
| "//src/main/kotlin/io/bazel/worker", | |||
| import java.io.InputStream | ||
| import java.io.PrintStream | ||
|
|
||
| class IO( |
There was a problem hiding this comment.
This is what I should have called PrintStreams in that other PR. Doh.
|
|
||
| @ExperimentalCoroutinesApi | ||
| override fun start(execute: Work) = WorkerContext.run { | ||
| captureIO().use { io -> |
There was a problem hiding this comment.
Ok, I really like this flow.
| class WorkerContext private constructor( | ||
| private val name: String = Companion::class.java.canonicalName, | ||
| private val verbose: Granularity = INFO | ||
| ) : Closeable, ScopeLogging by ContextLogger(name, verbose.level, null) { |
There was a problem hiding this comment.
Are you doing an anonymous delegate? That's... huh. I didn't realize you could do that. I'm used to delegating to a constructor parameter or property object, but not an anonymous instance like this.
| } | ||
|
|
||
| override fun close() { | ||
| Files.walk(dir).sorted(Comparator.reverseOrder()).forEach(Files::delete) |
There was a problem hiding this comment.
I presume you're just going to accept if it fails here? Best effort?
There was a problem hiding this comment.
Pretty much -- it's not really actionable. In many respects, this shouldn't be done during in the invocation.
There was a problem hiding this comment.
Ah, nm. Catch and log.
Multiplex is controlled on the command line, so enabling it means you know what you are doing. |
Replace worker strategy with a more general one relying on coroutines for multiplexing.
Fixes #453 as well.