scalapb_proto_library is broken when it depends on external targets

[beala-stripe]

I've added a repro of the issue in PR #685. The relevant bits are reproduced below:

test/proto/BUILD

proto_library(
    name = "test_external_dep_proto",
    srcs = ["test_external_dep.proto"],
    visibility = ["//visibility:public"],
    deps = [
        "@com_google_protobuf//:wrappers_proto",
    ],
)

scalapb_proto_library(
    name = "test_external_dep",
    visibility = ["//visibility:public"],
    deps = [":test_external_dep_proto"],
)

test/proto/test_external_dep.proto

syntax = "proto3";

import "google/protobuf/wrappers.proto";

message TestMessage {
    string message = 1;
}

bazel build test/proto:test_external_dep fails with the following output:

external/com_google_protobuf: warning: directory does not exist.
google/protobuf/wrappers.proto:53:10: "google.protobuf.DoubleValue.value" is already defined in file "bazel-out/k8-fastbuild/genfiles/external/com_google_protobuf/google/protobuf/wrappers.proto".
google/protobuf/wrappers.proto:51:9: "google.protobuf.DoubleValue" is already defined in file "bazel-out/k8-fastbuild/genfiles/external/com_google_protobuf/google/protobuf/wrappers.proto".
google/protobuf/wrappers.proto:61:9: "google.protobuf.FloatValue.value" is already defined in file "bazel-out/k8-fastbuild/genfiles/external/com_google_protobuf/google/protobuf/wrappers.proto".
google/protobuf/wrappers.proto:59:9: "google.protobuf.FloatValue" is already defined in file "bazel-out/k8-fastbuild/genfiles/external/com_google_protobuf/google/protobuf/wrappers.proto".
google/protobuf/wrappers.proto:69:9: "google.protobuf.Int64Value.value" is already defined in file "bazel-out/k8-fastbuild/genfiles/external/com_google_protobuf/google/protobuf/wrappers.proto".
google/protobuf/wrappers.proto:67:9: "google.protobuf.Int64Value" is already defined in file "bazel-out/k8-fastbuild/genfiles/external/com_google_protobuf/google/protobuf/wrappers.proto".
google/protobuf/wrappers.proto:77:10: "google.protobuf.UInt64Value.value" is already defined in file "bazel-out/k8-fastbuild/genfiles/external/com_google_protobuf/google/protobuf/wrappers.proto".
google/protobuf/wrappers.proto:75:9: "google.protobuf.UInt64Value" is already defined in file "bazel-out/k8-fastbuild/genfiles/external/com_google_protobuf/google/protobuf/wrappers.proto".
google/protobuf/wrappers.proto:85:9: "google.protobuf.Int32Value.value" is already defined in file "bazel-out/k8-fastbuild/genfiles/external/com_google_protobuf/google/protobuf/wrappers.proto".
google/protobuf/wrappers.proto:83:9: "google.protobuf.Int32Value" is already defined in file "bazel-out/k8-fastbuild/genfiles/external/com_google_protobuf/google/protobuf/wrappers.proto".
google/protobuf/wrappers.proto:93:10: "google.protobuf.UInt32Value.value" is already defined in file "bazel-out/k8-fastbuild/genfiles/external/com_google_protobuf/google/protobuf/wrappers.proto".
google/protobuf/wrappers.proto:91:9: "google.protobuf.UInt32Value" is already defined in file "bazel-out/k8-fastbuild/genfiles/external/com_google_protobuf/google/protobuf/wrappers.proto".
google/protobuf/wrappers.proto:101:8: "google.protobuf.BoolValue.value" is already defined in file "bazel-out/k8-fastbuild/genfiles/external/com_google_protobuf/google/protobuf/wrappers.proto".
google/protobuf/wrappers.proto:99:9: "google.protobuf.BoolValue" is already defined in file "bazel-out/k8-fastbuild/genfiles/external/com_google_protobuf/google/protobuf/wrappers.proto".
google/protobuf/wrappers.proto:109:10: "google.protobuf.StringValue.value" is already defined in file "bazel-out/k8-fastbuild/genfiles/external/com_google_protobuf/google/protobuf/wrappers.proto".
google/protobuf/wrappers.proto:107:9: "google.protobuf.StringValue" is already defined in file "bazel-out/k8-fastbuild/genfiles/external/com_google_protobuf/google/protobuf/wrappers.proto".
google/protobuf/wrappers.proto:117:9: "google.protobuf.BytesValue.value" is already defined in file "bazel-out/k8-fastbuild/genfiles/external/com_google_protobuf/google/protobuf/wrappers.proto".
google/protobuf/wrappers.proto:115:9: "google.protobuf.BytesValue" is already defined in file "bazel-out/k8-fastbuild/genfiles/external/com_google_protobuf/google/protobuf/wrappers.proto".
test/proto/test_external_dep.proto: Import "google/protobuf/wrappers.proto" was not found or had errors.
�[32m[2,037 / 2,041]�[0m //test/proto:test_external_dep_srcjar; 1s linux-sandbox

protoc-jar: protoc version: 3.6.0, detected platform: linux-x86_64 (linux/amd64)
protoc-jar: embedded: bin/3.6.0/protoc-3.6.0-linux-x86_64.exe
protoc-jar: executing: [/tmp/protocjar6410912116838245960/bin/protoc.exe, --plugin=protoc-gen-scala=/tmp/protocbridge8218211340725012779, --scala_out=/tmp/bazelscalapb8448931866336676386, --proto_path=external/com_google_protobuf, --proto_path=., -Igoogle/protobuf/wrappers.proto=bazel-out/k8-fastbuild/genfiles/external/com_google_protobuf/google/protobuf/wrappers.proto, -Itest/proto/test_external_dep.proto=test/proto/test_external_dep.proto, bazel-out/k8-fastbuild/genfiles/external/com_google_protobuf/google/protobuf/wrappers.proto, test/proto/test_external_dep.proto]
 //test/proto:test_external_dep_srcjar; 1s linux-sandbox

ERROR: �[0m/workdir/test/proto/BUILD:35:1: Couldn't build file test/proto/test_external_dep.jar: scala //test/proto:test_external_dep failed (Exit 1): scalac failed: error executing command 
  (cd /home/bazel/.cache/bazel/_bazel_bazel/ec321eb2cc2d0f8f91b676b6d4c66c29/execroot/io_bazel_rules_scala && \
  exec env - \
  bazel-out/k8-fastbuild/bin/external/io_bazel_rules_scala/src/java/io/bazel/rulesscala/scalac/scalac @bazel-out/k8-fastbuild/bin/test/proto/test_external_dep_scalac_worker_input)
Execution platform: @bazel_tools//platforms:host_platform
 scala //test/proto:test_external_dep; 0s worker

java.lang.RuntimeException: Must have input files from either source jars or local files.
	at io.bazel.rulesscala.scalac.ScalacProcessor.processRequest(ScalacProcessor.java:59)
	at io.bazel.rulesscala.worker.GenericWorker.runPersistentWorker(GenericWorker.java:45)
	at io.bazel.rulesscala.worker.GenericWorker.run(GenericWorker.java:111)
	at io.bazel.rulesscala.scalac.ScalaCInvoker.main(ScalaCInvoker.java:41)
scala //test/proto:test_external_dep; 0s worker

I've tracked the issue down to a change in the proto_library providers. For the same repo and build files, the contents of ProtoProvider.proto.transitive_proto_path is different. In 0.19.2, that field is an empty list. In 0.22.0, it contains external/com_google_protobuf and .. The contents of this field is used to populate --proto_path. This causes the following issues:

1. --proto_path=. causes the external proto files to be included twice. This causes scalapb to fail, and output no files. When the worker tries to compile 0 files, it fails with Must have input files from either source jars or local files.
2. --proto_path=external/com_google_protobuf is incorrect because that directory doesn't exist, and produces the error message external/com_google_protobuf: warning: directory does not exist.

I'm not sure what needs to change here. I'm happy to spend some more time fixing this, but I'm not sure if this is a bug that should be reported to bazel, or if rules_scala needs to be updated.

It looks like this was fixed in the java_proto_library rule by simply removing proto_path=. bazelbuild/bazel@af36058

I believe this is related to #312

cc @johnynek

[natansil]

didn't bazelbuild/bazel@af36058 fix something related in proto_library NOT in java_proto_library?
maybe there is some kind of a regression for this fix...

[beala-stripe]

Here's what I think happened:

This commit, in addition to adding the correct external/ prefix to proto paths in ProtoInfo.transitive_proto_path also started to add "." to the list of proto paths. I'm not sure why that change was made, but it's reflected in this test.

This change broke java_proto_library in the same way that it's breaking scalapb_proto_library, so a quick fix was implemented, which simply strips "." from the list of proto paths.

Is that accurate @lberki? So yes, @natansil, I think the fix was related to the proto_library change, but the breakage doesn't happen until a java_proto_library rule depends on it, mirroring the situation here in rules_scala, where the breakage occurs when a scalapb_proto_library target depends a proto_library.

I've added the same fix to the branch at #685. The proto targets build locally, but I'm still waiting on CI for the full test suite.

[beala-stripe]

Fix has been merged #685