META-INF/services files are not concatenated by scala_binary

[greggdonovan]

If multiple JARs contain the same Service Loader file, they should be combined. For example, we have jar://org_apache_beam_beam_sdks_java_extensions_google_cloud_platform_core/META-INF/services/org.apache.beam.sdk.io.FileSystemRegistrar containing org.apache.beam.sdk.extensions.gcp.storage.GcsFileSystemRegistrar and jar://org_apache_beam_beam_sdks_java_core/META-INF/services/org.apache.beam.sdk.io.FileSystemRegistrar containing org.apache.beam.sdk.io.LocalFileSystemRegistrar.

The _deploy JAR should contained the concatenated contents of both files. This is done by java_binary here.

I'm happy to help with this ticket if you can give me a pointer on the approach you would prefer to see. It seems that JarHelper will need a similar method of deferring writes to the output until the like entries could be concatenated.

[ittaiz]

I'm going on vacation and will be available to take a look in around 10 days. I think Oscar also has limited capacity. If it's not urgent I'd recommend you try to familiarize yourself with the code and maybe come up with a suggestion? Preferably one that closely resembles the java binary approach.

…

On Thu, 10 Aug 2017 at 22:40 Gregg Donovan ***@***.***> wrote: If multiple JARs contain the same Service Loader <https://docs.oracle.com/javase/8/docs/api/java/util/ServiceLoader.html> file, they should be combined. For example, we have jar://org_apache_beam_beam_sdks_java_extensions_google_cloud_platform_core/META-INF/services/org.apache.beam.sdk.io.FileSystemRegistrar containing org.apache.beam.sdk.extensions.gcp.storage.GcsFileSystemRegistrar and jar://org_apache_beam_beam_sdks_java_core/META-INF/services/org.apache.beam.sdk.io.FileSystemRegistrar containing org.apache.beam.sdk.io.LocalFileSystemRegistrar. The _deploy JAR should contained the concatenated contents of both files. This is done by java_binary here <https://github.com/bazelbuild/bazel/blob/master/src/java_tools/singlejar/java/com/google/devtools/build/singlejar/DefaultJarEntryFilter.java#L95:L97> . I'm happy to help with this ticket if you can give me a pointer on the approach you would prefer to see. It seems that JarHelper <https://github.com/bazelbuild/rules_scala/blob/master/src/java/io/bazel/rulesscala/jar/JarHelper.java> will need a similar method of deferring writes to the output until the like entries could be concatenated. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#261>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABUIFz4jXoGTQ6a5anAy98H31iiCcnTJks5sW1yhgaJpZM4Oz40k> .

[greggdonovan]

@ittaiz I'll give it a try! What do you think of using Bazel's built-in singlejar utility rather than depending on a copy of JarCreator? It seems like singlejar already has the logic for merging multiple JARs and resolving various conflicts.

[ittaiz]

I (almost) always prefer reusing code however: 1. Oscar made the decision back then so I don't have the context on why. 2. Given #1 to move I'd really like you to give a detailed account about the difference between the two since Oscar might have tweaked it. 3. Need to see Bazel are up for exposing it

…

On Fri, 11 Aug 2017 at 0:44 Gregg Donovan ***@***.***> wrote: @ittaiz <https://github.com/ittaiz> I'll give it a try! What do you think of using Bazel's built-in singlejar utility <https://github.com/bazelbuild/bazel/tree/master/src/java_tools/singlejar> rather than depending on a copy of JarCreator? It seems like singlejar already has the logic for merging multiple JARs and resolving various conflicts. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#261 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABUIFw1DruBPXENS3awTmaMUh-lB7tYZks5sW3m4gaJpZM4Oz40k> .

[azymnis]

We hit this exact issue too for jar://org_apache_beam_beam_sdks_java_extensions_google_cloud_platform_core/META-INF/services/org.apache.beam.sdk.io.FileSystemRegistrar. @greggdonovan have you found any temporary workaround?

[johnynek]

Sorry for the slow turn-around.

The reason we use the JarCreator code here:
https://github.com/bazelbuild/rules_scala/blob/master/scala/scala.bzl#L310
https://github.com/bazelbuild/rules_scala/blob/master/src/java/io/bazel/rulesscala/jar/JarCreator.java#L219

is that previously we did something worse (just call jar, which had correctness issues with timestamps). The main issue we wanted to solve was reproducibility. That JarCreator code was copied from bazel.

We try to minimize the bazel dependency since it makes building harder. That said, we do have some dependencies. It would be nice if bazel were more modular in this way and we didn't need to checkout a giant repo to depend on small item. This causes real performance problems for us.

I'd rather just copy this code:
https://github.com/bazelbuild/bazel/blob/master/src/java_tools/singlejar/java/com/google/devtools/build/singlejar/ConcatenateStrategy.java

since it looks like combining is pretty trivial.

I think the work around could be to just make a java_binary target instead of a scala_binary target and have no sources. That should work since java can depend on scala in the latest rules.

[johnynek]

another note: bazel ships some things in the @bazel_tools repo that is implicitly available. This is not well documented (or maybe documented at all), but if it is there and we can just rely on it (like we do with ijar) then calling it is probably the right move.

[ittaiz]

I'll just add that if it isn't exposed then maybe Bazel team are up to exposing it there

…

On Thu, 31 Aug 2017 at 21:32 P. Oscar Boykin ***@***.***> wrote: another note: bazel ships some things in the @bazel_tools repo that is implicitly available. This is not well documented (or maybe documented at all), but if it is there and we can just rely on it (like we do with ijar) then calling it is probably the right move. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#261 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABUIF0mxdrAL-OgFkGOl7j1SrpLifj_Aks5sdvxAgaJpZM4Oz40k> .

[azymnis]

@johnynek I'll take a look at this. FWIW having a java_binary rule depending on scala targets did not work for me, but I should check my bazel version etc

[johnynek]

that was added somewhat recently, so you may need later rules/bazel.

[azymnis]

@ittaiz @johnynek turns out that singlejar is already part of @bazel_tools. I just need to make sure that I'm invoking it correctly. I'll try this out locally on my code to see if it does the trick

[ittaiz]

#270 is merged

[greggdonovan]

Thank you (belatedly) @azymnis and @ittaiz!