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.