Repository: bazelbuild/bazel
Issue: sh_binary's data dependencies are not triggered during the build
Labels: type: bug
P3
team-Rules-Server

<!--
ATTENTION! Please read and follow:
- if this is a _question_ about how to build / test / query / deploy using Bazel, or a _discussion starter_, send it to bazel-discuss@googlegroups.com
- if this is a _bug_ or _feature request_, fill the form below as best as you can.
-->

### Description of the problem / feature request:

When a `sh_binary` has a target in the `data` attribute, the target is not built automatically when the `sh_binary` target is built.

### Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

* Check out [rules_docker](https://github.com/bazelbuild/rules_docker/tree/0b1a033eff28eeff474573c7ed2d5b9d9bec4af1)
* `bazel clean`
* `bazel build //docs:update`
* `ls bazel-bin/docs/container.md_`
Although `bazel-bin/docs/container.md_` is the output of `//docs:container_doc`, which is depended by `//docs:update`, it is not built.

### What operating system are you running Bazel on?

macOS

### What's the output of `bazel info release`?

release 5.0.0-homebrew



Issue comments:
I think that the root cause is a more fundamental inconsistency between how Starlark and native rules collect and declare data dependencies:

As per the [Starlark rules guidelines](https://docs.bazel.build/versions/main/skylark/rules.html#runfiles-features-to-avoid), Starlark rules should handle targets in `data`-like attributes by merging in both the files produced by these targets as well as their default runfiles. As a result, Starlark rules often return some files in their `DefaultInfo`'s `files` attribute that are not also contained in its `runfiles`.

However, native rules only collect transitive runfiles and not the output files from rule targets (see [addTargetIncludingFileTargets](https://github.com/bazelbuild/bazel/blob/4c56447fe18e31098ac14a80e2a98253de9c190d/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java#L931), which only adds output files for file targets). 

@linzhp Could you check whether https://github.com/bazelbuild/bazel/pull/15052 fixes the issue? It adapts the native logic to collect runfiles in the same way as is recommended for Starlark rules, but of course that may very well regress performance for native rules in general. I do not know whether changing this behavior that is so fundamental to Bazel's native rules is realistically possible. But if it doesn't fail too many tests, maybe there is a way to restrict it to Starlark rule targets or otherwise limit its impact on native rule targets.

Sorry for the delay. I pull from #15052, and built `//src:bazel-dev`. But when I used the built binary to build `//docs:update` of rules_docker, I got:

```
DEBUG: /private/var/tmp/_bazel_zplin/d6323f9e3146540840037c805ccd73ff/external/bazel_toolchains/rules/rbe_repo/version_check.bzl:59:14: 
Current running Bazel is not a release version and one was not defined explicitly in rbe_autoconfig target. Falling back to '4.0.0'
ERROR: /private/var/tmp/_bazel_zplin/d6323f9e3146540840037c805ccd73ff/external/bazel_tools/src/tools/launcher/BUILD:9:14: While resolving toolchains for target @bazel_tools//src/tools/launcher:launcher: No matching toolchains found for types @bazel_tools//tools/cpp:toolchain_type. Maybe --incompatible_use_cc_configure_from_rules_cc has been flipped and there is no default C++ toolchain added in the WORKSPACE file? See https://github.com/bazelbuild/bazel/issues/10134 for details and migration instructions.
ERROR: Analysis of target '//docs:update' failed; build aborted: 
INFO: Elapsed time: 6.538s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (50 packages loaded, 309 targets configured)
```
@linzhp The build succeeds for me following the instructions from your initial post. Are you on Linux and does `bazel build //...` succeed for you on the repo on the commit prior to mine? The change may cause building new transitive dependencies for `//docs:update`, but if those weren't broken before, they shouldn't be now. The result of rerunning the command with `--toolchain_resolution_debug='.*'` would also be useful to figure out what broke.
Switched to macOS for Intel and tried your fix again. It worked. Thanks!

I was on macOS for Apple Silicon before. The commit before yours fails too, but Bazel 5.0.0 works. I saw:

```
INFO: ToolchainResolution:     Type @bazel_tools//tools/cpp:toolchain_type: target platform @local_config_platform//:host: Rejected toolchain @local_config_cc//:cc-compiler-darwin_arm64; mismatching values: arm64
```

That looks more like an unrelated regression. Could you create a separate issue and perhaps rerun the test with 5.1.0rc2? 5.1.0 will be released soon.
Is this bug a dup (though more clearly restating the issue) of https://github.com/bazelbuild/bazel/issues/12348 ?
> Is this bug a dup (though more clearly restating the issue) of https://github.com/bazelbuild/bazel/issues/12348 ?

Pretty sure it is.
Dupping against #12348.
Sorry, I missed that #15052 is being actively developed, so I'll keep this issue alive as the author's preferred tracking bug.
As an FYI, there may be some dependencies on this behavior between the Python rules and some other Google-internal rules. i.e. enabling the flag introduced by this will likely show failures in the Google global regression tests (TGP). See the large comment block in blaze/python/common.bzl#collect_runfiles for more details. The gist, though, is some Google-internal rules have different default outputs vs data/default runfiles, and have come to rely on this so that otherwise unused outputs (some of which can be very large) don't get included in the runfiles.
@rickeylev Thanks for mentioning this. I added a workaround to the migration guide at https://github.com/bazelbuild/bazel/issues/16654. Could that even work for Google?

Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java b/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java
index 9d8b10955fa52d..d9933d60b19561 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java
@@ -935,7 +935,10 @@ public Builder add(
     /** Collects runfiles from data dependencies of a target. */
     @CanIgnoreReturnValue
     public Builder addDataDeps(RuleContext ruleContext) {
-      addTargets(getPrerequisites(ruleContext, "data"), RunfilesProvider.DATA_RUNFILES);
+      addTargets(
+          getPrerequisites(ruleContext, "data"),
+          RunfilesProvider.DATA_RUNFILES,
+          ruleContext.getConfiguration().alwaysIncludeFilesToBuildInData());
       return this;
     }
 
@@ -952,16 +955,20 @@ public Builder addNonDataDeps(
     @CanIgnoreReturnValue
     public Builder addTargets(
         Iterable<? extends TransitiveInfoCollection> targets,
-        Function<TransitiveInfoCollection, Runfiles> mapping) {
+        Function<TransitiveInfoCollection, Runfiles> mapping,
+        boolean alwaysIncludeFilesToBuildInData) {
       for (TransitiveInfoCollection target : targets) {
-        addTarget(target, mapping);
+        addTarget(target, mapping, alwaysIncludeFilesToBuildInData);
       }
       return this;
     }
 
-    public Builder addTarget(TransitiveInfoCollection target,
-        Function<TransitiveInfoCollection, Runfiles> mapping) {
-      return addTargetIncludingFileTargets(target, mapping);
+    @CanIgnoreReturnValue
+    public Builder addTarget(
+        TransitiveInfoCollection target,
+        Function<TransitiveInfoCollection, Runfiles> mapping,
+        boolean alwaysIncludeFilesToBuildInData) {
+      return addTargetIncludingFileTargets(target, mapping, alwaysIncludeFilesToBuildInData);
     }
 
     @CanIgnoreReturnValue
@@ -975,8 +982,10 @@ private Builder addTargetExceptFileTargets(
       return this;
     }
 
-    private Builder addTargetIncludingFileTargets(TransitiveInfoCollection target,
-        Function<TransitiveInfoCollection, Runfiles> mapping) {
+    private Builder addTargetIncludingFileTargets(
+        TransitiveInfoCollection target,
+        Function<TransitiveInfoCollection, Runfiles> mapping,
+        boolean alwaysIncludeFilesToBuildInData) {
       if (target.getProvider(RunfilesProvider.class) == null
           && mapping == RunfilesProvider.DATA_RUNFILES) {
         // RuleConfiguredTarget implements RunfilesProvider, so this will only be called on
@@ -988,6 +997,17 @@ private Builder addTargetIncludingFileTargets(TransitiveInfoCollection target,
         return this;
       }
 
+      if (alwaysIncludeFilesToBuildInData && mapping == RunfilesProvider.DATA_RUNFILES) {
+        // Ensure that `DefaultInfo.files` of Starlark rules is merged in so that native rules
+        // interoperate well with idiomatic Starlark rules..
+        // https://bazel.build/extending/rules#runfiles_features_to_avoid
+        // Internal tests fail if the order of filesToBuild is preserved.
+        addTransitiveArtifacts(
+            NestedSetBuilder.<Artifact>stableOrder()
+                .addTransitive(target.getProvider(FileProvider.class).getFilesToBuild())
+                .build());
+      }
+
       return addTargetExceptFileTargets(target, mapping);
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValue.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValue.java
index 9a59733973261b..19597cc44c23d8 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValue.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValue.java
@@ -636,6 +636,13 @@ public boolean legacyExternalRunfiles() {
     return options.legacyExternalRunfiles;
   }
 
+  /**
+   * Returns true if Runfiles should merge in FilesToBuild from deps when collecting data runfiles.
+   */
+  public boolean alwaysIncludeFilesToBuildInData() {
+    return options.alwaysIncludeFilesToBuildInData;
+  }
+
   /**
    * Returns user-specified test environment variables and their values, as set by the --test_env
    * options.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java
index f4f83bb319ff78..a7fbb8be55f51a 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java
@@ -433,6 +433,18 @@ public ExecConfigurationDistinguisherSchemeConverter() {
               + ".runfiles/wsname/external/repo (in addition to .runfiles/repo).")
   public boolean legacyExternalRunfiles;
 
+  @Option(
+      name = "incompatible_always_include_files_in_data",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.OUTPUT_SELECTION,
+      effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
+      metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+      help =
+          "If true, native rules add <code>DefaultInfo.files</code> of data dependencies to "
+              + "their runfiles, which matches the recommended behavior for Starlark rules ("
+              + "https://bazel.build/extending/rules#runfiles_features_to_avoid).")
+  public boolean alwaysIncludeFilesToBuildInData;
+
   @Option(
       name = "check_fileset_dependencies_recursively",
       defaultValue = "true",
@@ -930,6 +942,7 @@ public FragmentOptions getHost() {
     host.legacyExternalRunfiles = legacyExternalRunfiles;
     host.remotableSourceManifestActions = remotableSourceManifestActions;
     host.skipRunfilesManifests = skipRunfilesManifests;
+    host.alwaysIncludeFilesToBuildInData = alwaysIncludeFilesToBuildInData;
 
     // === Filesets ===
     host.strictFilesetOutput = strictFilesetOutput;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTestBase.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTestBase.java
index 45720b13462f39..344ebf23861505 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTestBase.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTestBase.java
@@ -89,7 +89,10 @@ public ConfiguredTarget create(RuleContext ruleContext)
             .addArtifact(testExecutable)
             .addArtifact(getInstrumentationApk(ruleContext))
             .addArtifact(getTargetApk(ruleContext))
-            .addTargets(runfilesDeps, RunfilesProvider.DEFAULT_RUNFILES)
+            .addTargets(
+                runfilesDeps,
+                RunfilesProvider.DEFAULT_RUNFILES,
+                ruleContext.getConfiguration().alwaysIncludeFilesToBuildInData())
             .addTransitiveArtifacts(AndroidCommon.getSupportApks(ruleContext))
             .addTransitiveArtifacts(getAdb(ruleContext).getFilesToRun())
             .merge(getAapt(ruleContext).getRunfilesSupport())
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
index 809fc927736960..33905dd3b0a9bf 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
@@ -469,7 +469,10 @@ private Runfiles collectDefaultRunfiles(
     // runtime jars always in naive link order, incompatible with compile order runfiles.
     builder.addArtifacts(getRuntimeJarsForTargets(getAndCheckTestSupport(ruleContext)).toList());
 
-    builder.addTargets(depsForRunfiles, RunfilesProvider.DEFAULT_RUNFILES);
+    builder.addTargets(
+        depsForRunfiles,
+        RunfilesProvider.DEFAULT_RUNFILES,
+        ruleContext.getConfiguration().alwaysIncludeFilesToBuildInData());
 
     // We assume that the runtime jars will not have conflicting artifacts
     // with the same root relative path
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
index dfb04a255166f7..4734c3a1754af8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
@@ -739,7 +739,10 @@ private void collectDefaultRunfiles(
         builder.addSymlinks(runfiles.getSymlinks());
         builder.addRootSymlinks(runfiles.getRootSymlinks());
       } else {
-        builder.addTarget(defaultLauncher, RunfilesProvider.DEFAULT_RUNFILES);
+        builder.addTarget(
+            defaultLauncher,
+            RunfilesProvider.DEFAULT_RUNFILES,
+            ruleContext.getConfiguration().alwaysIncludeFilesToBuildInData());
       }
     }
 
@@ -748,7 +751,10 @@ private void collectDefaultRunfiles(
 
     List<? extends TransitiveInfoCollection> runtimeDeps =
         ruleContext.getPrerequisites("runtime_deps");
-    builder.addTargets(runtimeDeps, RunfilesProvider.DEFAULT_RUNFILES);
+    builder.addTargets(
+        runtimeDeps,
+        RunfilesProvider.DEFAULT_RUNFILES,
+        ruleContext.getConfiguration().alwaysIncludeFilesToBuildInData());
 
     builder.addTransitiveArtifactsWrappedInStableOrder(common.getRuntimeClasspath());
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java
index b6e2245c82e379..b382b1f39f7a36 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java
@@ -886,11 +886,17 @@ public static Runfiles getRunfiles(
       depsForRunfiles.addAll(ruleContext.getPrerequisites("exports"));
     }
 
-    runfilesBuilder.addTargets(depsForRunfiles, RunfilesProvider.DEFAULT_RUNFILES);
+    runfilesBuilder.addTargets(
+        depsForRunfiles,
+        RunfilesProvider.DEFAULT_RUNFILES,
+        ruleContext.getConfiguration().alwaysIncludeFilesToBuildInData());
 
     TransitiveInfoCollection launcher = JavaHelper.launcherForTarget(semantics, ruleContext);
     if (launcher != null) {
-      runfilesBuilder.addTarget(launcher, RunfilesProvider.DATA_RUNFILES);
+      runfilesBuilder.addTarget(
+          launcher,
+          RunfilesProvider.DATA_RUNFILES,
+          ruleContext.getConfiguration().alwaysIncludeFilesToBuildInData());
     }
 
     semantics.addRunfilesForLibrary(ruleContext, runfilesBuilder);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaImport.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaImport.java
index a4d1d0556d3529..d3ed1d7f97a47f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaImport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaImport.java
@@ -140,7 +140,10 @@ public ConfiguredTarget create(RuleContext ruleContext)
                     ruleContext.getConfiguration().legacyExternalRunfiles())
                 // add the jars to the runfiles
                 .addArtifacts(javaArtifacts.getRuntimeJars())
-                .addTargets(targets, RunfilesProvider.DEFAULT_RUNFILES)
+                .addTargets(
+                    targets,
+                    RunfilesProvider.DEFAULT_RUNFILES,
+                    ruleContext.getConfiguration().alwaysIncludeFilesToBuildInData())
                 .addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES)
                 .build();
 


Test patch:
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/TestSuite.java b/src/main/java/com/google/devtools/build/lib/rules/test/TestSuite.java
index 40eeb359852495..07ad56196be619 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/TestSuite.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/TestSuite.java
@@ -79,10 +79,15 @@ public ConfiguredTarget create(RuleContext ruleContext)
       directTestsAndSuitesBuilder.add(dep);
     }
 
-    Runfiles runfiles = new Runfiles.Builder(
-        ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles())
-        .addTargets(directTestsAndSuitesBuilder, RunfilesProvider.DATA_RUNFILES)
-        .build();
+    Runfiles runfiles =
+        new Runfiles.Builder(
+                ruleContext.getWorkspaceName(),
+                ruleContext.getConfiguration().legacyExternalRunfiles())
+            .addTargets(
+                directTestsAndSuitesBuilder,
+                RunfilesProvider.DATA_RUNFILES,
+                ruleContext.getConfiguration().alwaysIncludeFilesToBuildInData())
+            .build();
 
     return new RuleConfiguredTargetBuilder(ruleContext)
         .add(RunfilesProvider.class,
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java
index 8ba84526fd64b9..0fad5fa2d6cb1e 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java
@@ -713,6 +713,134 @@ public void testDefaultInfoWithRunfilesConstructor() throws Exception {
     assertThat(getConfiguredTarget("//src:r_tools")).isNotNull();
   }
 
+  @Test
+  public void testDefaultInfoFilesAddedToCcBinaryTargetRunfiles() throws Exception {
+    scratch.file(
+        "test/starlark/extension.bzl",
+        "def custom_rule_impl(ctx):",
+        "  out = ctx.actions.declare_file(ctx.attr.name + '.out')",
+        "  ctx.actions.write(out, 'foobar')",
+        "  return [DefaultInfo(files = depset([out]))]",
+        "",
+        "custom_rule = rule(implementation = custom_rule_impl)");
+
+    scratch.file(
+        "test/starlark/BUILD",
+        "load('//test/starlark:extension.bzl', 'custom_rule')",
+        "",
+        "custom_rule(name = 'cr')",
+        "cc_binary(name = 'binary', data = [':cr'])");
+
+    useConfiguration("--incompatible_always_include_files_in_data");
+    ConfiguredTarget target = getConfiguredTarget("//test/starlark:binary");
+
+    assertThat(target.getLabel().toString()).isEqualTo("//test/starlark:binary");
+    assertThat(
+            ActionsTestUtil.baseArtifactNames(
+                target.getProvider(RunfilesProvider.class).getDefaultRunfiles().getAllArtifacts()))
+        .contains("cr.out");
+    assertThat(
+            ActionsTestUtil.baseArtifactNames(
+                target.getProvider(RunfilesProvider.class).getDataRunfiles().getAllArtifacts()))
+        .contains("cr.out");
+  }
+
+  @Test
+  public void testDefaultInfoFilesAddedToJavaBinaryTargetRunfiles() throws Exception {
+    scratch.file(
+        "test/starlark/extension.bzl",
+        "def custom_rule_impl(ctx):",
+        "  out = ctx.actions.declare_file(ctx.attr.name + '.out')",
+        "  ctx.actions.write(out, 'foobar')",
+        "  return [DefaultInfo(files = depset([out]))]",
+        "",
+        "custom_rule = rule(implementation = custom_rule_impl)");
+
+    scratch.file(
+        "test/starlark/BUILD",
+        "load('//test/starlark:extension.bzl', 'custom_rule')",
+        "",
+        "custom_rule(name = 'cr')",
+        "java_binary(name = 'binary', data = [':cr'], srcs = ['Foo.java'], main_class = 'Foo')");
+
+    useConfiguration("--incompatible_always_include_files_in_data");
+    ConfiguredTarget target = getConfiguredTarget("//test/starlark:binary");
+
+    assertThat(target.getLabel().toString()).isEqualTo("//test/starlark:binary");
+    assertThat(
+            ActionsTestUtil.baseArtifactNames(
+                target.getProvider(RunfilesProvider.class).getDefaultRunfiles().getAllArtifacts()))
+        .contains("cr.out");
+    assertThat(
+            ActionsTestUtil.baseArtifactNames(
+                target.getProvider(RunfilesProvider.class).getDataRunfiles().getAllArtifacts()))
+        .contains("cr.out");
+  }
+
+  @Test
+  public void testDefaultInfoFilesAddedToPyBinaryTargetRunfiles() throws Exception {
+    scratch.file(
+        "test/starlark/extension.bzl",
+        "def custom_rule_impl(ctx):",
+        "  out = ctx.actions.declare_file(ctx.attr.name + '.out')",
+        "  ctx.actions.write(out, 'foobar')",
+        "  return [DefaultInfo(files = depset([out]))]",
+        "",
+        "custom_rule = rule(implementation = custom_rule_impl)");
+
+    scratch.file(
+        "test/starlark/BUILD",
+        "load('//test/starlark:extension.bzl', 'custom_rule')",
+        "",
+        "custom_rule(name = 'cr')",
+        "py_binary(name = 'binary', data = [':cr'], srcs = ['binary.py'])");
+
+    useConfiguration("--incompatible_always_include_files_in_data");
+    ConfiguredTarget target = getConfiguredTarget("//test/starlark:binary");
+
+    assertThat(target.getLabel().toString()).isEqualTo("//test/starlark:binary");
+    assertThat(
+            ActionsTestUtil.baseArtifactNames(
+                target.getProvider(RunfilesProvider.class).getDefaultRunfiles().getAllArtifacts()))
+        .contains("cr.out");
+    assertThat(
+            ActionsTestUtil.baseArtifactNames(
+                target.getProvider(RunfilesProvider.class).getDataRunfiles().getAllArtifacts()))
+        .contains("cr.out");
+  }
+
+  @Test
+  public void testDefaultInfoFilesAddedToShBinaryTargetRunfiles() throws Exception {
+    scratch.file(
+        "test/starlark/extension.bzl",
+        "def custom_rule_impl(ctx):",
+        "  out = ctx.actions.declare_file(ctx.attr.name + '.out')",
+        "  ctx.actions.write(out, 'foobar')",
+        "  return [DefaultInfo(files = depset([out]))]",
+        "",
+        "custom_rule = rule(implementation = custom_rule_impl)");
+
+    scratch.file(
+        "test/starlark/BUILD",
+        "load('//test/starlark:extension.bzl', 'custom_rule')",
+        "",
+        "custom_rule(name = 'cr')",
+        "sh_binary(name = 'binary', data = [':cr'], srcs = ['script.sh'])");
+
+    useConfiguration("--incompatible_always_include_files_in_data");
+    ConfiguredTarget target = getConfiguredTarget("//test/starlark:binary");
+
+    assertThat(target.getLabel().toString()).isEqualTo("//test/starlark:binary");
+    assertThat(
+            ActionsTestUtil.baseArtifactNames(
+                target.getProvider(RunfilesProvider.class).getDefaultRunfiles().getAllArtifacts()))
+        .contains("cr.out");
+    assertThat(
+            ActionsTestUtil.baseArtifactNames(
+                target.getProvider(RunfilesProvider.class).getDataRunfiles().getAllArtifacts()))
+        .contains("cr.out");
+  }
+
   @Test
   public void testInstrumentedFilesProviderWithCodeCoverageDisabled() throws Exception {
     setBuildLanguageOptions("--incompatible_disallow_struct_provider_syntax=false");