Proposal: Add modify_classpath to JavaInfo

[ckilian867]

Context

We maintain a large Monorepo for multiple JVM languages that allows for multiple versions of external dependencies.

When JavaInfo objects are merged, the version that ends up on the classpath depends on the order that dependencies were declared in the 'deps' field (and other similar fields, such as exports and runtime_deps).

This can lead to situations where a target may depend directly on MyLibrary-1.0, but a different dependency appearing earlier in 'deps' transitively bring in MyLibrary-2.0. The version resolved on the classpath will be MyLibrary-2.0, despite the desired version being MyLibrary-1.0.

Proposal

Our module file adds a patch file that exposes a new method in JavaInfo called 'modify_classpath'. The method accepts a JavaInfo object and constructs a new JavaInfo object with selected jars pruned from the classpath. This is the method header:

def modify_classpath(java_info, exclude_jars = [], add_compile_jars = [], add_runtime_jars = []):
    """Returns a copy of the given JavaInfo with modified compile and runtime classpaths.

    Enables classpath manipulation: excluding specific jars and/or adding new jars to the compile-time 
    and runtime classpaths. 

    Args:
        java_info: (JavaInfo) The JavaInfo to modify.
        exclude_jars: ([File]) Jars to remove from all classpath fields.
        add_compile_jars: ([File]) Jars to prepend to the compile-time classpath.
        add_runtime_jars: ([File]) Jars to prepend to the runtime classpath.

    Returns:
        (JavaInfo) A new JavaInfo with the modified classpaths.

Our call flow is essentially:

dep_java_infos = [dep[JavaInfo] for dep in ctx.attr.deps if JavaInfo in dep]
merged_java_info = java_common.merge(dep_java_infos)
pruned_java_info = java_common.modify_classpath(merged_java_info, exclude_jars = exclude_jars)

This enables selective exclusion of MyLibrary-2.0 from the merged JavaInfo object, allowing MyLibrary-1.0 to be resolved on the classpath

We have a working proof of concept. We would appreciate high-level feedback before productionizing and creating the PR.

Thank you!

[hvadehra]

I'd like to hear what @cushon thinks about this but to me it seems like the wrong way to go about this.

Having multiple versions is antithetical to the Bazel philosophy¹. Have you considered adopting a one-version policy? With recent changes² this can now even be done in a gradual fashion.

This can lead to situations where a target may depend directly on MyLibrary-1.0, but a different dependency appearing earlier in 'deps' transitively bring in MyLibrary-2.0

I admit this is not ideal, and JavaInfo should use toplogical ordering instead of preorder³. That will be an incompatible change (and possibly a painful migration internally at Google), but something I would support. EDIT: probably infeasible

Footnotes

1. https://bazel.build/basics/dependencies#the_one-version_rule ↩

2. https://github.com/bazelbuild/bazel/issues/1071#issuecomment-3388668407 ↩

3. https://bazel.build/extending/depsets#order ↩

[ckilian867]

Thanks for the response @hvadehra!

Uber's Java Monorepo has over 7500 java_library and scala_library targets, with a very high amount of dependency divergence over years of development. A migration to a one-version policy would be a multi-year effort and is not viable as a short or medium term solution.

With recent changes2 this can now even be done in a gradual fashion.

This is very, very interesting! I think we can incorporate this as a way to stop the bleed, but we'd still need a way to bring classpath consistency for the very long tail of divergences we have.

[cushon]

I agree with @hvadehra's comment about one version enforcement being a preferred approach, I have concerns about making modify_classpath a part of the Java rules API.

I think that might be something you could create yourself using existing Java rules APIs, is that something you've already done or investigated? Where there gaps in the existing APIs that prevented that?

[ckilian867]

Thanks for the reply @cushon!

one version enforcement being a preferred approach

I agree that one-version is ideal. Unfortunately, we don't have a realistic path to getting there in our repo. There are dozens of multi-version dependencies spread out across thousands of services in the repo, and upgrade/downgrades can be a very delicate process.

I think that might be something you could create yourself using existing Java rules APIs, is that something you've already done or investigated?

Yes, I've spent a lot of time on this. The issue I run into is that the classpath is constructed implicitly from the deps, runtime_deps, and exports fields. We can control the order the dependencies in these fields get passed. However, when each dependency is processed, the transitive dependencies also get processed as well, and we can't control the order here.

Let's say that libOne brings in A-1.0 and B-2.0, and libTwo brings in A-2.0 and B-1.0. We can control which of libOne and libTwo gets processed first, but if the desired classpath were A-2.0:B-2.0, it is not possible.

Here are a few approaches we tried:

Approach: Pruning via creating new Java Infos

• Iterate through the transitive classpath of each dependent JavaInfo and combine
• Create a new JavaInfo for each dependency we want to keep, pruning out ones we don't
• Feed these JavaInfos to the underlying java_library

Issues

• Creating all these JavaInfos at every level is enormously expensive. bazel build --nobuild //... took upwards of 16 minutes
• Other JavaInfo information lost - it is difficult to create a JavaInfo using a subset of fields from another, and some information was lost from the original JavaInfo. We couldn't just merge the pruned list, since pruned jars could be re-introduced transitively.

Approach: Classpath Ordering

• Iterate through the transitive classpath of each dependent JavaInfo and combine
• Create new JavaInfo objects in the order we want them. For example, in the above case, we'd create them in the order [A-2.0, A-1.0, B-2.0, B-1.0]
• Feed these JavaInfos first, followed by the 'real' JavaInfo objects. These resolves the 'lost information' issue, since we still pass all the 'real' JavaInfos in addition to the new ones.

Issues

• Still creates a lot of JavaInfo objects, very expensive
• A-2.0:A-1.0:B-2.0:B-1.0 is meaningfully different than A-2.0:B-2.0. A class may accidentally pull in classes from a mixture of B-2.0 and B-1.0 if a method was removed in the 2.0 release. This can lead to successful compilation but runtime crashes.
• Ineffectual against transitive fat jars (another issue we need to resolve, but again, easier said than done with the current state of the repo)

I am open to feedback on other approaches - maybe there are some available tools I am missing. But without modifying rules_java to enable direct classpath manipulation, I was not able to reach an acceptable solution.

[cushon]

Creating all these JavaInfos at every level is enormously expensive

It makes sense that is very expensive, since the depsets are getting flattened and recreated at every level. Could that work happen once for the top level targets instead?

I think a built-in implementation of modify_classpath would also be very expensive if it was used once at every level.

Other JavaInfo information lost - it is difficult to create a JavaInfo using a subset of fields from another, and some information was lost from the original JavaInfo

I think that taking a JavaInfo apart and re-creating an equivalent one is something the APIs should support, if the current APIs don't support that I think that would be good to track as a separate issue.

[ckilian867]

Could that work happen once for the top level targets instead?

This can cause compilation errors in intermediate targets. The top-level may exclude B-1.0 but the intermediate target could still run into issues if its present in the classpath.

I think a built-in implementation of modify_classpath would also be very expensive if it was used once at every level.

The bottleneck was the number of JavaInfo objects created. We ran --starlark_cpu_profile and nearly all the time was JavaInfo constructor times. The flattening is expensive, but it doesn't cause the same performance issue as creating thousands of new JavaInfo objects. The modify_classpath would flatten the depset, but it would only create one call to _new_javainfo.

I think that taking a JavaInfo apart and re-creating an equivalent one is something the APIs should support

I think this would be great! In particular, we can call methods like transitive_compile_time_jars and transitive_runtime_jars but we can't set them. I'd love to be able to construct a JavaInfo object using my desired classpath as an input into the constructor.