Skip to content

Commit ccfb2df

Browse files
philwodamienmg
philwo
authored and
damienmg
committed
Allow py_binary to be the executable of a Skylark action or any
SpawnAction on Windows. RELNOTES: None. Change-Id: I2d926447511dab5fb804051abdbef9031cb089be PiperOrigin-RevId: 162201440
1 parent 90ed84c commit ccfb2df

8 files changed

Lines changed: 154 additions & 11 deletions

File tree

‎src/main/java/com/google/devtools/build/lib/BUILD‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -737,7 +737,7 @@ java_library(
737737
resources = [
738738
"bazel/rules/java/java_stub_template.txt",
739739
"bazel/rules/java/java_stub_template_windows.txt",
740-
"bazel/rules/python/stub_template.txt",
740+
"bazel/rules/python/python_stub_template.txt",
741741
"bazel/rules/sh/sh_stub_template_windows.txt",
742742
],
743743
deps = [

‎src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import com.google.devtools.build.lib.rules.test.InstrumentedFilesCollector.InstrumentationSpec;
4242
import com.google.devtools.build.lib.syntax.Type;
4343
import com.google.devtools.build.lib.util.FileTypeSet;
44+
import com.google.devtools.build.lib.util.OS;
4445
import com.google.devtools.build.lib.vfs.PathFragment;
4546
import java.util.ArrayList;
4647
import java.util.Collection;
@@ -51,7 +52,9 @@
5152
*/
5253
public class BazelPythonSemantics implements PythonSemantics {
5354
private static final Template STUB_TEMPLATE =
54-
Template.forResource(BazelPythonSemantics.class, "stub_template.txt");
55+
Template.forResource(BazelPythonSemantics.class, "python_stub_template.txt");
56+
private static final Template STUB_TEMPLATE_WINDOWS =
57+
Template.forResource(BazelPythonSemantics.class, "python_stub_template_windows.txt");
5558
public static final InstrumentationSpec PYTHON_COLLECTION_SPEC = new InstrumentationSpec(
5659
FileTypeSet.of(BazelPyRuleClasses.PYTHON_SOURCE),
5760
"srcs", "deps", "data");
@@ -123,7 +126,7 @@ public Artifact getPythonTemplateMainArtifact(RuleContext ruleContext, Artifact
123126
}
124127

125128
@Override
126-
public void createExecutable(
129+
public Artifact createExecutable(
127130
RuleContext ruleContext,
128131
PyCommon common,
129132
CcLinkParamsStore ccLinkParamsStore,
@@ -180,7 +183,21 @@ public void createExecutable(
180183
.useDefaultShellEnvironment()
181184
.setMnemonic("BuildBinary")
182185
.build(ruleContext));
186+
187+
if (OS.getCurrent() == OS.WINDOWS) {
188+
Artifact executableWrapper = common.getExecutableWrapper();
189+
ruleContext.registerAction(
190+
new TemplateExpansionAction(
191+
ruleContext.getActionOwner(),
192+
executableWrapper,
193+
STUB_TEMPLATE_WINDOWS,
194+
ImmutableList.of(Substitution.of("%python_path%", pythonBinary)),
195+
true));
196+
return executableWrapper;
197+
}
183198
}
199+
200+
return executable;
184201
}
185202

186203
@Override
@@ -216,7 +233,7 @@ private static String getZipRunfilesPath(PathFragment path, PathFragment workspa
216233
}
217234
// We put the whole runfiles tree under the ZIP_RUNFILES_DIRECTORY_NAME directory, by doing this
218235
// , we avoid the conflict between default workspace name "__main__" and __main__.py file.
219-
// Note: This name has to be the same with the one in stub_template.txt.
236+
// Note: This name has to be the same with the one in python_stub_template.txt.
220237
return ZIP_RUNFILES_DIRECTORY_NAME.getRelative(zipRunfilesPath).toString();
221238
}
222239

src/main/java/com/google/devtools/build/lib/bazel/rules/python/stub_template.txt renamed to src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt

File renamed without changes.

‎src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template_windows.txt‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
@rem Copyright 2017 The Bazel Authors. All rights reserved.
2+
@rem
3+
@rem Licensed under the Apache License, Version 2.0 (the "License");
4+
@rem you may not use this file except in compliance with the License.
5+
@rem You may obtain a copy of the License at
6+
@rem
7+
@rem http://www.apache.org/licenses/LICENSE-2.0
8+
@rem
9+
@rem Unless required by applicable law or agreed to in writing, software
10+
@rem distributed under the License is distributed on an "AS IS" BASIS,
11+
@rem WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
@rem See the License for the specific language governing permissions and
13+
@rem limitations under the License.
14+
@rem
15+
@rem This script was generated from python_stub_template_windows.txt. Please
16+
@rem don't edit it directly.
17+
18+
@SETLOCAL ENABLEEXTENSIONS
19+
20+
@rem launcher=${$0%.cmd}
21+
@set launcher=%~dp0%~n0
22+
23+
@call %python_path% %launcher% %*

‎src/main/java/com/google/devtools/build/lib/rules/python/PyBinary.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ static RuleConfiguredTargetBuilder init(RuleContext ruleContext, PythonSemantics
7777
return null;
7878
}
7979

80-
semantics.createExecutable(ruleContext, common, ccLinkParamsStore, imports);
80+
Artifact realExecutable =
81+
semantics.createExecutable(ruleContext, common, ccLinkParamsStore, imports);
8182
Runfiles commonRunfiles = collectCommonRunfiles(ruleContext, common, semantics);
8283

8384
Runfiles.Builder defaultRunfilesBuilder = new Runfiles.Builder(
@@ -117,7 +118,7 @@ static RuleConfiguredTargetBuilder init(RuleContext ruleContext, PythonSemantics
117118
return builder
118119
.setFilesToBuild(common.getFilesToBuild())
119120
.add(RunfilesProvider.class, runfilesProvider)
120-
.setRunfilesSupport(runfilesSupport, common.getExecutable())
121+
.setRunfilesSupport(runfilesSupport, realExecutable)
121122
.addNativeDeclaredProvider(new CcLinkParamsProvider(ccLinkParamsStore))
122123
.add(PythonImportsProvider.class, new PythonImportsProvider(imports));
123124
}
@@ -127,6 +128,9 @@ private static Runfiles collectCommonRunfiles(RuleContext ruleContext, PyCommon
127128
Runfiles.Builder builder = new Runfiles.Builder(
128129
ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles());
129130
builder.addArtifact(common.getExecutable());
131+
if (common.getExecutableWrapper() != null) {
132+
builder.addArtifact(common.getExecutableWrapper());
133+
}
130134
if (common.getConvertedFiles() != null) {
131135
builder.addSymlinks(common.getConvertedFiles());
132136
} else {

‎src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import com.google.devtools.build.lib.syntax.SkylarkType;
5252
import com.google.devtools.build.lib.syntax.Type;
5353
import com.google.devtools.build.lib.util.FileType;
54+
import com.google.devtools.build.lib.util.OS;
5455
import com.google.devtools.build.lib.util.Preconditions;
5556
import com.google.devtools.build.lib.vfs.PathFragment;
5657
import com.google.protobuf.GeneratedMessage.GeneratedExtension;
@@ -80,6 +81,7 @@ public void collectMetadataArtifacts(Iterable<Artifact> artifacts,
8081
private final RuleContext ruleContext;
8182

8283
private Artifact executable = null;
84+
private Artifact executableWrapper = null;
8385

8486
private NestedSet<Artifact> transitivePythonSources;
8587

@@ -114,15 +116,21 @@ public void initBinary(List<Artifact> srcs) {
114116

115117
validatePackageName();
116118
executable = ruleContext.createOutputArtifact();
119+
if (OS.getCurrent() == OS.WINDOWS) {
120+
executableWrapper =
121+
ruleContext.getImplicitOutputArtifact(ruleContext.getTarget().getName() + ".cmd");
122+
}
117123
if (this.version == PythonVersion.PY2AND3) {
118124
// TODO(bazel-team): we need to create two actions
119125
ruleContext.ruleError("PY2AND3 is not yet implemented");
120126
}
121127

122-
filesToBuild = NestedSetBuilder.<Artifact>stableOrder()
123-
.addAll(srcs)
124-
.add(executable)
125-
.build();
128+
NestedSetBuilder<Artifact> filesToBuildBuilder =
129+
NestedSetBuilder.<Artifact>stableOrder().addAll(srcs).add(executable);
130+
if (executableWrapper != null) {
131+
filesToBuildBuilder.add(executableWrapper);
132+
}
133+
filesToBuild = filesToBuildBuilder.build();
126134

127135
if (ruleContext.hasErrors()) {
128136
return;
@@ -447,6 +455,10 @@ public Artifact getExecutable() {
447455
return executable;
448456
}
449457

458+
public Artifact getExecutableWrapper() {
459+
return executableWrapper;
460+
}
461+
450462
public Map<PathFragment, Artifact> getConvertedFiles() {
451463
return convertedFiles;
452464
}

‎src/main/java/com/google/devtools/build/lib/rules/python/PythonSemantics.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ Collection<Artifact> precompiledPythonFiles(
6969
*
7070
* <p>This should create a generating action for {@code common.getExecutable()}.
7171
*/
72-
void createExecutable(
72+
Artifact createExecutable(
7373
RuleContext ruleContext,
7474
PyCommon common,
7575
CcLinkParamsStore ccLinkParamsStore,

‎src/test/py/bazel/launcher_script_test.py‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,93 @@ def testShBinaryLauncher(self):
178178
self.AssertExitCode(exit_code, 0, stderr)
179179
self.assertEqual(stdout[0], 'hello batch')
180180

181+
def testPyBinaryLauncher(self):
182+
self.ScratchFile('WORKSPACE')
183+
self.ScratchFile('foo/foo.bzl', [
184+
'def _impl(ctx):',
185+
' ctx.action(',
186+
' arguments=[ctx.outputs.out.path],',
187+
' outputs=[ctx.outputs.out],',
188+
' executable=ctx.executable._hello_world,',
189+
' use_default_shell_env=True)',
190+
'',
191+
'helloworld = rule(',
192+
' implementation=_impl,',
193+
' attrs={',
194+
' "srcs": attr.label_list(allow_files=True),',
195+
' "out": attr.output(mandatory=True),',
196+
' "_hello_world": attr.label(executable=True, cfg="host",',
197+
' allow_files=True,',
198+
' default=Label("//foo:foo"))',
199+
' }',
200+
')',
201+
])
202+
self.ScratchFile('foo/BUILD', [
203+
'load(":foo.bzl", "helloworld")',
204+
'',
205+
'py_binary(',
206+
' name = "foo",',
207+
' srcs = ["foo.py"],',
208+
' data = ["//bar:bar.txt"],',
209+
')',
210+
'',
211+
'helloworld(',
212+
' name = "hello",',
213+
' out = "hello.txt",',
214+
')'
215+
])
216+
foo_py = self.ScratchFile('foo/foo.py', [
217+
'#!/usr/bin/env python',
218+
'import sys',
219+
'if len(sys.argv) == 2:',
220+
' with open(sys.argv[1], "w") as f:',
221+
' f.write("Hello World!")',
222+
'else:',
223+
' print "Hello World!"',
224+
])
225+
self.ScratchFile('bar/BUILD', ['exports_files(["bar.txt"])'])
226+
self.ScratchFile('bar/bar.txt', ['hello'])
227+
os.chmod(foo_py, stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR)
228+
229+
exit_code, stdout, stderr = self.RunBazel(['info', 'bazel-bin'])
230+
self.AssertExitCode(exit_code, 0, stderr)
231+
bazel_bin = stdout[0]
232+
233+
# Verify that the build of our py_binary succeeds.
234+
exit_code, _, stderr = self.RunBazel(['build', '//foo:foo'])
235+
self.AssertExitCode(exit_code, 0, stderr)
236+
237+
# Verify that generated files exist.
238+
foo_bin = os.path.join(bazel_bin, 'foo', 'foo.cmd'
239+
if self.IsWindows() else 'foo')
240+
self.assertTrue(os.path.isfile(foo_bin))
241+
self.assertTrue(os.path.isdir(os.path.join(bazel_bin, 'foo/foo.runfiles')))
242+
243+
# Verify contents of runfiles (manifest).
244+
if self.IsWindows():
245+
self.AssertRunfilesManifestContains(
246+
os.path.join(bazel_bin, 'foo/foo.runfiles/MANIFEST'),
247+
'__main__/bar/bar.txt')
248+
else:
249+
self.assertTrue(
250+
os.path.islink(
251+
os.path.join(bazel_bin, 'foo/foo.runfiles/__main__/bar/bar.txt')))
252+
253+
# Try to run the built py_binary.
254+
exit_code, stdout, stderr = self.RunProgram([foo_bin])
255+
self.AssertExitCode(exit_code, 0, stderr)
256+
self.assertEqual(stdout[0], 'Hello World!')
257+
258+
# Try to use the py_binary as an executable in a Skylark rule.
259+
exit_code, stdout, stderr = self.RunBazel(['build', '//foo:hello'])
260+
self.AssertExitCode(exit_code, 0, stderr)
261+
262+
# Verify that the Skylark action generated the right output.
263+
hello_path = os.path.join(bazel_bin, 'foo', 'hello.txt')
264+
self.assertTrue(os.path.isfile(hello_path))
265+
with open(hello_path, 'r') as f:
266+
self.assertEqual(f.read(), 'Hello World!')
267+
181268
def AssertRunfilesManifestContains(self, manifest, entry):
182269
with open(manifest, 'r') as f:
183270
for l in f:

0 commit comments

Comments
 (0)