Skip to content

Commit

Permalink
Add --incompatible_python_disable_py2, which fails if Python 2 is spe…
Browse files Browse the repository at this point in the history
…cified.

When true, using Python 2 only attribute values will cause an error.

More specifically, using `python_version=PY2`, `srcs_version=PY2` or `srcs_version=PY2ONLY` with `py_binary`, `py_test`, `py_library`, `py_runtime`,
or `py_runtime_pair` will result in an error.

Work towards #15684

PiperOrigin-RevId: 501959931
Change-Id: I7bc089a2f7b46f2538e4a92d8753c788193a71d5
  • Loading branch information
rickeylev authored and copybara-github committed Jan 14, 2023
1 parent 24dbe0d commit d1bbf4b
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction;
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.rules.cpp.CcInfo;
import com.google.devtools.build.lib.rules.python.PyCcLinkParamsProvider;
import com.google.devtools.build.lib.rules.python.PyCommon;
Expand Down Expand Up @@ -77,6 +78,19 @@ public String getSrcsVersionDocURL() {

@Override
public void validate(RuleContext ruleContext, PyCommon common) {
PythonConfiguration config = ruleContext.getFragment(PythonConfiguration.class);
if (config.getDisablePy2()) {
var attrs = ruleContext.attributes();
if (config.getDefaultPythonVersion().equals(PythonVersion.PY2)
|| attrs.getOrDefault("python_version", Type.STRING, "UNSET").equals("PY2")
|| attrs.getOrDefault("srcs_version", Type.STRING, "UNSET").equals("PY2")
|| attrs.getOrDefault("srcs_version", Type.STRING, "UNSET").equals("PY2ONLY")) {
ruleContext.ruleError(
"Using Python 2 is not supported and disabled; see "
+ "https://github.com/bazelbuild/bazel/issues/15684");
return;
}
}
}

@Override
Expand Down Expand Up @@ -116,18 +130,18 @@ public List<String> getImports(RuleContext ruleContext) {
PathFragment packageFragment = ruleContext.getLabel().getPackageIdentifier().getRunfilesPath();
// Python scripts start with x.runfiles/ as the module space, so everything must be manually
// adjusted to be relative to the workspace name.
packageFragment = PathFragment.create(ruleContext.getWorkspaceName())
.getRelative(packageFragment);
packageFragment =
PathFragment.create(ruleContext.getWorkspaceName()).getRelative(packageFragment);
for (String importsAttr : ruleContext.getExpander().list("imports")) {
if (importsAttr.startsWith("/")) {
ruleContext.attributeWarning("imports",
"ignoring invalid absolute path '" + importsAttr + "'");
ruleContext.attributeWarning(
"imports", "ignoring invalid absolute path '" + importsAttr + "'");
continue;
}
PathFragment importsPath = packageFragment.getRelative(importsAttr);
if (importsPath.containsUplevelReferences()) {
ruleContext.attributeError("imports",
"Path " + importsAttr + " references a path above the execution root");
ruleContext.attributeError(
"imports", "Path " + importsAttr + " references a path above the execution root");
}
result.add(importsPath.getPathString());
}
Expand Down Expand Up @@ -279,7 +293,7 @@ public void createExecutable(
// unix. See also https://github.com/bazelbuild/bazel/issues/7947#issuecomment-491385802.
pythonBinary,
executable,
/*useZipFile=*/ buildPythonZip);
/* useZipFile= */ buildPythonZip);
}
}

Expand Down Expand Up @@ -468,7 +482,7 @@ private static String getPythonBinary(
pythonBinary =
workspaceName.getRelative(provider.getInterpreter().getRunfilesPath()).getPathString();
}
} else {
} else {
// make use of the Python interpreter in an absolute path
pythonBinary = bazelConfig.getPythonPath();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public class PythonConfiguration extends Fragment implements StarlarkValue {
private final boolean useToolchains;

private final boolean defaultToExplicitInitPy;
private final boolean disablePy2;

public PythonConfiguration(BuildOptions buildOptions) {
PythonOptions pythonOptions = buildOptions.get(PythonOptions.class);
Expand All @@ -64,6 +65,7 @@ public PythonConfiguration(BuildOptions buildOptions) {
this.py2OutputsAreSuffixed = pythonOptions.incompatiblePy2OutputsAreSuffixed;
this.useToolchains = pythonOptions.incompatibleUsePythonToolchains;
this.defaultToExplicitInitPy = pythonOptions.incompatibleDefaultToExplicitInitPy;
this.disablePy2 = pythonOptions.disablePy2;
}

@Override
Expand Down Expand Up @@ -172,4 +174,12 @@ public boolean useToolchains() {
public boolean defaultToExplicitInitPy() {
return defaultToExplicitInitPy;
}

@StarlarkMethod(
name = "disable_py2",
structField = true,
doc = "The value of the --incompatible_python_disable_py2 flag.")
public boolean getDisablePy2() {
return disablePy2;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,18 @@ public String getTypeDescription() {
+ "`--incompatible_py2_outputs_are_suffixed`.")
public boolean incompatiblePy3IsDefault;

@Option(
name = "incompatible_python_disable_py2",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.INPUT_STRICTNESS,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
help =
"If true, using Python 2 settings will cause an error. This includes "
+ "python_version=PY2, srcs_version=PY2, and srcs_version=PY2ONLY. See "
+ "https://github.com/bazelbuild/bazel/issues/15684 for more information.")
public boolean disablePy2;

@Option(
name = "incompatible_py2_outputs_are_suffixed",
defaultValue = "true",
Expand Down Expand Up @@ -342,6 +354,7 @@ public FragmentOptions getExec() {
hostPythonOptions.incompatibleDefaultToExplicitInitPy = incompatibleDefaultToExplicitInitPy;
hostPythonOptions.incompatibleDisallowLegacyPyProvider = incompatibleDisallowLegacyPyProvider;
hostPythonOptions.incompatibleRemoveOldPythonVersionApi = incompatibleRemoveOldPythonVersionApi;
hostPythonOptions.disablePy2 = disablePy2;

// Save host options in case of a further exec->host transition.
hostPythonOptions.hostForcePython = hostForcePython;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ def _py_runtime_impl(ctx):
else:
python_version = ctx.fragments.py.default_python_version

if ctx.fragments.py.disable_py2 and python_version == "PY2":
fail("Using Python 2 is not supported and disabled; see " +
"https://github.com/bazelbuild/bazel/issues/15684")

return [
_PyRuntimeInfo(
interpreter_path = interpreter_path or None,
Expand Down
13 changes: 13 additions & 0 deletions tools/python/toolchain.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,23 @@ def _py_runtime_pair_impl(ctx):
else:
py3_runtime = None

if _is_py2_disabled(ctx) and py2_runtime != None:
fail("Using Python 2 is not supported and disabled; see " +
"https://github.com/bazelbuild/bazel/issues/15684")

return [platform_common.ToolchainInfo(
py2_runtime = py2_runtime,
py3_runtime = py3_runtime,
)]

def _is_py2_disabled(ctx):
# In Google, this file isn't bundled with Bazel, so we have to conditionally
# check for this flag.
# TODO: Remove this once a build with the flag is released in Google
if not hasattr(ctx.fragments.py, "disable_py"):
return False
return ctx.fragments.py.disable_py2

py_runtime_pair = rule(
implementation = _py_runtime_pair_impl,
attrs = {
Expand All @@ -61,6 +73,7 @@ The runtime to use for Python 3 targets. Must have `python_version` set to
""",
),
},
fragments = ["py"],
doc = """\
A toolchain rule for Python.
Expand Down

0 comments on commit d1bbf4b

Please sign in to comment.