Skip to content

Commit

Permalink
fix(py_wheel): Avoid reliance on bash in py_wheel macro. (bazelbuil…
Browse files Browse the repository at this point in the history
…d#2171)

While trying to modernize an older repo I tried building wheels on
windows and ran into a failure:
```
Windows Subsystem for Linux has no installed distributions.

Use 'wsl.exe --list --online' to list available distributions
and 'wsl.exe --install <Distro>' to install.

Distributions can also be installed by visiting the Microsoft Store:
https://aka.ms/wlstore
Errorcode: Bash/Service/CreateInstance/GetDefaultDistro/WSL_E_DEFAULT_DISTRO_NOT_FOUND
```

This appears to be caused by the `py_wheel_dist` rule which gets caught
by `//...`. This target should be considered a side-effect/optional
target of `py_wheel`. To fix:
1. Mark the target as `manual`, so it's only built when explicitly
requested.
2. Implement copying to the directory with a Python program instead of
shell, so bash
   isn't required.
  • Loading branch information
UebelAndre authored Aug 31, 2024
1 parent 54c9fab commit b97a5d6
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 27 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ A brief description of the categories of changes:
* (gazelle) Correctly resolve deps that have top-level module overlap with a gazelle_python.yaml dep module

### Added
* Nothing yet
* (py_wheel) Removed use of bash to avoid failures on Windows machines which do not
have it installed.

### Removed
* Nothing yet
Expand Down
80 changes: 54 additions & 26 deletions python/packaging.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -35,25 +35,28 @@ This rule is intended to be used as data dependency to py_wheel rule.
attrs = py_package_lib.attrs,
)

# Based on https://github.com/aspect-build/bazel-lib/tree/main/lib/private/copy_to_directory.bzl
# Avoiding a bazelbuild -> aspect-build dependency :(
def _py_wheel_dist_impl(ctx):
dir = ctx.actions.declare_directory(ctx.attr.out)
out = ctx.actions.declare_directory(ctx.attr.out)
name_file = ctx.attr.wheel[PyWheelInfo].name_file
cmds = [
"mkdir -p \"%s\"" % dir.path,
"""cp "{}" "{}/$(cat "{}")" """.format(ctx.files.wheel[0].path, dir.path, name_file.path),
]
ctx.actions.run_shell(
inputs = ctx.files.wheel + [name_file],
outputs = [dir],
command = "\n".join(cmds),
mnemonic = "CopyToDirectory",
progress_message = "Copying files to directory",
use_default_shell_env = True,
wheel = ctx.attr.wheel[PyWheelInfo].wheel

args = ctx.actions.args()
args.add("--wheel", wheel)
args.add("--name_file", name_file)
args.add("--output", out.path)

ctx.actions.run(
mnemonic = "PyWheelDistDir",
executable = ctx.executable._copier,
inputs = [wheel, name_file],
outputs = [out],
arguments = [args],
)
return [
DefaultInfo(files = depset([dir]), runfiles = ctx.runfiles([dir])),
DefaultInfo(
files = depset([out]),
runfiles = ctx.runfiles([out]),
),
]

py_wheel_dist = rule(
Expand All @@ -67,12 +70,28 @@ This also has the advantage that stamping information is included in the wheel's
""",
implementation = _py_wheel_dist_impl,
attrs = {
"out": attr.string(doc = "name of the resulting directory", mandatory = True),
"wheel": attr.label(doc = "a [py_wheel target](#py_wheel)", providers = [PyWheelInfo]),
"out": attr.string(
doc = "name of the resulting directory",
mandatory = True,
),
"wheel": attr.label(
doc = "a [py_wheel target](#py_wheel)",
providers = [PyWheelInfo],
),
"_copier": attr.label(
cfg = "exec",
executable = True,
default = Label("//python/private:py_wheel_dist"),
),
},
)

def py_wheel(name, twine = None, twine_binary = Label("//tools/publish:twine") if BZLMOD_ENABLED else None, publish_args = [], **kwargs):
def py_wheel(
name,
twine = None,
twine_binary = Label("//tools/publish:twine") if BZLMOD_ENABLED else None,
publish_args = [],
**kwargs):
"""Builds a Python Wheel.
Wheels are Python distribution format defined in https://www.python.org/dev/peps/pep-0427/.
Expand Down Expand Up @@ -153,24 +172,31 @@ def py_wheel(name, twine = None, twine_binary = Label("//tools/publish:twine") i
Note that you can also pass additional args to the bazel run command as in the example above.
**kwargs: other named parameters passed to the underlying [py_wheel rule](#py_wheel_rule)
"""
_dist_target = "{}.dist".format(name)
tags = kwargs.pop("tags", [])
manual_tags = depset(tags + ["manual"]).to_list()

dist_target = "{}.dist".format(name)
py_wheel_dist(
name = _dist_target,
name = dist_target,
wheel = name,
out = kwargs.pop("dist_folder", "{}_dist".format(name)),
tags = manual_tags,
**copy_propagating_kwargs(kwargs)
)

_py_wheel(name = name, **kwargs)
_py_wheel(
name = name,
tags = tags,
**kwargs
)

twine_args = []
if twine or twine_binary:
twine_args = ["upload"]
twine_args.extend(publish_args)
twine_args.append("$(rootpath :{})/*".format(_dist_target))
twine_args.append("$(rootpath :{})/*".format(dist_target))

if twine_binary:
twine_kwargs = {"tags": ["manual"]}
native_binary(
name = "{}.publish".format(name),
src = twine_binary,
Expand All @@ -179,9 +205,10 @@ def py_wheel(name, twine = None, twine_binary = Label("//tools/publish:twine") i
"//conditions:default": "{}.publish_script".format(name),
}),
args = twine_args,
data = [_dist_target],
data = [dist_target],
tags = manual_tags,
visibility = kwargs.get("visibility"),
**copy_propagating_kwargs(kwargs, twine_kwargs)
**copy_propagating_kwargs(kwargs)
)
elif twine:
if not twine.endswith(":pkg"):
Expand All @@ -193,10 +220,11 @@ def py_wheel(name, twine = None, twine_binary = Label("//tools/publish:twine") i
name = "{}.publish".format(name),
srcs = [twine_main],
args = twine_args,
data = [_dist_target],
data = [dist_target],
imports = ["."],
main = twine_main,
deps = [twine],
tags = manual_tags,
visibility = kwargs.get("visibility"),
**copy_propagating_kwargs(kwargs)
)
Expand Down
6 changes: 6 additions & 0 deletions python/private/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,12 @@ py_binary(
],
)

py_binary(
name = "py_wheel_dist",
srcs = ["py_wheel_dist.py"],
visibility = ["//visibility:public"],
)

py_library(
name = "py_console_script_gen_lib",
srcs = ["py_console_script_gen.py"],
Expand Down
41 changes: 41 additions & 0 deletions python/private/py_wheel_dist.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
"""A utility for generating the output directory for `py_wheel_dist`."""

import argparse
import shutil
from pathlib import Path


def parse_args() -> argparse.Namespace:
"""Parse command line arguments."""
parser = argparse.ArgumentParser()

parser.add_argument(
"--wheel", type=Path, required=True, help="The path to a wheel."
)
parser.add_argument(
"--name_file",
type=Path,
required=True,
help="A file containing the sanitized name of the wheel.",
)
parser.add_argument(
"--output",
type=Path,
required=True,
help="The output location to copy the wheel to.",
)

return parser.parse_args()


def main() -> None:
"""The main entrypoint."""
args = parse_args()

wheel_name = args.name_file.read_text(encoding="utf-8").strip()
args.output.mkdir(exist_ok=True, parents=True)
shutil.copyfile(args.wheel, args.output / wheel_name)


if __name__ == "__main__":
main()

0 comments on commit b97a5d6

Please sign in to comment.