rules_kotlin icon indicating copy to clipboard operation
rules_kotlin copied to clipboard

Transitive `data` files do not propagate through `kt_jvm_library` as they do in `java_library`.

Open freetheinterns opened this issue 2 years ago • 2 comments

I've been tracking down a bug we've noticed where files specified in the data attribute of either a kt_jvm_library or a java_library do not show up in the runtime of our unit tests.

The issue only happens when there is at least 1 hop between the data attribute and the test target. EG: library w/ data -> kt library -> test binary

The issue is also specific to kt_jvm_library. If all of the libraries in the chain are java_library targets, then we see the file as we expect.

The issue can be reproduced with 3 files (excluding workspace & maven setup).

repro/BUILD.bazel

load("@io_bazel_rules_kotlin//kotlin:jvm.bzl", "kt_jvm_library")
load("@rules_java//java:defs.bzl", "java_library", "java_test")

java_library(
    name = "java_library_with_data",
    data = ["transitive_data.txt"],
)

java_library(
    name = "java_test_lib",
    srcs = ["DataAttributeTest.java"],
    deps = ["@maven//:junit_junit"],
    runtime_deps = ["java_library_with_data"],
)

kt_jvm_library(
    name = "kt_test_lib",
    srcs = ["DataAttributeTest.java"],
    deps = ["@maven//:junit_junit"],
    runtime_deps = ["java_library_with_data"],
)

java_test(
    name = "java_test",
    runtime_deps = ["java_test_lib"],
    test_class = "repro.DataAttributeTest",
)

java_test(
    name = "kt_test",
    runtime_deps = ["kt_test_lib"],
    test_class = "repro.DataAttributeTest",
)

repro/DataAttributeTest.java

package repro;

import static org.junit.Assert.assertEquals;

import java.io.IOException;
import java.nio.file.FileVisitOption;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.List;
import java.util.stream.Collectors;
import org.junit.Test;

public class DataAttributeTest {
  @Test
  public void testTransitiveDataFile() throws IOException {
    // Walk the file tree in the current directory and look for any file matching the filename.
    List<String> searchResult =
        Files.walk(Paths.get("").toAbsolutePath(), 1000, FileVisitOption.FOLLOW_LINKS)
            .map(it -> it.toAbsolutePath().toString())
            .filter(it -> it.endsWith("transitive_data.txt"))
            .collect(Collectors.toList());

    assertEquals(1, searchResult.size());
  }
}

repro/transitive_data.txt

any text

Given the above setup, you will notice that the //repro:java_test will pass, but //repro:kt_test will fail. Adding data = ["transitive_data.txt"] directly to the kt_jvm_library will make things pass, so will adding java_library_with_data to the runtime deps of the kt_test target. Switching java_library_with_data to use kt_jvm_library does not help, and neither does switching kt_test to use kt_jvm_test.

This was tested using: [email protected] [email protected] [email protected] [email protected] [email protected] targeting 1.5 api version junit:junit:4.13.2

freetheinterns avatar Sep 21 '23 17:09 freetheinterns

There's a TODO which looks related. I think the solution is something like this:

diff --git a/kotlin/internal/jvm/impl.bzl b/kotlin/internal/jvm/impl.bzl
index 03b3d0c..341b6e5 100644
--- a/kotlin/internal/jvm/impl.bzl
+++ b/kotlin/internal/jvm/impl.bzl
@@ -50,10 +50,10 @@ def _make_providers(ctx, providers, transitive_files = depset(order = "default")
                     # explicitly include data files, otherwise they appear to be missing
                     files = ctx.files.data,
                     transitive_files = transitive_files,
-                    # continue to use collect_default until proper transitive data collecting is
-                    # implmented.
-                    collect_default = True,
-                ),
+                ).merge_all([
+                    target[DefaultInfo].default_runfiles
+                    for target in ctx.attr.runtime_deps
+                ]),
             ),
         ] + list(additional_providers),
     )

alexeagle avatar Sep 22 '23 19:09 alexeagle

I can confirm that applying that patch appears to fix the issue when I reproduce the example under examples/android 👍

freetheinterns avatar Sep 26 '23 00:09 freetheinterns