From 763dd0ce6e1644bf895231432f616427a11d385a Mon Sep 17 00:00:00 2001 From: Ryan Beasley Date: Tue, 27 Apr 2021 00:31:43 -0700 Subject: [PATCH] Add `stub_shebang` to `py_runtime` Added a `stub_shebang` to `py_runtime` to allow users to specify a custom "shebang" expression used by the Python stub script. Motivation is to support environments where `#!/usr/bin/env python` isn't valid (ex: no `/usr/bin/python`, but there's a `/usr/bin/python3`). Closes #8685. ### Guided tour - Added field to `PyRuntimeInfoApi` inc. default value. - Added field to `PyRuntimeInfo` provider inc. tests. - Added field to `py_runtime` Starlark rule inc tests. - Replaced static `#!/usr/bin/env python` in the stub w/ `%shebang%`. - There are a few redundancies w/r/t declaring defaults and documentation. This is because there are a few separate public APIs (ex: `PyRuntimeInfo(...)`, `py_runtime(...)`), and I want to make sure defaults appear in the generated docs. Testing Done: - `bazelisk test src/test/java/com/google/devtools/build/lib/bazel/rules/python/...` `bazelisk test src/test/java/com/google/devtools/build/lib/rules/python/...` Closes #11434. PiperOrigin-RevId: 370622012 --- .../rules/python/BazelPythonSemantics.java | 10 ++++ .../rules/python/python_stub_template.txt | 2 +- .../build/lib/rules/python/PyRuntime.java | 5 +- .../build/lib/rules/python/PyRuntimeInfo.java | 50 +++++++++++++++---- .../build/lib/rules/python/PyRuntimeRule.java | 11 ++++ .../python/PyRuntimeInfoApi.java | 23 +++++++++ .../python/FakePyRuntimeInfo.java | 6 +++ .../BazelPyBinaryConfiguredTargetTest.java | 40 +++++++++++---- .../lib/rules/python/PyRuntimeInfoTest.java | 21 +++++++- 9 files changed, 144 insertions(+), 24 deletions(-) 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 a34fd7dc96e738..dd98037f288109 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 @@ -167,6 +167,7 @@ private static void createStubFile( stubOutput, STUB_TEMPLATE, ImmutableList.of( + Substitution.of("%shebang%", getStubShebang(ruleContext, common)), Substitution.of( "%main%", common.determineMainExecutableSource(/*withWorkspaceName=*/ true)), Substitution.of("%python_binary%", pythonBinary), @@ -451,6 +452,15 @@ private static String getPythonBinary( return pythonBinary; } + private static String getStubShebang(RuleContext ruleContext, PyCommon common) { + PyRuntimeInfo provider = getRuntime(ruleContext, common); + if (provider != null) { + return provider.getStubShebang(); + } else { + return PyRuntimeInfo.DEFAULT_STUB_SHEBANG; + } + } + @Override public CcInfo buildCcInfoProvider(Iterable deps) { ImmutableList ccInfos = diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt index 14e900cd58ecc1..c389af01c2df1f 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt @@ -1,4 +1,4 @@ -#!/usr/bin/env python +%shebang% # This script must retain compatibility with a wide variety of Python versions # since it is run for every py_binary target. Currently we guarantee support diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyRuntime.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyRuntime.java index c7566cfcde949d..f985a04084c8ef 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PyRuntime.java +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyRuntime.java @@ -42,6 +42,7 @@ public ConfiguredTarget create(RuleContext ruleContext) PythonVersion pythonVersion = PythonVersion.parseTargetOrSentinelValue( ruleContext.attributes().get("python_version", Type.STRING)); + String stubShebang = ruleContext.attributes().get("stub_shebang", Type.STRING); // Determine whether we're pointing to an in-build target (hermetic) or absolute system path // (non-hermetic). @@ -80,8 +81,8 @@ public ConfiguredTarget create(RuleContext ruleContext) PyRuntimeInfo provider = hermetic - ? PyRuntimeInfo.createForInBuildRuntime(interpreter, files, pythonVersion) - : PyRuntimeInfo.createForPlatformRuntime(interpreterPath, pythonVersion); + ? PyRuntimeInfo.createForInBuildRuntime(interpreter, files, pythonVersion, stubShebang) + : PyRuntimeInfo.createForPlatformRuntime(interpreterPath, pythonVersion, stubShebang); return new RuleConfiguredTargetBuilder(ruleContext) .setFilesToBuild(files) diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyRuntimeInfo.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyRuntimeInfo.java index 13c622827489e3..2e2f1ad0eeb69c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PyRuntimeInfo.java +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyRuntimeInfo.java @@ -54,17 +54,20 @@ public final class PyRuntimeInfo implements Info, PyRuntimeInfoApi { private final Location location; @Nullable private final PathFragment interpreterPath; @Nullable private final Artifact interpreter; - // Validated on initalization to contain Artifact + // Validated on initialization to contain Artifact @Nullable private final Depset files; /** Invariant: either PY2 or PY3. */ private final PythonVersion pythonVersion; + private final String stubShebang; + private PyRuntimeInfo( @Nullable Location location, @Nullable PathFragment interpreterPath, @Nullable Artifact interpreter, @Nullable Depset files, - PythonVersion pythonVersion) { + PythonVersion pythonVersion, + @Nullable String stubShebang) { Preconditions.checkArgument((interpreterPath == null) != (interpreter == null)); Preconditions.checkArgument((interpreter == null) == (files == null)); Preconditions.checkArgument(pythonVersion.isTargetValue()); @@ -73,6 +76,11 @@ private PyRuntimeInfo( this.interpreterPath = interpreterPath; this.interpreter = interpreter; this.pythonVersion = pythonVersion; + if (stubShebang != null && !stubShebang.isEmpty()) { + this.stubShebang = stubShebang; + } else { + this.stubShebang = PyRuntimeInfoApi.DEFAULT_STUB_SHEBANG; + } } @Override @@ -87,20 +95,29 @@ public Location getCreationLocation() { /** Constructs an instance from native rule logic (built-in location) for an in-build runtime. */ public static PyRuntimeInfo createForInBuildRuntime( - Artifact interpreter, NestedSet files, PythonVersion pythonVersion) { + Artifact interpreter, + NestedSet files, + PythonVersion pythonVersion, + @Nullable String stubShebang) { return new PyRuntimeInfo( /*location=*/ null, /*interpreterPath=*/ null, interpreter, Depset.of(Artifact.TYPE, files), - pythonVersion); + pythonVersion, + stubShebang); } /** Constructs an instance from native rule logic (built-in location) for a platform runtime. */ public static PyRuntimeInfo createForPlatformRuntime( - PathFragment interpreterPath, PythonVersion pythonVersion) { + PathFragment interpreterPath, PythonVersion pythonVersion, @Nullable String stubShebang) { return new PyRuntimeInfo( - /*location=*/ null, interpreterPath, /*interpreter=*/ null, /*files=*/ null, pythonVersion); + /*location=*/ null, + interpreterPath, + /*interpreter=*/ null, + /*files=*/ null, + pythonVersion, + stubShebang); } @Override @@ -113,12 +130,13 @@ public boolean equals(Object other) { PyRuntimeInfo otherInfo = (PyRuntimeInfo) other; return (this.interpreterPath.equals(otherInfo.interpreterPath) && this.interpreter.equals(otherInfo.interpreter) - && this.files.equals(otherInfo.files)); + && this.files.equals(otherInfo.files) + && this.stubShebang.equals(otherInfo.stubShebang)); } @Override public int hashCode() { - return Objects.hash(PyRuntimeInfo.class, interpreterPath, interpreter, files); + return Objects.hash(PyRuntimeInfo.class, interpreterPath, interpreter, files, stubShebang); } /** @@ -153,6 +171,11 @@ public Artifact getInterpreter() { return interpreter; } + @Override + public String getStubShebang() { + return stubShebang; + } + @Nullable public NestedSet getFiles() { try { @@ -191,6 +214,7 @@ public PyRuntimeInfo constructor( Object interpreterUncast, Object filesUncast, String pythonVersion, + String stubShebang, StarlarkThread thread) throws EvalException { String interpreterPath = @@ -225,14 +249,20 @@ public PyRuntimeInfo constructor( filesDepset = Depset.of(Artifact.TYPE, NestedSetBuilder.emptySet(Order.STABLE_ORDER)); } return new PyRuntimeInfo( - loc, /*interpreterPath=*/ null, interpreter, filesDepset, parsedPythonVersion); + loc, + /*interpreterPath=*/ null, + interpreter, + filesDepset, + parsedPythonVersion, + stubShebang); } else { return new PyRuntimeInfo( loc, PathFragment.create(interpreterPath), /*interpreter=*/ null, /*files=*/ null, - parsedPythonVersion); + parsedPythonVersion, + stubShebang); } } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyRuntimeRule.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyRuntimeRule.java index d65db55596c1c5..aceca7b76b412c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PyRuntimeRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyRuntimeRule.java @@ -66,6 +66,17 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) attr("python_version", STRING) .value(PythonVersion._INTERNAL_SENTINEL.toString()) .allowedValues(PyRuleClasses.TARGET_PYTHON_ATTR_VALUE_SET)) + + /* + "Shebang" expression prepended to the bootstrapping Python stub script + used when executing py_binary targets. + +

See issue 8685 for + motivation. + +

Does not apply to Windows. + */ + .add(attr("stub_shebang", STRING).value(PyRuntimeInfo.DEFAULT_STUB_SHEBANG)) .add(attr("output_licenses", LICENSE)) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/python/PyRuntimeInfoApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/python/PyRuntimeInfoApi.java index 090588b17d212c..9a178cee3a9a11 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/python/PyRuntimeInfoApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/python/PyRuntimeInfoApi.java @@ -45,6 +45,8 @@ category = DocCategory.PROVIDER) public interface PyRuntimeInfoApi extends StarlarkValue { + static final String DEFAULT_STUB_SHEBANG = "#!/usr/bin/env python"; + @StarlarkMethod( name = "interpreter_path", structField = true, @@ -88,6 +90,15 @@ public interface PyRuntimeInfoApi extends StarlarkValue { + "(only) \"PY2\" and \"PY3\".") String getPythonVersionForStarlark(); + @StarlarkMethod( + name = "stub_shebang", + structField = true, + doc = + "\"Shebang\" expression prepended to the bootstrapping Python stub script " + + "used when executing py_binary targets. Does not apply " + + "to Windows.") + String getStubShebang(); + /** Provider type for {@link PyRuntimeInfoApi} objects. */ @StarlarkBuiltin(name = "Provider", documented = false, doc = "") interface PyRuntimeInfoProviderApi extends ProviderApi { @@ -139,6 +150,17 @@ interface PyRuntimeInfoProviderApi extends ProviderApi { positional = false, named = true, doc = "The value for the new object's python_version field."), + @Param( + name = "stub_shebang", + allowedTypes = {@ParamType(type = String.class)}, + positional = false, + named = true, + defaultValue = "'" + DEFAULT_STUB_SHEBANG + "'", + doc = + "The value for the new object's stub_shebang field. " + + "Default is " + + DEFAULT_STUB_SHEBANG + + "."), }, useStarlarkThread = true, selfCall = true) @@ -148,6 +170,7 @@ PyRuntimeInfoApi constructor( Object interpreterUncast, Object filesUncast, String pythonVersion, + String stubShebang, StarlarkThread thread) throws EvalException; } diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/python/FakePyRuntimeInfo.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/python/FakePyRuntimeInfo.java index 43b8446df5342b..986125dcefb779 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/python/FakePyRuntimeInfo.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/python/FakePyRuntimeInfo.java @@ -44,6 +44,11 @@ public String getPythonVersionForStarlark() { return ""; } + @Override + public String getStubShebang() { + return ""; + } + @Override public void repr(Printer printer) {} @@ -56,6 +61,7 @@ public PyRuntimeInfoApi constructor( Object interpreterUncast, Object filesUncast, String pythonVersion, + String stubShebang, StarlarkThread thread) throws EvalException { return new FakePyRuntimeInfo(); 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 ea191eb86eb055..37bd77d1f629c6 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 @@ -45,14 +45,15 @@ private static String join(String... lines) { } /** - * Given a {@code py_binary} or {@code py_test} target, returns the path of the Python interpreter - * used by the generated stub script. + * Given a {@code py_binary} or {@code py_test} target and substitution key, returns the + * corresponding substitution value used by the generated stub script. * *

This works by casting the stub script's generating action to a template expansion action and - * looking for the expansion key for the Python interpreter. It's therefore linked to the - * implementation of the rule, but that's the cost we pay for avoiding an execution-time test. + * looking for the requested substitution key. It's therefore linked to the implementation of the + * rule, but that's the cost we pay for avoiding an execution-time test. */ - private String getInterpreterPathFromStub(ConfiguredTarget pyExecutableTarget) { + private String getSubstitutionValueFromStub( + ConfiguredTarget pyExecutableTarget, String substitutionKey) { // First find the stub script. Normally this is just the executable associated with the target. // But for Windows the executable is a separate launcher with an ".exe" extension, and the stub // script artifact has the same base name with the extension ".temp" instead. (At least, when @@ -75,12 +76,23 @@ private String getInterpreterPathFromStub(ConfiguredTarget pyExecutableTarget) { assertThat(generatingAction).isInstanceOf(TemplateExpansionAction.class); TemplateExpansionAction templateAction = (TemplateExpansionAction) generatingAction; for (Substitution sub : templateAction.getSubstitutions()) { - if (sub.getKey().equals("%python_binary%")) { + if (sub.getKey().equals(substitutionKey)) { return sub.getValue(); } } throw new AssertionError( - "Failed to find the '%python_binary%' key in the stub script's template expansion action"); + "Failed to find the '" + + substitutionKey + + "' key in the stub script's template " + + "expansion action"); + } + + private String getInterpreterPathFromStub(ConfiguredTarget pyExecutableTarget) { + return getSubstitutionValueFromStub(pyExecutableTarget, "%python_binary%"); + } + + private String getShebangFromStub(ConfiguredTarget pyExecutableTarget) { + return getSubstitutionValueFromStub(pyExecutableTarget, "%shebang%"); } // TODO(#8169): Delete tests of the legacy --python_top / --python_path behavior. @@ -168,11 +180,13 @@ private void defineToolchains() throws Exception { " name = 'py2_runtime',", " interpreter_path = '/system/python2',", " python_version = 'PY2',", + " stub_shebang = '#!/usr/bin/env python',", ")", "py_runtime(", " name = 'py3_runtime',", " interpreter_path = '/system/python3',", " python_version = 'PY3',", + " stub_shebang = '#!/usr/bin/env python3',", ")", "py_runtime_pair(", " name = 'py_runtime_pair',", @@ -214,10 +228,18 @@ public void runtimeObtainedFromToolchain() throws Exception { "--incompatible_use_python_toolchains=true", "--extra_toolchains=//toolchains:py_toolchain"); - String py2Path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:py2_bin")); - String py3Path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:py3_bin")); + ConfiguredTarget py2 = getConfiguredTarget("//pkg:py2_bin"); + ConfiguredTarget py3 = getConfiguredTarget("//pkg:py3_bin"); + + String py2Path = getInterpreterPathFromStub(py2); + String py3Path = getInterpreterPathFromStub(py3); assertThat(py2Path).isEqualTo("/system/python2"); assertThat(py3Path).isEqualTo("/system/python3"); + + String py2Shebang = getShebangFromStub(py2); + String py3Shebang = getShebangFromStub(py3); + assertThat(py2Shebang).isEqualTo("#!/usr/bin/env python"); + assertThat(py3Shebang).isEqualTo("#!/usr/bin/env python3"); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PyRuntimeInfoTest.java b/src/test/java/com/google/devtools/build/lib/rules/python/PyRuntimeInfoTest.java index 2b2af3f35dd510..03a91656267965 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/python/PyRuntimeInfoTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/python/PyRuntimeInfoTest.java @@ -58,7 +58,7 @@ private static void assertHasOrderAndContainsExactly( public void factoryMethod_InBuildRuntime() throws Exception { NestedSet files = NestedSetBuilder.create(Order.STABLE_ORDER, dummyFile); PyRuntimeInfo inBuildRuntime = - PyRuntimeInfo.createForInBuildRuntime(dummyInterpreter, files, PythonVersion.PY2); + PyRuntimeInfo.createForInBuildRuntime(dummyInterpreter, files, PythonVersion.PY2, null); assertThat(inBuildRuntime.getCreationLocation()).isEqualTo(Location.BUILTIN); assertThat(inBuildRuntime.getInterpreterPath()).isNull(); @@ -68,12 +68,14 @@ public void factoryMethod_InBuildRuntime() throws Exception { assertThat(inBuildRuntime.getFilesForStarlark().getSet(Artifact.class)).isEqualTo(files); assertThat(inBuildRuntime.getPythonVersion()).isEqualTo(PythonVersion.PY2); assertThat(inBuildRuntime.getPythonVersionForStarlark()).isEqualTo("PY2"); + assertThat(inBuildRuntime.getStubShebang()).isEqualTo(PyRuntimeInfo.DEFAULT_STUB_SHEBANG); } @Test public void factoryMethod_PlatformRuntime() { PathFragment path = PathFragment.create("/system/interpreter"); - PyRuntimeInfo platformRuntime = PyRuntimeInfo.createForPlatformRuntime(path, PythonVersion.PY2); + PyRuntimeInfo platformRuntime = + PyRuntimeInfo.createForPlatformRuntime(path, PythonVersion.PY2, null); assertThat(platformRuntime.getCreationLocation()).isEqualTo(Location.BUILTIN); assertThat(platformRuntime.getInterpreterPath()).isEqualTo(path); @@ -83,6 +85,7 @@ public void factoryMethod_PlatformRuntime() { assertThat(platformRuntime.getFilesForStarlark()).isNull(); assertThat(platformRuntime.getPythonVersion()).isEqualTo(PythonVersion.PY2); assertThat(platformRuntime.getPythonVersionForStarlark()).isEqualTo("PY2"); + assertThat(platformRuntime.getStubShebang()).isEqualTo(PyRuntimeInfo.DEFAULT_STUB_SHEBANG); } @Test @@ -99,6 +102,7 @@ public void starlarkConstructor_InBuildRuntime() throws Exception { assertThat(info.getInterpreter()).isEqualTo(dummyInterpreter); assertHasOrderAndContainsExactly(info.getFiles(), Order.STABLE_ORDER, dummyFile); assertThat(info.getPythonVersion()).isEqualTo(PythonVersion.PY2); + assertThat(info.getStubShebang()).isEqualTo(PyRuntimeInfo.DEFAULT_STUB_SHEBANG); } @Test @@ -114,6 +118,19 @@ public void starlarkConstructor_PlatformRuntime() throws Exception { assertThat(info.getInterpreter()).isNull(); assertThat(info.getFiles()).isNull(); assertThat(info.getPythonVersion()).isEqualTo(PythonVersion.PY2); + assertThat(info.getStubShebang()).isEqualTo(PyRuntimeInfo.DEFAULT_STUB_SHEBANG); + } + + @Test + public void starlarkConstructor_CustomShebang() throws Exception { + ev.exec( + "info = PyRuntimeInfo(", // + " interpreter_path = '/system/interpreter',", + " python_version = 'PY2',", + " stub_shebang = '#!/usr/bin/custom',", + ")"); + PyRuntimeInfo info = (PyRuntimeInfo) ev.lookup("info"); + assertThat(info.getStubShebang()).isEqualTo("#!/usr/bin/custom"); } @Test