bazeltools / bazel-deps

Generate bazel dependencies for maven artifacts
MIT License
249 stars 121 forks source link

confused about transitive dependency visibility #301

Closed dlaflamme closed 2 years ago

dlaflamme commented 3 years ago

I have a simple scala file that has:

import play.api.libs.json.{JsString, JsValue, Reads, Writes}

My dependencies.yaml file includes:

  com.typesafe.play:
    play-json:
      lang: scala
      version: "2.6.7"

and the generated play BUILD file is:

load("@io_bazel_rules_scala//scala:scala_import.bzl", "scala_import")
load("@io_bazel_rules_scala//scala:scala.bzl", "scala_library")
scala_import(
    name = "play_functional",
    jars = [
        "//external:jar/com/typesafe/play/play_functional_2_12"
    ],
    runtime_deps = [
        "//3rdparty/jvm/org/scala_lang:scala_library"
    ],
    visibility = [
        "//3rdparty/jvm:__subpackages__"
    ]
)

scala_import(
    name = "play_json",
    jars = [
        "//external:jar/com/typesafe/play/play_json_2_12"
    ],
    runtime_deps = [
        "//3rdparty/jvm/com/fasterxml/jackson/core:jackson_annotations",
        "//3rdparty/jvm/com/fasterxml/jackson/core:jackson_core",
        "//3rdparty/jvm/com/fasterxml/jackson/core:jackson_databind",
        "//3rdparty/jvm/com/fasterxml/jackson/datatype:jackson_datatype_jdk8",
        "//3rdparty/jvm/com/fasterxml/jackson/datatype:jackson_datatype_jsr310",
        "//3rdparty/jvm/joda_time:joda_time",
        "//3rdparty/jvm/org/scala_lang:scala_library",
        "//3rdparty/jvm/org/scala_lang:scala_reflect",
        "//3rdparty/jvm/org/typelevel:macro_compat",
        ":play_functional"
    ],
    visibility = [
        "//visibility:public"
    ]
)

Note that play_functional is a transitive dependency and thus has visibility as subpackages, presumably as a result of https://github.com/johnynek/bazel-deps/pull/165.

My BUILD file only lists play_json as a dependency:

[...]
scala_library(
    name = "widget",
    srcs = glob(
        [
            "src/main/scala/com/example/*.scala",
        ]
    ),
    deps = [
        "//3rdparty/jvm/com/typesafe/play:play_json",
    ],
)
[...]

bazel build fails with:

scala //widget:widget failed: (Exit 1): scalac failed: error executing command bazel-out/k8-opt-exec-2B5CBBC6/bin/external/io_bazel_rules_scala/src/java/io/bazel/rulesscala/scalac/scalac @bazel-out/k8-fastbuild/bin/widget/widget_scalac_worker_input
widget/src/main/scala/com/example/Widget.scala:28: error: Symbol 'type play.api.libs.functional.Reducer' is missing from the classpath.
This symbol is required by 'value play.api.libs.json.Reads.JsArrayReducer'.
Make sure that type Reducer is in your classpath and check for conflicting dependencies with `-Ylog-classpath`.
A full rebuild may help if 'Reads.class' was compiled against an incompatible version of play.api.libs.functional.
    implicit def StringToWidget(ws: String): Widget = Widget(ws)
                                    ^
one error found
Build failed
java.lang.RuntimeException: Build failed
        at io.bazel.rulesscala.scalac.ScalacWorker.compileScalaSources(ScalacWorker.java:254)
        at io.bazel.rulesscala.scalac.ScalacWorker.work(ScalacWorker.java:70)
        at io.bazel.rulesscala.worker.Worker.persistentWorkerMain(Worker.java:92)
        at io.bazel.rulesscala.worker.Worker.workerMain(Worker.java:46)
        at io.bazel.rulesscala.scalac.ScalacWorker.main(ScalacWorker.java:37)

I can get this to build, but only if I add play_functional as a dependency in my BUILD file and I change the visibility of play_functional in the generated BUILD file (3rdparty/jvm/com/typesafe/play/BUILD) to public visibility. But doing this feels wrong. What is the correct way to deal with this?

gannicottb commented 2 years ago

I'd love to know the answer to this, I'm facing the exact same issue.

johnynek commented 2 years ago

I didn't see this until recently.

Most JVM libraries aren't really developed with a clear idea of compilation classpath and runtime classpath. Sometimes things you don't use directly still need to be on the compilation classpath because they appear on some APIs that the compiler needs.

This seems to be such a case.

So, you will need to add play-functional or whatever the dependency is to your dependencies.yaml, that will make it a non-private dependency. And then you can depend on it without hacking the internal visibility of the file.

The motivation for separating the compile classpath from the runtime classpath is to improve the cache efficiency (you don't need to recompile if the compile classpath API jars didn't change and the source code didn't change even if the runtime classpath did change).

gannicottb commented 2 years ago

For my case, I found that with transitivity: runtimeDeps set in my dependencies.yaml, I had to list a lot of transitive dependencies out explicitly (and then require them in the deps of the target in question), which is probably the recommended path, but it made adding any new library a lot of work. I set transitivity: exports and then I didn't need to list those transitive deps, though I suspect that's not good practice (even if it is convenient and allows me to avoid overspecifying the deps for my target). Is there a middle ground between "listing out every single transitive dependency in dependencies.yaml" and "using transitivity: exports (which is bad, I suppose?)"?

johnynek commented 2 years ago

yeah, it's a tough call: how pedantic do we want to be and how minimal do we want to keep the class paths in order to minimize rebuilds.

Also, with rules_scala using strict deps, having jars on the class path you don't use can be an issue.