Add 'incompatible_default_to_explicit_init_py' flag

This effectively changes the default value of legacy_create_init to false.

Technically, the attribute type has changed from boolean (default true) to tristate (default auto). Therefore, it is possible to see a semantic change in code that introspects a py_binary's or py_test's attrs dict (via native.existing_rules). We think it is unlikely this will break anyone, and in any case it is not currently possible to gate a rule's definition on an incompatible change flag.

Closes #9271. Work toward #7386 and #10076.

RELNOTES: [Python] Added flag --incomaptible_default_to_explicit_init_py to switch the default value of legacy_create_init to True. With this flag enabled, your py_binary and py_test targets will no longer behave as if empty __init__.py files were implicitly littered in your runfiles tree. See [#10076](#10076).
PiperOrigin-RevId: 276526057

src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java

@@ -19,7 +19,6 @@
 import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
 import static com.google.devtools.build.lib.packages.BuildType.NODEP_LABEL;
 import static com.google.devtools.build.lib.packages.BuildType.TRISTATE;
-import static com.google.devtools.build.lib.packages.Type.BOOLEAN;
 import static com.google.devtools.build.lib.packages.Type.STRING;
 import static com.google.devtools.build.lib.packages.Type.STRING_LIST;
 
@@ -229,11 +228,12 @@ The list of source (<code>.py</code>) files that are processed to create the tar
           Whether to implicitly create empty __init__.py files in the runfiles tree.
           These are created in every directory containing Python source code or
           shared libraries, and every parent directory of those directories, excluding the repo root
-          directory. The default is true for backward compatibility. If false, the user is
+          directory. The default, auto, means true unless
+          <code>--incompatible_remove_legacy_create_init</code> is used. If false, the user is
           responsible for creating (possibly empty) __init__.py files and adding them to the
           <code>srcs</code> of Python targets as required.
           <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-          .add(attr("legacy_create_init", BOOLEAN).value(true))
+          .add(attr("legacy_create_init", TRISTATE).value(TriState.AUTO))
           /* <!-- #BLAZE_RULE($base_py_binary).ATTRIBUTE(stamp) -->
           Enable link stamping.
           Whether to encode build information into the binary. Possible values:

src/main/java/com/google/devtools/build/lib/rules/python/PyExecutable.java

@@ -24,7 +24,8 @@
 import com.google.devtools.build.lib.analysis.RunfilesProvider;
 import com.google.devtools.build.lib.analysis.RunfilesSupport;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
-import com.google.devtools.build.lib.packages.Type;
+import com.google.devtools.build.lib.packages.BuildType;
+import com.google.devtools.build.lib.packages.TriState;
 import com.google.devtools.build.lib.rules.cpp.CcCommon.CcFlagsSupplier;
 import com.google.devtools.build.lib.rules.cpp.CcInfo;
 import java.util.ArrayList;
@@ -117,6 +118,31 @@ public ConfiguredTarget create(RuleContext ruleContext)
         .build();
   }
 
+  /**
+   * If requested, creates empty __init__.py files for each manifest file.
+   *
+   * <p>We do this if the rule defines {@code legacy_create_init} and its value is true. Auto is
+   * treated as false iff {@code --incompatible_remove_legacy_Create_init_py} is given.
+   *
+   * <p>See {@link PythonUtils#getInitPyFiles} for details about how the files are created.
+   */
+  private static void maybeCreateInitFiles(RuleContext ruleContext, Runfiles.Builder builder) {
+    boolean createFiles;
+    if (!ruleContext.attributes().has("legacy_create_init", BuildType.TRISTATE)) {
+      createFiles = true;
+    } else {
+      TriState legacy = ruleContext.attributes().get("legacy_create_init", BuildType.TRISTATE);
+      if (legacy == TriState.AUTO) {
+        createFiles = !ruleContext.getFragment(PythonConfiguration.class).defaultToExplicitInitPy();
+      } else {
+        createFiles = legacy != TriState.NO;
+      }
+    }
+    if (createFiles) {
+      builder.setEmptyFilesSupplier(PythonUtils.GET_INIT_PY_FILES);
+    }
+  }
+
   private static Runfiles collectCommonRunfiles(
       RuleContext ruleContext, PyCommon common, PythonSemantics semantics, CcInfo ccInfo)
       throws InterruptedException, RuleErrorException {
@@ -131,10 +157,8 @@ private static Runfiles collectCommonRunfiles(
     semantics.collectDefaultRunfiles(ruleContext, builder);
     builder.add(ruleContext, PythonRunfilesProvider.TO_RUNFILES);
 
-    if (!ruleContext.attributes().has("legacy_create_init", Type.BOOLEAN)
-        || ruleContext.attributes().get("legacy_create_init", Type.BOOLEAN)) {
-      builder.setEmptyFilesSupplier(PythonUtils.GET_INIT_PY_FILES);
-    }
+    maybeCreateInitFiles(ruleContext, builder);
+
     semantics.collectRunfilesForBinary(ruleContext, builder, common, ccInfo);
     return builder.build();
   }

src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java

@@ -58,6 +58,8 @@ public class PythonConfiguration extends BuildConfiguration.Fragment {
   // TODO(brandjon): Remove this once migration for native rule access is complete.
   private final boolean loadPythonRulesFromBzl;
 
+  private final boolean defaultToExplicitInitPy;
+
   PythonConfiguration(
       PythonVersion version,
       PythonVersion defaultVersion,
@@ -68,7 +70,8 @@ public class PythonConfiguration extends BuildConfiguration.Fragment {
       boolean py2OutputsAreSuffixed,
       boolean disallowLegacyPyProvider,
       boolean useToolchains,
-      boolean loadPythonRulesFromBzl) {
+      boolean loadPythonRulesFromBzl,
+      boolean defaultToExplicitInitPy) {
     this.version = version;
     this.defaultVersion = defaultVersion;
     this.buildPythonZip = buildPythonZip;
@@ -79,6 +82,7 @@ public class PythonConfiguration extends BuildConfiguration.Fragment {
     this.disallowLegacyPyProvider = disallowLegacyPyProvider;
     this.useToolchains = useToolchains;
     this.loadPythonRulesFromBzl = loadPythonRulesFromBzl;
+    this.defaultToExplicitInitPy = defaultToExplicitInitPy;
   }
 
   /**
@@ -204,4 +208,12 @@ public boolean useToolchains() {
   public boolean loadPythonRulesFromBzl() {
     return loadPythonRulesFromBzl;
   }
+
+  /**
+   * Returns true if executable Python rules should only write out empty __init__ files to their
+   * runfiles tree when explicitly requested via {@code legacy_create_init}.
+   */
+  public boolean defaultToExplicitInitPy() {
+    return defaultToExplicitInitPy;
+  }
 }

src/main/java/com/google/devtools/build/lib/rules/python/PythonConfigurationLoader.java

@@ -44,7 +44,8 @@ public PythonConfiguration create(BuildOptions buildOptions)
         /*py2OutputsAreSuffixed=*/ pythonOptions.incompatiblePy2OutputsAreSuffixed,
         /*disallowLegacyPyProvider=*/ pythonOptions.incompatibleDisallowLegacyPyProvider,
         /*useToolchains=*/ pythonOptions.incompatibleUsePythonToolchains,
-        /*loadPythonRulesFromBzl=*/ pythonOptions.loadPythonRulesFromBzl);
+        /*loadPythonRulesFromBzl=*/ pythonOptions.loadPythonRulesFromBzl,
+        /*defaultToExplicitInitPy=*/ pythonOptions.incompatibleDefaultToExplicitInitPy);
   }
 
   @Override

src/main/java/com/google/devtools/build/lib/rules/python/PythonOptions.java

@@ -269,6 +269,23 @@ public String getTypeDescription() {
               + "data runfiles of another binary.")
   public boolean buildTransitiveRunfilesTrees;
 
+  @Option(
+      name = "incompatible_default_to_explicit_init_py",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.GENERIC_INPUTS,
+      effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
+      metadataTags = {
+        OptionMetadataTag.INCOMPATIBLE_CHANGE,
+        OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+      },
+      help =
+          "This flag changes the default behavior so that __init__.py files are no longer "
+              + "automatically created in the runfiles of Python targets. Precisely, when a "
+              + "py_binary or py_test target has legacy_create_init set to \"auto\" (the default), "
+              + "it is treated as false if and only if this flag is set. See "
+              + "https://github.com/bazelbuild/bazel/issues/10076.")
+  public boolean incompatibleDefaultToExplicitInitPy;
+
   @Override
   public Map<OptionDefinition, SelectRestriction> getSelectRestrictions() {
     // TODO(brandjon): Instead of referencing the python_version target, whose path depends on the

src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyBinaryConfiguredTargetTest.java

@@ -40,6 +40,10 @@ public class BazelPyBinaryConfiguredTargetTest extends BuildViewTestCase {
   private static final String TOOLCHAIN_TYPE =
       TestConstants.TOOLS_REPOSITORY + "//tools/python:toolchain_type";
 
+  private static String join(String... lines) {
+    return String.join("\n", lines);
+  }
+
   /**
    * Given a {@code py_binary} or {@code py_test} target, returns the path of the Python interpreter
    * used by the generated stub script.
@@ -281,7 +285,7 @@ private void defineCustomToolchain(String... lines) throws Exception {
     if (lines.length == 0) {
       indentedBody = "    pass";
     } else {
-      indentedBody = "    " + String.join("\n", lines).replace("\n", "\n    ");
+      indentedBody = "    " + join(lines).replace("\n", "\n    ");
     }
     scratch.file(
         "toolchains/rules.bzl",
@@ -372,4 +376,60 @@ public void toolchainInfoFieldHasBadVersion() throws Exception {
         "Error retrieving the Python runtime from the toolchain: Expected field 'py3_runtime' to "
             + "have a runtime with python_version = 'PY3', but got python_version = 'PY2'");
   }
+
+  @Test
+  public void explicitInitPy_CanBeGloballyEnabled() throws Exception {
+    scratch.file(
+        "pkg/BUILD",
+        join(
+            "py_binary(", //
+            "    name = 'foo',",
+            "    srcs = ['foo.py'],",
+            ")"));
+    useConfiguration("--incompatible_default_to_explicit_init_py=true");
+    assertThat(getDefaultRunfiles(getConfiguredTarget("//pkg:foo")).getEmptyFilenames()).isEmpty();
+  }
+
+  @Test
+  public void explicitInitPy_CanBeSelectivelyDisabled() throws Exception {
+    scratch.file(
+        "pkg/BUILD",
+        join(
+            "py_binary(", //
+            "    name = 'foo',",
+            "    srcs = ['foo.py'],",
+            "    legacy_create_init = True,",
+            ")"));
+    useConfiguration("--incompatible_default_to_explicit_init_py=true");
+    assertThat(getDefaultRunfiles(getConfiguredTarget("//pkg:foo")).getEmptyFilenames())
+        .containsExactly("pkg/__init__.py");
+  }
+
+  @Test
+  public void explicitInitPy_CanBeGloballyDisabled() throws Exception {
+    scratch.file(
+        "pkg/BUILD",
+        join(
+            "py_binary(", //
+            "    name = 'foo',",
+            "    srcs = ['foo.py'],",
+            ")"));
+    useConfiguration("--incompatible_default_to_explicit_init_py=false");
+    assertThat(getDefaultRunfiles(getConfiguredTarget("//pkg:foo")).getEmptyFilenames())
+        .containsExactly("pkg/__init__.py");
+  }
+
+  @Test
+  public void explicitInitPy_CanBeSelectivelyEnabled() throws Exception {
+    scratch.file(
+        "pkg/BUILD",
+        join(
+            "py_binary(", //
+            "    name = 'foo',",
+            "    srcs = ['foo.py'],",
+            "    legacy_create_init = False,",
+            ")"));
+    useConfiguration("--incompatible_default_to_explicit_init_py=false");
+    assertThat(getDefaultRunfiles(getConfiguredTarget("//pkg:foo")).getEmptyFilenames()).isEmpty();
+  }
 }