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 17367979f8ad7c..61aeafc118a870 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 @@ The list of source (.py) 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 + --incompatible_remove_legacy_create_init is used. If false, the user is responsible for creating (possibly empty) __init__.py files and adding them to the srcs of Python targets as required. */ - .add(attr("legacy_create_init", BOOLEAN).value(true)) + .add(attr("legacy_create_init", TRISTATE).value(TriState.AUTO)) /* 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 db957f730f5d30..7c7be40508c15b 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 @@ public ConfiguredTarget create(RuleContext ruleContext) .build(); } + /** + * If requested, creates empty __init__.py files for each manifest file. + * + *

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. + * + *

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(); } 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 4a65e7aec86c46..1a5feb0a8eb2e1 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 @@ 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; + } } 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 67bc0735658617..a898e2cb426098 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 @@ public PythonConfiguration create(BuildOptions buildOptions) /*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 ad094464859536..aca45d8490ed53 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 @@ 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 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 7fbe0e1982ac0e..65f899553426e6 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 @@ 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(); + } }