Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable custom py_binary stub_template attribute (via py_runtime) #16806

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/rules/cpp",
"//src/main/java/com/google/devtools/build/lib/rules/python",
"//src/main/java/com/google/devtools/build/lib/util:filetype",
"//src/main/java/com/google/devtools/build/lib/util:os",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/common/options",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
import com.google.devtools.build.lib.rules.python.PyInfo;
import com.google.devtools.build.lib.rules.python.PyRuleClasses;
import com.google.devtools.build.lib.rules.python.PythonVersion;
import com.google.devtools.build.lib.rules.python.PyRuntimeInfo;
import com.google.devtools.build.lib.util.FileTypeSet;

/**
* Bazel-specific rule definitions for Python rules.
Expand Down Expand Up @@ -221,6 +223,11 @@ responsible for creating (possibly empty) __init__.py files and adding them to t
.add(
attr("$py_toolchain_type", NODEP_LABEL)
.value(env.getToolsLabel("//tools/python:toolchain_type")))
/* Only used when no py_runtime() is available. See #7901
*/
.add(
attr("$default_bootstrap_template", LABEL)
.value(env.getToolsLabel(PyRuntimeInfo.DEFAULT_BOOTSTRAP_TEMPLATE)))
.addToolchainTypes(
ToolchainTypeRequirement.builder(env.getToolsLabel("//tools/python:toolchain_type"))
.mandatory(true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.google.devtools.build.lib.actions.CommandLineItem;
import com.google.devtools.build.lib.actions.ParamFileInfo;
import com.google.devtools.build.lib.actions.ParameterFile;
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
import com.google.devtools.build.lib.analysis.AnalysisUtils;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
Expand Down Expand Up @@ -62,8 +63,6 @@ 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 PathFragment ZIP_RUNFILES_DIRECTORY_NAME = PathFragment.create("runfiles");

Expand Down Expand Up @@ -166,12 +165,14 @@ private static void createStubFile(
attrVersion = config.getDefaultPythonVersion();
}

Artifact bootstrapTemplate = getBootstrapTemplate(ruleContext, common);

// Create the stub file.
ruleContext.registerAction(
new TemplateExpansionAction(
ruleContext.getActionOwner(),
bootstrapTemplate,
stubOutput,
STUB_TEMPLATE,
ImmutableList.of(
Substitution.of("%shebang%", getStubShebang(ruleContext, common)),
Substitution.of("%main%", common.determineMainExecutableSource()),
Expand Down Expand Up @@ -337,7 +338,7 @@ private static String getZipRunfilesPath(
}
// We put the whole runfiles tree under the ZIP_RUNFILES_DIRECTORY_NAME directory, by doing this
// , we avoid the conflict between default workspace name "__main__" and __main__.py file.
// Note: This name has to be the same with the one in python_stub_template.txt.
// Note: This name has to be the same with the one in python_bootstrap_template.txt.
return ZIP_RUNFILES_DIRECTORY_NAME.getRelative(zipRunfilesPath).toString();
}

Expand Down Expand Up @@ -427,6 +428,17 @@ private static PyRuntimeInfo getRuntime(RuleContext ruleContext, PyCommon common
: ruleContext.getPrerequisite(":py_interpreter", PyRuntimeInfo.PROVIDER);
}

private static Artifact getBootstrapTemplate(RuleContext ruleContext, PyCommon common) {
PyRuntimeInfo provider = getRuntime(ruleContext, common);
if (provider != null) {
Artifact bootstrapTemplate = provider.getBootstrapTemplate();
if (bootstrapTemplate != null) {
return bootstrapTemplate;
}
}
return ruleContext.getPrerequisiteArtifact("$default_bootstrap_template");
}

private static void addRuntime(
RuleContext ruleContext, PyCommon common, Runfiles.Builder builder) {
PyRuntimeInfo provider = getRuntime(ruleContext, common);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,14 @@ public ConfiguredTarget create(RuleContext ruleContext)
return null;
}
Preconditions.checkState(pythonVersion.isTargetValue());
Artifact bootstrapTemplate = ruleContext.getPrerequisiteArtifact("bootstrap_template");

PyRuntimeInfo provider =
hermetic
? PyRuntimeInfo.createForInBuildRuntime(
interpreter, files, coverageTool, coverageFiles, pythonVersion, stubShebang)
interpreter, files, coverageTool, coverageFiles, pythonVersion, stubShebang, bootstrapTemplate)
: PyRuntimeInfo.createForPlatformRuntime(
interpreterPath, coverageTool, coverageFiles, pythonVersion, stubShebang);
interpreterPath, coverageTool, coverageFiles, pythonVersion, stubShebang, bootstrapTemplate);

return new RuleConfiguredTargetBuilder(ruleContext)
.setFilesToBuild(files)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ public final class PyRuntimeInfo implements Info, PyRuntimeInfoApi<Artifact> {
private final PythonVersion pythonVersion;

private final String stubShebang;
@Nullable private final Artifact bootstrapTemplate;
fahhem marked this conversation as resolved.
Show resolved Hide resolved

private PyRuntimeInfo(
@Nullable Location location,
Expand All @@ -71,7 +72,8 @@ private PyRuntimeInfo(
@Nullable Artifact coverageTool,
@Nullable Depset coverageFiles,
PythonVersion pythonVersion,
@Nullable String stubShebang) {
@Nullable String stubShebang,
@Nullable Artifact bootstrapTemplate) {
Preconditions.checkArgument((interpreterPath == null) != (interpreter == null));
Preconditions.checkArgument((interpreter == null) == (files == null));
Preconditions.checkArgument((coverageTool == null) == (coverageFiles == null));
Expand All @@ -88,6 +90,7 @@ private PyRuntimeInfo(
} else {
this.stubShebang = PyRuntimeInfoApi.DEFAULT_STUB_SHEBANG;
}
this.bootstrapTemplate = bootstrapTemplate;
}

@Override
Expand All @@ -107,7 +110,8 @@ public static PyRuntimeInfo createForInBuildRuntime(
@Nullable Artifact coverageTool,
@Nullable NestedSet<Artifact> coverageFiles,
PythonVersion pythonVersion,
@Nullable String stubShebang) {
@Nullable String stubShebang,
@Nullable Artifact bootstrapTemplate) {
return new PyRuntimeInfo(
/*location=*/ null,
/*interpreterPath=*/ null,
Expand All @@ -116,7 +120,8 @@ public static PyRuntimeInfo createForInBuildRuntime(
coverageTool,
coverageFiles == null ? null : Depset.of(Artifact.TYPE, coverageFiles),
pythonVersion,
stubShebang);
stubShebang,
bootstrapTemplate);
}

/** Constructs an instance from native rule logic (built-in location) for a platform runtime. */
Expand All @@ -125,7 +130,8 @@ public static PyRuntimeInfo createForPlatformRuntime(
@Nullable Artifact coverageTool,
@Nullable NestedSet<Artifact> coverageFiles,
PythonVersion pythonVersion,
@Nullable String stubShebang) {
@Nullable String stubShebang,
@Nullable Artifact bootstrapTemplate) {
return new PyRuntimeInfo(
/*location=*/ null,
interpreterPath,
Expand All @@ -134,7 +140,8 @@ public static PyRuntimeInfo createForPlatformRuntime(
coverageTool,
coverageFiles == null ? null : Depset.of(Artifact.TYPE, coverageFiles),
pythonVersion,
stubShebang);
stubShebang,
bootstrapTemplate);
}

@Override
Expand Down Expand Up @@ -202,6 +209,12 @@ public String getStubShebang() {
return stubShebang;
}

@Override
@Nullable
public Artifact getBootstrapTemplate() {
return bootstrapTemplate;
}

@Nullable
public NestedSet<Artifact> getFiles() {
try {
Expand Down Expand Up @@ -264,11 +277,16 @@ public PyRuntimeInfo constructor(
Object coverageFilesUncast,
String pythonVersion,
String stubShebang,
Object bootstrapTemplateUncast,
StarlarkThread thread)
throws EvalException {
String interpreterPath =
interpreterPathUncast == NONE ? null : (String) interpreterPathUncast;
Artifact interpreter = interpreterUncast == NONE ? null : (Artifact) interpreterUncast;
Artifact bootstrapTemplate = null;
if (bootstrapTemplateUncast != NONE) {
bootstrapTemplate = (Artifact) bootstrapTemplateUncast;
}
Depset filesDepset = null;
if (filesUncast != NONE) {
// Validate type of filesDepset.
Expand Down Expand Up @@ -312,7 +330,8 @@ public PyRuntimeInfo constructor(
coverageTool,
coverageDepset,
parsedPythonVersion,
stubShebang);
stubShebang,
bootstrapTemplate);
} else {
return new PyRuntimeInfo(
loc,
Expand All @@ -322,7 +341,8 @@ public PyRuntimeInfo constructor(
coverageTool,
coverageDepset,
parsedPythonVersion,
stubShebang);
stubShebang,
bootstrapTemplate);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ <p>The entry point for the tool must be loadable by a python interpreter (e.g. a
.allowedValues(PyRuleClasses.TARGET_PYTHON_ATTR_VALUE_SET))

/* <!-- #BLAZE_RULE(py_runtime).ATTRIBUTE(stub_shebang) -->
"Shebang" expression prepended to the bootstrapping Python stub script
"Shebang" expression prepended to the bootstrapping Python script
used when executing <code>py_binary</code> targets.

<p>See <a href="https://github.com/bazelbuild/bazel/issues/8685">issue 8685</a> for
Expand All @@ -93,6 +93,14 @@ <p>The entry point for the tool must be loadable by a python interpreter (e.g. a
<p>Does not apply to Windows.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(attr("stub_shebang", STRING).value(PyRuntimeInfo.DEFAULT_STUB_SHEBANG))

/* <!-- #BLAZE_RULE(py_runtime).ATTRIBUTE(bootstrap_template) -->
Previously referred to as the "Python stub script", this is the
entrypoint to every Python executable target.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(attr("bootstrap_template", LABEL)
.value(env.getToolsLabel(PyRuntimeInfo.DEFAULT_BOOTSTRAP_TEMPLATE))
.allowedFileTypes(FileTypeSet.ANY_FILE).singleArtifact())
.add(attr("output_licenses", LICENSE))
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import com.google.devtools.build.docgen.annot.DocCategory;
import com.google.devtools.build.docgen.annot.StarlarkConstructor;
import com.google.devtools.build.lib.collect.nestedset.Depset;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.starlarkbuildapi.FileApi;
import com.google.devtools.build.lib.starlarkbuildapi.core.ProviderApi;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -46,6 +47,8 @@
public interface PyRuntimeInfoApi<FileT extends FileApi> extends StarlarkValue {

static final String DEFAULT_STUB_SHEBANG = "#!/usr/bin/env python3";
// Must call getToolsLabel() when using this.
static final String DEFAULT_BOOTSTRAP_TEMPLATE = "//tools/python:python_bootstrap_template.txt";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this have "@bazel_tools"? Or is that implicit in this context for some reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it was, but it's not used here anymore so deleted it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually added it back, it's used (but via the subclass PyRuntimeInfo, that's why I didn't find it via rg). I added a comment that it should be used with getToolsLabel() to use it correctly


@StarlarkMethod(
name = "interpreter_path",
Expand Down Expand Up @@ -119,6 +122,15 @@ public interface PyRuntimeInfoApi<FileT extends FileApi> extends StarlarkValue {
+ "to Windows.")
String getStubShebang();

@StarlarkMethod(
name = "bootstrap_template",
structField = true,
doc =
"The stub script template file to use. Should have %python_binary%, "
+ "%workspace_name%, %main%, and %imports%. See "
+ "@bazel_tools//tools/python:python_bootstrap_template.txt for more variables.")
FileT getBootstrapTemplate();

/** Provider type for {@link PyRuntimeInfoApi} objects. */
@StarlarkBuiltin(name = "Provider", documented = false, doc = "")
interface PyRuntimeInfoProviderApi extends ProviderApi {
Expand Down Expand Up @@ -204,6 +216,16 @@ interface PyRuntimeInfoProviderApi extends ProviderApi {
+ "Default is <code>"
+ DEFAULT_STUB_SHEBANG
+ "</code>."),
@Param(
name = "bootstrap_template",
allowedTypes = {
@ParamType(type = FileApi.class),
@ParamType(type = NoneType.class),
},
positional = false,
named = true,
defaultValue = "None",
doc = ""),
},
useStarlarkThread = true,
selfCall = true)
Expand All @@ -216,6 +238,7 @@ PyRuntimeInfoApi<?> constructor(
Object coverageFilesUncast,
String pythonVersion,
String stubShebang,
Object bootstrapTemplate,
fahhem marked this conversation as resolved.
Show resolved Hide resolved
StarlarkThread thread)
throws EvalException;
}
Expand Down
1 change: 1 addition & 0 deletions src/main/starlark/builtins_bzl/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ genrule(
cmd = "$(location //src:zip_builtins)" +
" ''" + # system zip
" $@ src/main/starlark/builtins_bzl $(SRCS)",
message = "Building builtins_bzl.zip",
output_to_bindir = 1,
tools = ["//src:zip_builtins"],
visibility = [
Expand Down
10 changes: 9 additions & 1 deletion src/main/starlark/builtins_bzl/common/python/providers.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@
# limitations under the License.
"""Providers for Python rules."""

load(":common/python/semantics.bzl", "TOOLS_REPO")

DEFAULT_STUB_SHEBANG = "#!/usr/bin/env python3"
DEFAULT_BOOTSTRAP_TEMPLATE = "@" + TOOLS_REPO + "//tools/python:python_bootstrap_template.txt"
_PYTHON_VERSION_VALUES = ["PY2", "PY3"]

def _PyRuntimeInfo_init(
Expand All @@ -24,7 +27,8 @@ def _PyRuntimeInfo_init(
coverage_tool = None,
coverage_files = None,
python_version,
stub_shebang = None):
stub_shebang = None,
bootstrap_template = None):
if (interpreter_path == None) == (interpreter == None):
fail("exactly one of interpreter_path or interpreter must be set")
if (interpreter == None) != (files == None):
Expand All @@ -46,6 +50,7 @@ def _PyRuntimeInfo_init(
"coverage_files": coverage_files,
"python_version": python_version,
"stub_shebang": stub_shebang,
"bootstrap_template": bootstrap_template,
}

# TODO(#15897): Rename this to PyRuntimeInfo when we're ready to replace the Java
Expand Down Expand Up @@ -98,6 +103,9 @@ the same conventions as the standard CPython interpreter.
"script used when executing `py_binary` targets. Does not " +
"apply to Windows."
),
"bootstrap_template": (
"See py_runtime_rule.bzl%py_runtime.bootstrap_template for docs."
),
},
)

Expand Down
25 changes: 24 additions & 1 deletion src/main/starlark/builtins_bzl/common/python/py_runtime_rule.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"""Implementation of py_runtime rule."""

load(":common/paths.bzl", "paths")
load(":common/python/providers.bzl", "DEFAULT_STUB_SHEBANG", _PyRuntimeInfo = "PyRuntimeInfo")
load(":common/python/providers.bzl", "DEFAULT_STUB_SHEBANG", "DEFAULT_BOOTSTRAP_TEMPLATE", _PyRuntimeInfo = "PyRuntimeInfo")

_py_builtins = _builtins.internal.py_builtins

Expand Down Expand Up @@ -75,6 +75,7 @@ def _py_runtime_impl(ctx):
coverage_files = coverage_files,
python_version = python_version,
stub_shebang = ctx.attr.stub_shebang,
bootstrap_template = ctx.file.bootstrap_template,
),
DefaultInfo(
files = runtime_files,
Expand Down Expand Up @@ -177,6 +178,28 @@ See https://github.com/bazelbuild/bazel/issues/8685 for
motivation.

Does not apply to Windows.
""",
),
"bootstrap_template": attr.label(
allow_single_file = True,
default = DEFAULT_BOOTSTRAP_TEMPLATE,
doc = """
The bootstrap script template file to use. Should have %python_binary%,
%workspace_name%, %main%, and %imports%.

This template, after expansion, becomes the executable file used to start the
process, so it is responsible for initial bootstrapping actions such as finding
the Python interpreter, runfiles, and constructing an environment to run the
intended Python application.

While this attribute is currently optional, it will become required when the
Python rules are moved out of Bazel itself.

The exact variable names expanded is an unstable API and is subject to change.
The API will become more stable when the Python rules are moved out of Bazel
itself.

See @bazel_tools//tools/python:python_bootstrap_template.txt for more variables.
""",
),
},
Expand Down
Loading