Single-jar pack_sources should conform to Bazel behavior

scala/private/phases/phase_compile.bzl

@@ -308,17 +308,32 @@ def _create_scala_compilation_provider(ctx, ijar, source_jar, deps_providers):
         neverlink = ctx.attr.neverlink,
     )
 
-def _pack_source_jar(ctx, scala_srcs, in_srcjars):
-    output_jar = ctx.outputs.jar
-    source_jar_name = output_jar.basename[:-len(output_jar.extension)] + "-src.jar"
-    output_source_jar = ctx.actions.declare_file(source_jar_name, sibling = output_jar)
-    return java_common.pack_sources(
-        ctx.actions,
-        output_source_jar = output_source_jar,
-        sources = scala_srcs,
-        source_jars = in_srcjars,
-        java_toolchain = find_java_toolchain(ctx, ctx.attr._java_toolchain),
-    )
+def _pack_source_jar(ctx, scala_srcs, input_srcjars):
+    # https://github.com/bazelbuild/bazel/blob/ff6c0333e4f957bb9f7ab5401b01dbf3e9b515b1/src/main/java/com/google/devtools/build/lib/rules/java/JavaInfoBuildHelper.java#L180-L183
+    # java_common.pack_sources checks for no srcs and only a single input jar
+    # if so, it checks that output_source_jar is null
+    # passing that, it will return the input source jar directly
+    # However, pack_sources will FAIL if both output_source_jar and
+    # the deprecated output_jar field are BOTH null
+    # Therefore, we can return the single input jar ourselves
+    if not scala_srcs and len(input_srcjars) == 1:
+        return input_srcjars[0]
+    else:
+        output_jar = ctx.outputs.jar
+        without_ext = output_jar.basename[:-len(output_jar.extension)]
+        if without_ext.endswith("."):
+            without_ext = without_ext[:-1]
+        source_jar_name = without_ext + "-src.jar"
+
+        output_source_jar = ctx.actions.declare_file(source_jar_name, sibling = output_jar)
+
+        return java_common.pack_sources(
+            ctx.actions,
+            output_source_jar = output_source_jar,
+            sources = scala_srcs,
+            source_jars = input_srcjars,
+            java_toolchain = find_java_toolchain(ctx, ctx.attr._java_toolchain),
+        )
 
 def _try_to_compile_java_jar(
         ctx,

test/src/main/scala/scalarules/test/srcjars/BUILD

@@ -1,4 +1,5 @@
 load("//scala:scala.bzl", "scala_library")
+load(":pack_sources_test.bzl", "pack_sources_test_suite")
 
 #TODO the way it SHOULD work (but isn't currently) is that
 # the source_jar target should make available a compiled jar,
@@ -12,8 +13,21 @@ scala_library(
     srcs = ["SourceJar1.srcjar"],
 )
 
+scala_library(
+    name = "multi_source_jar",
+    # SourceJar1.jar was created by:
+    # jar -cfM test/src/main/scala/scalarules/test/srcjars/SourceJar1.srcjar \
+    # test/src/main/scala/scalarules/test/srcjars/SourceJar1.scala
+    srcs = [
+        "SourceJar1.srcjar",
+        "SourceJar2.scala",
+    ],
+)
+
 scala_library(
     name = "use_source_jar",
     srcs = ["SourceJar2.scala"],
     deps = [":source_jar"],
 )
+
+pack_sources_test_suite(name = "pack_sources_test")

test/src/main/scala/scalarules/test/srcjars/pack_sources_test.bzl

@@ -0,0 +1,91 @@
+load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
+load("@bazel_skylib//lib:new_sets.bzl", "sets")
+
+# Tests and documents the functionality of phase_compile.bzl's _pack_source_jar
+
+def _single_source_jar_test_impl(ctx):
+    env = analysistest.begin(ctx)
+
+    target_under_test = analysistest.target_under_test(env)
+
+    srcjar_names = sets.make(
+        [j.basename for j in target_under_test[JavaInfo].source_jars],
+    )
+
+    # In line with Bazel's java_common.pack_sources,
+    # We return the initial .srcjar file since there are no source files
+    # Not sure where the second -src.jar comes from, maybe due to java rules
+    expected_names = sets.make([
+        "SourceJar1.srcjar",
+        "source_jar_java-src.jar",
+    ])
+
+    asserts.set_equals(env, expected = expected_names, actual = srcjar_names)
+
+    return analysistest.end(env)
+
+single_source_jar_test = analysistest.make(_single_source_jar_test_impl)
+
+def _multi_source_jar_test_impl(ctx):
+    env = analysistest.begin(ctx)
+
+    target_under_test = analysistest.target_under_test(env)
+
+    srcjar_names = sets.make(
+        [j.basename for j in target_under_test[JavaInfo].source_jars],
+    )
+
+    expected_names = sets.make([
+        "multi_source_jar-src.jar",
+        "multi_source_jar_java-src.jar",
+    ])
+
+    asserts.set_equals(env, expected = expected_names, actual = srcjar_names)
+
+    return analysistest.end(env)
+
+multi_source_jar_test = analysistest.make(_multi_source_jar_test_impl)
+
+def _source_jar_with_srcs_test_impl(ctx):
+    env = analysistest.begin(ctx)
+
+    target_under_test = analysistest.target_under_test(env)
+
+    srcjar_names = sets.make(
+        [j.basename for j in target_under_test[JavaInfo].source_jars],
+    )
+
+    # Since we have source files, we don't output a .srcjar
+    # Instead, we just return the bundle
+    expected_names = sets.make([
+        "use_source_jar-src.jar",
+    ])
+
+    asserts.set_equals(env, expected = expected_names, actual = srcjar_names)
+
+    return analysistest.end(env)
+
+source_jar_with_srcs_test = analysistest.make(_source_jar_with_srcs_test_impl)
+
+def pack_sources_test_suite(name):
+    single_source_jar_test(
+        name = "single_source_jar_test",
+        target_under_test = ":source_jar",
+    )
+    multi_source_jar_test(
+        name = "multi_source_jar_test",
+        target_under_test = ":multi_source_jar",
+    )
+    source_jar_with_srcs_test(
+        name = "source_jar_with_srcs_test",
+        target_under_test = ":use_source_jar",
+    )
+
+    native.test_suite(
+        name = name,
+        tests = [
+            ":single_source_jar_test",
+            ":multi_source_jar_test",
+            ":source_jar_with_srcs_test",
+        ],
+    )