PythonUtils.GetInitPyFiles gains a Predicate<PathFragment> supplied at construction time so that other implementations can supply their own logic for determining when a file is a Python package defining __init__.py equivalent, meaning none need be created.
Adds PythonSemantics.getEmptyRunFilesSupplier() and moves the static PythonUtils.GET_INIT_PY_FILES into the BazelPythonSemantics implementation in favor of PyExecutable calling the abstract getEmptyRunfilesSupplier() so that alternate PythonSemantics implementations can supply their own behavior.
PiperOrigin-RevId: 317142545
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java
index 14f1dce..70db056 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java
@@ -49,18 +49,23 @@
import com.google.devtools.build.lib.rules.python.PyRuntimeInfo;
import com.google.devtools.build.lib.rules.python.PythonConfiguration;
import com.google.devtools.build.lib.rules.python.PythonSemantics;
+import com.google.devtools.build.lib.rules.python.PythonUtils;
import com.google.devtools.build.lib.rules.python.PythonVersion;
import com.google.devtools.build.lib.util.FileTypeSet;
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.vfs.PathFragment;
+import java.io.Serializable;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
+import java.util.function.Predicate;
import javax.annotation.Nullable;
/** Functionality specific to the Python rules in Bazel. */
public class BazelPythonSemantics implements PythonSemantics {
+ public static final Runfiles.EmptyFilesSupplier GET_INIT_PY_FILES =
+ new PythonUtils.GetInitPyFiles((Predicate<PathFragment> & Serializable) source -> false);
private static final Template STUB_TEMPLATE =
Template.forResource(BazelPythonSemantics.class, "python_stub_template.txt");
public static final InstrumentationSpec PYTHON_COLLECTION_SPEC = new InstrumentationSpec(
@@ -70,6 +75,11 @@
public static final PathFragment ZIP_RUNFILES_DIRECTORY_NAME = PathFragment.create("runfiles");
@Override
+ public Runfiles.EmptyFilesSupplier getEmptyRunfilesSupplier() {
+ return GET_INIT_PY_FILES;
+ }
+
+ @Override
public String getSrcsVersionDocURL() {
// TODO(#8996): Update URL to point to rules_python's docs instead of the Bazel site.
return "https://docs.bazel.build/versions/master/be/python.html#py_binary.srcs_version";
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 f0f6ad2..bb47f6b 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
@@ -126,7 +126,8 @@
*
* <p>See {@link PythonUtils#getInitPyFiles} for details about how the files are created.
*/
- private static void maybeCreateInitFiles(RuleContext ruleContext, Runfiles.Builder builder) {
+ private static void maybeCreateInitFiles(
+ RuleContext ruleContext, Runfiles.Builder builder, PythonSemantics semantics) {
boolean createFiles;
if (!ruleContext.attributes().has("legacy_create_init", BuildType.TRISTATE)) {
createFiles = true;
@@ -139,7 +140,7 @@
}
}
if (createFiles) {
- builder.setEmptyFilesSupplier(PythonUtils.GET_INIT_PY_FILES);
+ builder.setEmptyFilesSupplier(semantics.getEmptyRunfilesSupplier());
}
}
@@ -157,7 +158,7 @@
semantics.collectDefaultRunfiles(ruleContext, builder);
builder.add(ruleContext, PythonRunfilesProvider.TO_RUNFILES);
- maybeCreateInitFiles(ruleContext, builder);
+ maybeCreateInitFiles(ruleContext, builder, semantics);
semantics.collectRunfilesForBinary(ruleContext, builder, common, ccInfo);
return builder.build();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PythonSemantics.java b/src/main/java/com/google/devtools/build/lib/rules/python/PythonSemantics.java
index 4225a4c..5a055ce 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PythonSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PythonSemantics.java
@@ -93,4 +93,12 @@
throws InterruptedException, RuleErrorException;
CcInfo buildCcInfoProvider(Iterable<? extends TransitiveInfoCollection> deps);
+
+ /**
+ * Called when building executables or packages to fill in missing empty __init__.py files if the
+ * --incompatible_default_to_explicit_init_py has not yet been enabled. This usually returns a
+ * public static final reference, code is free to use that directly on specific implementations
+ * instead of making this call.
+ */
+ Runfiles.EmptyFilesSupplier getEmptyRunfilesSupplier();
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PythonUtils.java b/src/main/java/com/google/devtools/build/lib/rules/python/PythonUtils.java
index 4a80c32..49a9415 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PythonUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PythonUtils.java
@@ -22,18 +22,16 @@
import com.google.devtools.build.lib.analysis.TransitionMode;
import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
import com.google.devtools.build.lib.analysis.actions.SpawnAction;
-import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.util.FileType;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
+import java.util.function.Predicate;
import javax.annotation.Nullable;
-/**
- * Various utility methods for Python support.
- */
+/** Various utility methods for Python support. */
public final class PythonUtils {
public static final PathFragment INIT_PY = PathFragment.create("__init__.py");
public static final PathFragment INIT_PYC = PathFragment.create("__init__.pyc");
@@ -41,48 +39,75 @@
private static final FileType REQUIRES_INIT_PY = FileType.of(".py", ".so", ".pyc");
- static class GetInitPyFiles implements Runfiles.EmptyFilesSupplier {
+ /**
+ * Used to get the set of empty __init__.py files to be added to a given set of files to allow the
+ * Python runtime to import subdirectories potentially containing Python code to be imported as
+ * packages. Ideally this feature goes away with --incompatible_default_to_explicit_init_py as the
+ * long term default behavior.
+ */
+ public static class GetInitPyFiles implements Runfiles.EmptyFilesSupplier {
+ private final Predicate<PathFragment> isPackageInit;
+
+ /**
+ * The Predicate isPackageInit's .test(source) should be true when a given source is known to be
+ * a valid __init__.py file equivalent, meaning no empty __init__.py file need be created.
+ * Useful for custom Python runtimes that may have non-standard Python package import logic.
+ */
+ public GetInitPyFiles(Predicate<PathFragment> isPackageInit) {
+ this.isPackageInit = isPackageInit;
+ }
+
@Override
- public Iterable<PathFragment> getExtraPaths(Set<PathFragment> manifestPaths) {
+ public Set<PathFragment> getExtraPaths(Set<PathFragment> manifestPaths) {
return getInitPyFiles(manifestPaths);
}
- }
- @AutoCodec
- public static final Runfiles.EmptyFilesSupplier GET_INIT_PY_FILES = new GetInitPyFiles();
+ /**
+ * Returns the set of empty __init__.py(c) files to be added to a given set of files to allow
+ * the Python runtime to find the <code>.py</code> and <code>.so</code> files present in the
+ * tree.
+ */
+ private ImmutableSet<PathFragment> getInitPyFiles(Set<PathFragment> manifestFiles) {
+ Set<PathFragment> result = new HashSet<>();
+ // A set of directories that already have package init files.
+ Set<PathFragment> hasPackageInitDirs = new HashSet<>(); // For b/142135992.
- private PythonUtils() {
- // This is a utility class, not to be instantiated
- }
+ // Find directories containing Python package init files based on a caller supplied test in
+ // order to support non-standard Python package init naming schemes.
+ // This loop is done prior to the one below as we assume no order in the set and that we may
+ // find inits in parent directories listed after subdirectories which the nested loop below
+ // would need to know of.
+ for (PathFragment source : manifestFiles) {
+ if (isPackageInit.test(source)) {
+ hasPackageInitDirs.add(source.getParentDirectory());
+ }
+ }
- /**
- * Returns the set of empty __init__.py(c) files to be added to a given set of files to allow
- * the Python runtime to find the <code>.py</code> and <code>.so</code> files present in the
- * tree.
- */
- public static Set<PathFragment> getInitPyFiles(Set<PathFragment> manifestFiles) {
- Set<PathFragment> result = new HashSet<>();
+ for (PathFragment source : manifestFiles) {
+ // If we have a python or .so file at this level...
+ if (REQUIRES_INIT_PY.matches(source)) {
+ // ...then record that we need an __init__.py in this and all parents directories...
+ while (source.segmentCount() > 1) {
+ source = source.getParentDirectory();
+ // ...unless it's a Python .pyc cache or we already have __init__ there.
+ if (!source.endsWith(PYCACHE) && !hasPackageInitDirs.contains(source)) {
+ PathFragment initpy = source.getRelative(INIT_PY);
+ PathFragment initpyc = source.getRelative(INIT_PYC);
- for (PathFragment source : manifestFiles) {
- // If we have a python or .so file at this level...
- if (REQUIRES_INIT_PY.matches(source)) {
- // ...then record that we need an __init__.py in this and all parents directories...
- while (source.segmentCount() > 1) {
- source = source.getParentDirectory();
- // ...unless it's a Python .pyc cache or we already have __init__ there.
- if (!source.endsWith(PYCACHE)) {
- PathFragment initpy = source.getRelative(INIT_PY);
- PathFragment initpyc = source.getRelative(INIT_PYC);
-
- if (!manifestFiles.contains(initpy) && !manifestFiles.contains(initpyc)) {
- result.add(initpy);
+ if (!manifestFiles.contains(initpy) && !manifestFiles.contains(initpyc)) {
+ result.add(initpy);
+ }
}
}
}
}
- }
- return ImmutableSet.copyOf(result);
+ return ImmutableSet.copyOf(result);
+ }
+ } // class GetInitPyFiles
+
+ private PythonUtils() {
+ // This is a utility class, not to be instantiated.
}
/**
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/SourceManifestActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/SourceManifestActionTest.java
index d90ef90..96fa7dc 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/SourceManifestActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/SourceManifestActionTest.java
@@ -24,7 +24,6 @@
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.analysis.SourceManifestAction.ManifestType;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
-import com.google.devtools.build.lib.rules.python.PythonUtils;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -106,7 +105,8 @@
Runfiles.Builder builder = new Runfiles.Builder("TESTING", false);
builder.addSymlinks(fakeManifest);
if (addInitPy) {
- builder.setEmptyFilesSupplier(PythonUtils.GET_INIT_PY_FILES);
+ builder.setEmptyFilesSupplier(
+ analysisMock.pySupport().getPythonSemantics().getEmptyRunfilesSupplier());
}
return new SourceManifestAction(type, NULL_ACTION_OWNER, manifestOutputFile, builder.build());
}
@@ -313,16 +313,6 @@
assertThat(computeKey(action2)).isNotEqualTo(computeKey(action1));
}
- /**
- * Constructs a new manifest file artifact with the given name, writes the given contents
- * to that file, and returns the artifact.
- */
- private Artifact manifestFile(String name, String... lines) throws Exception {
- Artifact artifact = getBinArtifactWithNoOwner(name);
- scratch.file(artifact.getPath().getPathString(), lines);
- return artifact;
- }
-
private String computeKey(SourceManifestAction action) {
Fingerprint fp = new Fingerprint();
action.computeKey(actionKeyContext, fp);
diff --git a/src/test/java/com/google/devtools/build/lib/packages/BUILD b/src/test/java/com/google/devtools/build/lib/packages/BUILD
index ec361e4..7382797 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/packages/BUILD
@@ -132,6 +132,7 @@
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
"//src/main/java/com/google/devtools/build/lib/analysis:server_directories",
+ "//src/main/java/com/google/devtools/build/lib/bazel/rules/python",
"//src/main/java/com/google/devtools/build/lib/clock",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/events",
@@ -142,6 +143,7 @@
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
"//src/main/java/com/google/devtools/build/lib/rules/cpp",
"//src/main/java/com/google/devtools/build/lib/rules/proto",
+ "//src/main/java/com/google/devtools/build/lib/rules/python",
"//src/main/java/com/google/devtools/build/lib/runtime/commands",
"//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster",
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockPythonSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockPythonSupport.java
index d96b9f2..3339c70 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockPythonSupport.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockPythonSupport.java
@@ -14,6 +14,8 @@
package com.google.devtools.build.lib.packages.util;
+import com.google.devtools.build.lib.bazel.rules.python.BazelPythonSemantics;
+import com.google.devtools.build.lib.rules.python.PythonSemantics;
import com.google.devtools.build.lib.testutil.TestConstants;
import java.io.IOException;
@@ -91,4 +93,9 @@
// Under BazelPythonSemantics, we can simply set --python_top to be the py_runtime target.
return pyRuntimeLabel;
}
+
+ @Override
+ public PythonSemantics getPythonSemantics() {
+ return new BazelPythonSemantics();
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockPythonSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockPythonSupport.java
index 794ee28..7d1c973 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/MockPythonSupport.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockPythonSupport.java
@@ -17,6 +17,7 @@
import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.rules.python.PythonSemantics;
import com.google.devtools.build.lib.testutil.TestConstants;
import java.io.IOException;
import java.util.List;
@@ -37,6 +38,8 @@
public abstract String createPythonTopEntryPoint(MockToolsConfig config, String pyRuntimeLabel)
throws IOException;
+ public abstract PythonSemantics getPythonSemantics();
+
/**
* Defines a file simulating the part of @rules_python//python:defs.bzl that defines macros for
* native rules.
