-
-
Notifications
You must be signed in to change notification settings - Fork 286
Copy the protos we need into a temp folder #848
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,15 +34,16 @@ object ScalaPBWorker extends GenericWorker(new ScalaPBGenerator) { | |
} | ||
|
||
class ScalaPBGenerator extends Processor { | ||
def setupIncludedProto(includedProto: List[(Path, Path)]): Unit = { | ||
def setupIncludedProto(tmpRoot: Path, includedProto: List[(Path, Path)]): Unit = { | ||
includedProto.foreach { case (root, fullPath) => | ||
require(fullPath.toFile.exists, s"Path $fullPath does not exist, which it should as a dependency of this rule") | ||
val relativePath = root.relativize(fullPath) | ||
val relativePath = if(root == fullPath) fullPath else root.relativize(fullPath) | ||
val targetPath = tmpRoot.resolve(relativePath) | ||
|
||
relativePath.toFile.getParentFile.mkdirs | ||
Try(Files.copy(fullPath, relativePath)) match { | ||
targetPath.toFile.getParentFile.mkdirs | ||
Try(Files.copy(fullPath, targetPath)) match { | ||
case Failure(err: FileAlreadyExistsException) => | ||
Console.println(s"File already exists, skipping: ${err.getMessage}") | ||
Console.println(s"File already exists, skipping copy from $fullPath to $targetPath: ${err.getMessage}") | ||
case Failure(err) => throw err | ||
case _ => () | ||
} | ||
|
@@ -55,8 +56,10 @@ class ScalaPBGenerator extends Processor { | |
} | ||
|
||
def processRequest(args: java.util.List[String]) { | ||
val extractRequestResult = PBGenerateRequest.from(args) | ||
setupIncludedProto(extractRequestResult.includedProto) | ||
val tmpRoot = Files.createTempDirectory("proto_builder") | ||
|
||
val extractRequestResult = PBGenerateRequest.from(tmpRoot)(args) | ||
setupIncludedProto(tmpRoot, extractRequestResult.includedProto) | ||
|
||
val extraClassesClassLoader = new URLClassLoader(extractRequestResult.extraJars.map { e => | ||
val f = e.toFile | ||
|
@@ -89,10 +92,21 @@ class ScalaPBGenerator extends Processor { | |
} | ||
JarCreator.buildJar(Array(extractRequestResult.jarOutput, extractRequestResult.scalaPBOutput.toString)) | ||
} finally { | ||
deleteDir(tmpRoot) | ||
deleteDir(extractRequestResult.scalaPBOutput) | ||
} | ||
} | ||
|
||
protected def exec(protoc: Path): Seq[String] => Int = (args: Seq[String]) => | ||
new ProcessBuilder(protoc.toString +: args: _*).inheritIO().start().waitFor() | ||
protected def exec(protoc: Path): Seq[String] => Int = (args: Seq[String]) => { | ||
val tmpFile = Files.createTempFile("stdout", "log") | ||
try { | ||
val ret = new ProcessBuilder(protoc.toString +: args: _*).redirectErrorStream(true).redirectOutput(ProcessBuilder.Redirect.to(tmpFile.toFile)).start().waitFor() | ||
Try { | ||
scala.io.Source.fromFile(tmpFile.toFile).getLines.foreach{System.out.println} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this looks like debug. Can we remove? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is how we get the stdout/stderr from protoc into the bazel output stream. So can't remove. (prior to this PR any failure in protoc would result in just seeing a message that it exited with code 1 and no further info) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh yeah... I overlooked that. nit: can we do |
||
} | ||
ret | ||
} finally { | ||
tmpFile.toFile.delete | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
::: List("")
I think is a bit more efficient on lists than++
but maybe not...