salesforce / rules_spring

Bazel rule for building Spring Boot apps as a deployable jar
BSD 3-Clause "New" or "Revised" License
221 stars 49 forks source link

deps_exclude deps are incorrectly modelled in the dependency graph #171

Open plaird opened 11 months ago

plaird commented 11 months ago

The deps_exclude list passed to the springboot() rule is intended to logically disengage those dependencies from the springboot target.

springboot(
    name = "foo",
    boot_app_class = "com.salesforce.foo.FooService",
    java_library = ":foo_lib",
    deps_exclude = [
        "@maven//:org_projectlombok_lombok", # lombok jar will be removed from the springboot jar because it is not wanted at runtime
    ],
)

This is implemented in the packaging phase of springboot() - those dependencies are not added to the springboot jar. So far so good.

But within the Bazel dependency graph, using this feature creates hard dependencies on the targets in the list because we are listing their labels in the target.

This is wrong for three reasons:

  1. The point of the attribute is to remove a dependency, not solidify it.
  2. Any extraneous dependency in the list will become a new dependency of the springboot target. For example, if your springboot target does not have //a/b/c:my_java_lib in the transitive graph, but you add it to deps_exclude by mistake, any change to my_java_lib will trigger a rebuild of the springboot app.
  3. In one case we have internally, listing a grpc dependency in the deps_exclude attribute upgraded it from an implicit dependency to a hard dependency. This caused it to start showing in bazel query output when --noimplicit_deps was used.

The point of using labels in the deps_exclude list is so that if someone changes the name of an upstream target, it would break the springboot target and the developer would know to fix it. So I like using the labels for this.

Alternatively, the deps_exclude_paths attribute uses a raw string to match deps that need to be excluded. It does not suffer from this hard-dependency problem, but it also is fragile. If someone changes the name of the upstream target, the springboot target will silently stop excluding it.

I don't think Bazel has a good way to support what we are doing here and we can't fix this. This bug is informational.