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](https://github.com/bazelbuild/bazel/issues/10076).
PiperOrigin-RevId: 276526057
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java
index 1736797..61aeafc 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java
+++ b/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 @@
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:
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyExecutable.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyExecutable.java
index db957f7..7c7be40 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PyExecutable.java
+++ b/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 @@
.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 @@
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();
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java
index 4a65e7a..1a5feb0 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java
@@ -58,6 +58,8 @@
// 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 @@
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 @@
this.disallowLegacyPyProvider = disallowLegacyPyProvider;
this.useToolchains = useToolchains;
this.loadPythonRulesFromBzl = loadPythonRulesFromBzl;
+ this.defaultToExplicitInitPy = defaultToExplicitInitPy;
}
/**
@@ -204,4 +208,12 @@
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;
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfigurationLoader.java
index 67bc073..a898e2c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfigurationLoader.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfigurationLoader.java
@@ -44,7 +44,8 @@
/*py2OutputsAreSuffixed=*/ pythonOptions.incompatiblePy2OutputsAreSuffixed,
/*disallowLegacyPyProvider=*/ pythonOptions.incompatibleDisallowLegacyPyProvider,
/*useToolchains=*/ pythonOptions.incompatibleUsePythonToolchains,
- /*loadPythonRulesFromBzl=*/ pythonOptions.loadPythonRulesFromBzl);
+ /*loadPythonRulesFromBzl=*/ pythonOptions.loadPythonRulesFromBzl,
+ /*defaultToExplicitInitPy=*/ pythonOptions.incompatibleDefaultToExplicitInitPy);
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PythonOptions.java b/src/main/java/com/google/devtools/build/lib/rules/python/PythonOptions.java
index ad09446..aca45d8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PythonOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PythonOptions.java
@@ -269,6 +269,23 @@
+ "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
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyBinaryConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyBinaryConfiguredTargetTest.java
index 7fbe0e1..65f8995 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyBinaryConfiguredTargetTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyBinaryConfiguredTargetTest.java
@@ -40,6 +40,10 @@
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 @@
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 @@
"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();
+ }
}
