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

Use provided stub_shebang when building with --build_python_zip #13469

Closed
wants to merge 2 commits into from
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 @@ -233,32 +233,25 @@ public void createExecutable(

// Create the zip file if requested. On unix, copy it from the intermediate artifact to the
// final executable while prepending the shebang.
if (buildPythonZip) {
if (buildPythonZip && OS.getCurrent() != OS.WINDOWS) {
Artifact zipFile = common.getPythonZipArtifact(executable);

if (OS.getCurrent() != OS.WINDOWS) {
PathFragment shExecutable = ShToolchain.getPathOrError(ruleContext);
// TODO(#8685): Remove this special-case handling as part of making the proper shebang a
// property of the Python toolchain configuration.
String pythonExecutableName = OS.getCurrent() == OS.OPENBSD ? "python3" : "python";
// NOTE: keep the following line intact to support nix builds
String pythonShebang = "#!/usr/bin/env " + pythonExecutableName;
ruleContext.registerAction(
new SpawnAction.Builder()
.addInput(zipFile)
.addOutput(executable)
.setShellCommand(
shExecutable,
"echo '"
+ pythonShebang
+ "' | cat - "
+ zipFile.getExecPathString()
+ " > "
+ executable.getExecPathString())
.useDefaultShellEnvironment()
.setMnemonic("BuildBinary")
.build(ruleContext));
}
PathFragment shExecutable = ShToolchain.getPathOrError(ruleContext);
String pythonShebang = getStubShebang(ruleContext, common);
ruleContext.registerAction(
new SpawnAction.Builder()
.addInput(zipFile)
.addOutput(executable)
.setShellCommand(
shExecutable,
"echo '"
+ escapeShellSingleQuotes(pythonShebang)
+ "' | cat - "
+ zipFile.getExecPathString()
+ " > "
+ executable.getExecPathString())
.useDefaultShellEnvironment()
.setMnemonic("BuildBinary")
.build(ruleContext));
}

// On Windows, create the launcher.
Expand Down Expand Up @@ -452,6 +445,10 @@ private static String getPythonBinary(
return pythonBinary;
}

private static String escapeShellSingleQuotes(String str) {
return str.replaceAll("'", "'\"'\"'");
}

private static String getStubShebang(RuleContext ruleContext, PyCommon common) {
PyRuntimeInfo provider = getRuntime(ruleContext, common);
if (provider != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
import com.google.devtools.build.lib.analysis.actions.SpawnAction;
import com.google.devtools.build.lib.analysis.actions.Substitution;
import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.testutil.TestConstants;
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import java.util.List;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
Expand Down Expand Up @@ -95,6 +97,16 @@ private String getShebangFromStub(ConfiguredTarget pyExecutableTarget) {
return getSubstitutionValueFromStub(pyExecutableTarget, "%shebang%");
}

private List<String> getArguments(ConfiguredTarget pyExecutableTarget) throws Exception {
assertThat(OS.getCurrent()).isNotEqualTo(OS.WINDOWS);
Artifact executable = pyExecutableTarget.getProvider(FilesToRunProvider.class).getExecutable();
assertThat(executable).isNotNull();
Action generatingAction = getGeneratingAction(executable);
assertThat(generatingAction).isInstanceOf(SpawnAction.class);
SpawnAction spawnAction = (SpawnAction) generatingAction;
return spawnAction.getArguments();
}

// TODO(#8169): Delete tests of the legacy --python_top / --python_path behavior.

@Test
Expand Down Expand Up @@ -186,7 +198,7 @@ private void defineToolchains() throws Exception {
" name = 'py3_runtime',",
" interpreter_path = '/system/python3',",
" python_version = 'PY3',",
" stub_shebang = '#!/usr/bin/env python3',",
" stub_shebang = '#!/usr/bin/env \\'python3\\'',",
")",
"py_runtime_pair(",
" name = 'py_runtime_pair',",
Expand Down Expand Up @@ -239,7 +251,43 @@ public void runtimeObtainedFromToolchain() throws Exception {
String py2Shebang = getShebangFromStub(py2);
String py3Shebang = getShebangFromStub(py3);
assertThat(py2Shebang).isEqualTo("#!/usr/bin/env python");
assertThat(py3Shebang).isEqualTo("#!/usr/bin/env python3");
assertThat(py3Shebang).isEqualTo("#!/usr/bin/env 'python3'");
}

@Test
public void runtimeObtainedFromToolchainForZippedTarget() throws Exception {
if (OS.getCurrent() == OS.WINDOWS) {
return;
}
defineToolchains();
scratch.file(
"pkg/BUILD",
"py_binary(",
" name = 'py2_bin',",
" srcs = ['py2_bin.py'],",
" python_version = 'PY2',",
")",
"py_binary(",
" name = 'py3_bin',",
" srcs = ['py3_bin.py'],",
" python_version = 'PY3',",
")");
useConfiguration(
"--incompatible_use_python_toolchains=true",
"--extra_toolchains=//toolchains:py_toolchain",
"--build_python_zip");

ConfiguredTarget py2 = getConfiguredTarget("//pkg:py2_bin");
ConfiguredTarget py3 = getConfiguredTarget("//pkg:py3_bin");

List<String> py2Args = getArguments(py2);
List<String> py3Args = getArguments(py3);

assertThat(py2Args).isNotEmpty();
assertThat(py2Args.get(py2Args.size()-1)).startsWith("echo '#!/usr/bin/env python'");

assertThat(py3Args).isNotEmpty();
assertThat(py3Args.get(py3Args.size()-1)).startsWith("echo '#!/usr/bin/env '\"'\"'python3'\"'\"''");
}

@Test
Expand Down