bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
23.05k stars 4.03k forks source link

Support `runtime_deps` in `java_plugin` rule #16777

Open rdesgroppes opened 1 year ago

rdesgroppes commented 1 year ago

Description of the feature request:

As of Bazel 5.3.2, the java_plugin documentation mentions that:

Arguments are identical to java_library, except for the addition of the processor_class argument.

... but this is not true for runtime_deps:

no such attribute 'runtime_deps' in 'java_plugin' rule

Would it be possible to support this argument there to allow passing dependencies that only make sense for the execution of the annotation processor?

What underlying problem are you trying to solve with this feature?

When registering an existing annotation processor (jmh, immutables, etc.), there is no need to pass any srcs (sources to compile) and thus no deps (compile-time dependencies), but deps is currently the only way to pass the jar in which the processor_class resides.

Having the possibility to pass the annotation processor jar through runtime_deps in this case would also prevent some unused dependency detection/removal tools (unused_deps, unused-deps-py) from spotting it when passed through deps.

Which operating system are you running Bazel on?

Ubuntu 22.04.1 LTS (GNU/Linux)

What is the output of bazel info release?

release 5.3.2

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

comius commented 1 year ago

The implementation already uses deps as something that only makes sense during execution of the plugin. Those deps cannot be used by targets where plugin is run. Thus, that's why there are no runtime_deps.

Maybe there is a problem that unused_deps don't account for this correctly?

rdesgroppes commented 1 year ago

@comius thanks for commenting. But I don't see where unused_deps tool(s) are wrong there. I mean, I wouldn't have thought differently ("POLA"): needed compile-time go through deps, whereas runtime-only needed dependencies go through runtime_deps, shouldn't they? At least similarly to:

Moreover, java_plugin documents runtime_deps here and there, either implicitly or explicitly:

Implicit output targets Arguments are identical to java_library, except for the addition of the processor_class argument.

  • deps The jars built by java_library rules listed in deps will be on the compile-time classpath of this rule. Furthermore the transitive closure of their deps, runtime_deps and exports will be on the runtime classpath.
  • srcs This argument is almost always required, except if a main_class attribute specifies a class on the runtime classpath or you specify the runtime_deps argument.
  • resource_jars Deprecated: Use java_import and deps or runtime_deps instead.
davido commented 1 year ago

It works as expected, consider this auto-factory plugin:

java_plugin(
    name = "auto-factory-plugin",
    generates_api = 1,
    processor_class = "com.google.auto.factory.processor.AutoFactoryProcessor",
    visibility = ["//visibility:private"],
    deps = [
        "@auto-common//jar",
        "@auto-factory//jar",
        "@auto-service-annotations//jar",
        "@auto-value-annotations//jar",
        "@auto-value//jar",
        "@guava//jar",
        "@javapoet//jar",
        "@javax_inject//jar",
    ],
)

Given that quite some transitive dependencies are required, the exported dependency is only @auto-factory//jar:

java_library(
    name = "auto-factory",
    data = ["//lib:LICENSE-Apache2.0"],
    exported_plugins = [
        ":auto-factory-plugin",
    ],
    visibility = ["//visibility:public"],
    exports = ["@auto-factory//jar"],
)

Say rule foo depends on auto-factory and trying to retrieve transitive dependencies, only exported dependency will be reported ("@auto-factory//jar").

rdesgroppes commented 1 year ago

@davido how does this contribute to the present feature request?

davido commented 1 year ago

Having the possibility to pass the annotation processor jar through runtime_deps in this case would also prevent some unused dependency detection/removal tools (unused_deps, unused-deps-py) from spotting it when passed through deps.

I am running unused_deps on this repository, with bunch of java_plugin (e.g. here) with transitive dependencies, provided by deps attribute and unused_depsdoesn't report any unused dependencies.

rdesgroppes commented 1 year ago

Ah, thanks for the additional comment: should I understand that there are counterexamples where the distinction between deps and runtime_deps may not be necessary from the point of view of unused_deps, but may be from the point of view of unused-deps-py?

In fact, it doesn't matter because this is not the primary reason for this feature request, and I now realize I should not have added a secondary reason.

rdesgroppes commented 1 year ago

I guess the long-standing inconsistency have to be coped with until further notice.