diff --git a/src/scala/scripts/PBGenerateRequest.scala b/src/scala/scripts/PBGenerateRequest.scala index a35d03c31..5eea6e146 100644 --- a/src/scala/scripts/PBGenerateRequest.scala +++ b/src/scala/scripts/PBGenerateRequest.scala @@ -6,6 +6,30 @@ case class PBGenerateRequest(jarOutput: String, scalaPBOutput: Path, scalaPBArgs object PBGenerateRequest { + // This little function fixes a problem, where external/com_google_protobuf is not found. The com_google_protobuf + // is special in a way that it also brings-in protoc and also google well-known proto files. This, possibly, + // confuses Bazel and external/com_google_protobuf is not made available for target builds. Actual causes are unknown + // and this fixTransitiveProtoPath fixes this problem in the following way: + // (1) We have a list of all required .proto files; this is a tuple list (root -> full path), for example: + // bazel-out/k8-fastbuild/bin -> bazel-out/k8-fastbuild/bin/external/com_google_protobuf/google/protobuf/source_context.proto + // (2) Convert the full path to relative from the root: + // bazel-out/k8-fastbuild/bin -> external/com_google_protobuf/google/protobuf/source_context.proto + // (3) From all the included protos we find the first one that is located within dir we are processing -- relative + // path starts with the dir we are processing + // (4) If found -- the include folder is "orphan" and is not anchored in either host or target. To fix we prepend + // root. If not found, return original. This works as long as "external/com_google_protobuf" is available in + // target root. + def fixTransitiveProtoPath(includedProto: List[(Path, Path)]): String => String = { + val includedRelProto = includedProto.map { case (root, full) => (root.toString, root.relativize(full).toString) } + + { orig => + includedRelProto.find { case (_, rel) => rel.startsWith(orig) } match { + case Some((root, _)) => s"$root/$orig" + case None => orig + } + } + } + def from(args: java.util.List[String]): PBGenerateRequest = { val jarOutput = args.get(0) val protoFiles = args.get(4).split(':') @@ -23,17 +47,18 @@ object PBGenerateRequest { case s if s.charAt(0) == '-' => Some(s.tail) //drop padding character case other => sys.error(s"expected a padding character of - (dash), but found: $other") } - val transitiveProtoPaths = (args.get(3) match { + + val transitiveProtoPaths: List[String] = (args.get(3) match { case "-" => Nil case s if s.charAt(0) == '-' => s.tail.split(':').toList //drop padding character case other => sys.error(s"expected a padding character of - (dash), but found: $other") - }) ++ List(".") + }).map(fixTransitiveProtoPath(includedProto)) ++ List(".") val tmp = Paths.get(Option(System.getProperty("java.io.tmpdir")).getOrElse("/tmp")) val scalaPBOutput = Files.createTempDirectory(tmp, "bazelscalapb") val flagPrefix = flagOpt.fold("")(_ + ":") - val namedGenerators = args.get(6).drop(1).split(',').filter(_.nonEmpty).map { e => + val namedGenerators = args.get(6).drop(1).split(',').filter(_.nonEmpty).map { e => val kv = e.split('=') (kv(0), kv(1)) } diff --git a/src/scala/scripts/ScalaPBGenerator.scala b/src/scala/scripts/ScalaPBGenerator.scala index 5fdaa8937..9ed457d09 100644 --- a/src/scala/scripts/ScalaPBGenerator.scala +++ b/src/scala/scripts/ScalaPBGenerator.scala @@ -34,20 +34,6 @@ object ScalaPBWorker extends GenericWorker(new ScalaPBGenerator) { } class ScalaPBGenerator extends Processor { - def setupIncludedProto(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) - - relativePath.toFile.getParentFile.mkdirs - Try(Files.copy(fullPath, relativePath)) match { - case Failure(err: FileAlreadyExistsException) => - Console.println(s"File already exists, skipping: ${err.getMessage}") - case Failure(err) => throw err - case _ => () - } - } - } def deleteDir(path: Path): Unit = try DeleteRecursively.run(path) catch { @@ -56,8 +42,6 @@ class ScalaPBGenerator extends Processor { def processRequest(args: java.util.List[String]) { val extractRequestResult = PBGenerateRequest.from(args) - setupIncludedProto(extractRequestResult.includedProto) - val extraClassesClassLoader = new URLClassLoader(extractRequestResult.extraJars.map { e => val f = e.toFile require(f.exists, s"Expected file for classpath loading $f to exist") diff --git a/test/src/main/scala/scalarules/test/scripts/BUILD b/test/src/main/scala/scalarules/test/scripts/BUILD new file mode 100644 index 000000000..ce0778ee3 --- /dev/null +++ b/test/src/main/scala/scalarules/test/scripts/BUILD @@ -0,0 +1,10 @@ +load("//scala:scala.bzl", "scala_library", "scala_specs2_junit_test") +load("//scala:scala_import.bzl", "scala_import") + +scala_specs2_junit_test( + name = "pb_generate_request_test", + size = "small", + srcs = ["PBGenerateRequestTest.scala"], + suffixes = ["Test"], + deps = ["//src/scala/scripts:scala_proto_request_extractor"], +) diff --git a/test/src/main/scala/scalarules/test/scripts/PBGenerateRequestTest.scala b/test/src/main/scala/scalarules/test/scripts/PBGenerateRequestTest.scala new file mode 100644 index 000000000..87a37e569 --- /dev/null +++ b/test/src/main/scala/scalarules/test/scripts/PBGenerateRequestTest.scala @@ -0,0 +1,21 @@ +package scalarules.test.scripts + +import java.nio.file.Paths +import scripts.PBGenerateRequest +import org.specs2.mutable.SpecWithJUnit + +class PBGenerateRequestTest extends SpecWithJUnit { + "fixTransitiveProtoPath should fix path when included proto is available, ignore otherwise" >> { + val includedProtos = List(Paths.get("a/b/c") -> Paths.get("a/b/c/d/e/f.proto")) + Seq("d/e", "x/y/z").map(PBGenerateRequest.fixTransitiveProtoPath(includedProtos)) must + beEqualTo(Seq("a/b/c/d/e", "x/y/z")) + } + + "actual case observed in builds" >> { + val includedProtos = List( + Paths.get("bazel-out/k8-fastbuild/bin") -> + Paths.get("bazel-out/k8-fastbuild/bin/external/com_google_protobuf/google/protobuf/source_context.proto")) + Seq("external/com_google_protobuf").map(PBGenerateRequest.fixTransitiveProtoPath(includedProtos)) must + beEqualTo(Seq("bazel-out/k8-fastbuild/bin/external/com_google_protobuf")) + } +} \ No newline at end of file