Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Creating a JAR With No Dependencies? #55

Closed
amit-traiana opened this issue Feb 13, 2020 · 5 comments · Fixed by #106
Closed

Creating a JAR With No Dependencies? #55

amit-traiana opened this issue Feb 13, 2020 · 5 comments · Fixed by #106
Labels
bug Something isn't working lang-java Java rules specific resolved-next-release Code to resolve issue is pending release

Comments

@amit-traiana
Copy link

amit-traiana commented Feb 13, 2020

At the moment, when building a Java Library, all dependencies are being included into a single artifact (JAR). For example:

proto_library(
    name = 'test',
    srcs = ['test.java'],
    deps = ['://dependencies:java_dependency']
)

java_grpc_library(
    name = "grpc",
    deps = [":test"],
)

The Jar that will get created will have both the test.java class, and the test2 classes. This is kinda goes against how java_proto_library works. The reason it's important for Java users is due to dependency management. Let's say for example I have two Jars. One would be test (the one defined above), and another one called test2. Both have dependency in ['://dependencies:java_dependency'] - but one Jar was created a week ago, and one Jar today and include changes in ['://dependencies:java_dependency']. When both Jars will load, JVM will randomly pick one - and that's a problem.

EDIT: Well, this is more complex than I originally thought.

        DefaultInfo(
            files = depset(all_outputs),
            data_runfiles = ctx.runfiles(files = all_outputs),
        ),

It looks like all the Provider returns all the chain of dependencies. I'm assuming that's on purpose, so you won't have to re-define the same deps on target that use the rule. So while makes writing Bazel easier, it's a problem with Java. Maybe it's worth considering adding a 'Transitive' flag of a sort that defines if the return value form the rule return all transitive dependencies - or just the head.

@amit-traiana
Copy link
Author

It's seems like stackb version has the transitive flag. Is it something that got lost in the move to aspects? it's one of those things that you can't do with aspects?

@aaliddell
Copy link
Member

it's one of those things that you can't do with aspects?

Bingo! The aspect solves a bunch of problems and produces just as many itself...

If I'm understanding correctly, this is similar to the issue with C++, where shared upstream dependencies lead to unexpected behavior due to multiple definitions: #25. The solution in that case is to make cpp_*_library interact natively with the C++ compilation, rather than just being a macro around the library function. I expect we'll have to do the same here, unless I've missed the point?

@aaliddell aaliddell added the bug Something isn't working label Feb 16, 2020
@amit-traiana
Copy link
Author

If I'm understanding correctly, this is similar to the issue with C++, where shared upstream dependencies lead to unexpected behavior due to multiple definitions: #25.

Yes, I think so. The difference is that everything works on Scala/Java - but the end artifact is somehow tainted.

I expect we'll have to do the same here, unless I've missed the point?
I'm not entirely sure this will help the Java case, because the problem is with what the Provider returns. The Depset includes all the transitive dependencies. This is good on most scenario. For example:

proto_library(
    name = "service_b",
    srcs =  ["service_b.proto"],
)

proto_library(
    name = "service_a",
    srcs =  ["service_a.proto"],
    deps = [":service_b"], 
)

java_grpc_compile(
    name = "java_grpc",
    deps = ["service_a"], 
)

java_library(
    name = "java_library",
    srcs = [":java_grpc", "app.java"]
)

The provider of java_grpc_compile will return both the service_a classes, and the service_b classes. This is pretty cool, because it means now on java_library, I don't have to state service_b as dependency - it's already there. If I would be using the build-in rule, I would have need to change my java_library to reflect all my dependencies like so:

java_library(
    name = "java_library",
    srcs = [":java_grpc", "app.java"]
    deps = [":service_b"],
)

So there's benefits in return all transitive dependencies. That being said - The rule java_library work in a way that it takes all the information it gets from the provider, and add it to the artifact. So my artifact JAR will libraries of both service_a, service_b and app.java. What I really wanted is just the classes of service_a and app.java in the JAR.

So I think some people will find it useful to get all transitive dependencies - while other won't. I'm assuming that's why I was a flag on the original rules_proto. On rules_proto it starts with just the root files, and add the transitive dependencies into final Provider if the flag is being set. I think something like that could be useful here too. There's a section you add the transitive dependencies? maybe it's possible to also save the state of the files before merging with all dependencies and by the flag decide if to return the modifies transitive list or the original list?

@amit-traiana
Copy link
Author

Well, I was able to kinda solve it - but the solution is specific to how ouר Bazel project looks like. I added another argument to the proto_compile_attr called transitive:

    "transitive": attr.bool(
        doc = "If true, generates outputs for dependencies directly named in 'deps' AND all transitive dependencies",
        default = True,
    ),

I'm letting everything compiles, but I modify the DefaultProvider to return just the root outputs:

    # Create default and proto compile providers
    transitive_outputs = [f for files in final_output_files.values() for f in files] + final_output_dirs

    default_provider = None

    if ctx.attr.transitive:
        default_provider = DefaultInfo(
            files = depset(transitive_outputs),
            data_runfiles = ctx.runfiles(files = transitive_outputs),
        )
    else:
        non_transitive_outputs = []
        # Bad Logic

        default_provider = DefaultInfo(
            files = depset(non_transitive_outputs),
            data_runfiles = ctx.runfiles(files = transitive_outputs),
        )

    return [
        ProtoCompileInfo(
            label = ctx.label,
            output_files = final_output_files,
            output_dirs = final_output_dirs,
        ),
        default_provider,
    ]

The problem is how I extract the direct ouputs. The logic is very coupled with how I build my project, and it's not a generic solution. Another thing to remember, is that some rules has multiple tools, so Java for example will output two outputs - one for grpc and one for proto - both should be return because they are being generated from a single direct dependency. Maybe you have an idea for a generic solution to extract the direct dep outputs?

@aaliddell
Copy link
Member

This should be resolved in 3.0.0, where non-transitive build is available

@aaliddell aaliddell added the resolved-next-release Code to resolve issue is pending release label Feb 13, 2021
@aaliddell aaliddell added the lang-java Java rules specific label Feb 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lang-java Java rules specific resolved-next-release Code to resolve issue is pending release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants