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

pycross: Add patching support to py_wheel_library #1436

Merged
merged 27 commits into from
Dec 19, 2023
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
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
17 changes: 17 additions & 0 deletions tests/pycross/0001-Add-new-file-for-testing-patch-support.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
From b2ebe6fe67ff48edaf2ae937d24b1f0b67c16f81 Mon Sep 17 00:00:00 2001
From: Philipp Schrader <[email protected]>
Date: Thu, 28 Sep 2023 09:02:44 -0700
Subject: [PATCH] Add new file for testing patch support

---
site-packages/numpy/file_added_via_patch.txt | 1 +
1 file changed, 1 insertion(+)
create mode 100644 site-packages/numpy/file_added_via_patch.txt

diff --git a/site-packages/numpy/file_added_via_patch.txt b/site-packages/numpy/file_added_via_patch.txt
new file mode 100644
index 0000000..9d947a4
--- /dev/null
+++ b/site-packages/numpy/file_added_via_patch.txt
@@ -0,0 +1 @@
+Hello from a patch!
30 changes: 30 additions & 0 deletions tests/pycross/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,33 @@ py_test(
"//python/runfiles",
],
)

py_wheel_library(
name = "patched_extracted_wheel_for_testing",
patch_args = [
"-p1",
],
patch_tool = "patch",
patches = [
"0001-Add-new-file-for-testing-patch-support.patch",
],
target_compatible_with = select({
# We don't have `patch` available on the Windows CI machines.
"@platforms//os:windows": ["@platforms//:incompatible"],
"//conditions:default": [],
}),
wheel = "@wheel_for_testing//file",
)

py_test(
name = "patched_py_wheel_library_test",
srcs = [
"patched_py_wheel_library_test.py",
],
data = [
":patched_extracted_wheel_for_testing",
],
deps = [
"//python/runfiles",
],
)
38 changes: 38 additions & 0 deletions tests/pycross/patched_py_wheel_library_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Copyright 2023 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import unittest
from pathlib import Path

from python.runfiles import runfiles

RUNFILES = runfiles.Create()


class TestPyWheelLibrary(unittest.TestCase):
def setUp(self):
self.extraction_dir = Path(
RUNFILES.Rlocation("rules_python/tests/pycross/patched_extracted_wheel_for_testing")
)
self.assertTrue(self.extraction_dir.exists(), self.extraction_dir)
self.assertTrue(self.extraction_dir.is_dir(), self.extraction_dir)

def test_patched_file_contents(self):
"""Validate that the patch got applied correctly."""
file = self.extraction_dir / "site-packages/numpy/file_added_via_patch.txt"
self.assertEqual(file.read_text(), "Hello from a patch!\n")


if __name__ == "__main__":
unittest.main()
4 changes: 1 addition & 3 deletions tests/pycross/py_wheel_library_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@
class TestPyWheelLibrary(unittest.TestCase):
def setUp(self):
self.extraction_dir = Path(
RUNFILES.Rlocation(
"rules_python/tests/pycross/extracted_wheel_for_testing"
)
RUNFILES.Rlocation("rules_python/tests/pycross/extracted_wheel_for_testing")
)
self.assertTrue(self.extraction_dir.exists(), self.extraction_dir)
self.assertTrue(self.extraction_dir.is_dir(), self.extraction_dir)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import argparse
import os
import shutil
import subprocess
import sys
import tempfile
from pathlib import Path
Expand Down Expand Up @@ -97,6 +98,30 @@ def main(args: Any) -> None:

setup_namespace_pkg_compatibility(lib_dir)
aignas marked this conversation as resolved.
Show resolved Hide resolved

if args.patch:
if not args.patch_tool and not args.patch_tool_target:
raise ValueError("Specify one of 'patch_tool' or 'patch_tool_target'.")
philsc marked this conversation as resolved.
Show resolved Hide resolved

patch_args = [
args.patch_tool or Path.cwd() / args.patch_tool_target
] + args.patch_arg
patch_dir = args.patch_dir or "."
for patch in args.patch:
with patch.open("r") as stdin:
philsc marked this conversation as resolved.
Show resolved Hide resolved
try:
subprocess.run(
patch_args,
stdin=stdin,
check=True,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
cwd=args.directory / patch_dir,
)
except subprocess.CalledProcessError as error:
print(f"Patch {patch} failed to apply:")
print(error.stdout.decode("utf-8"))
raise


def parse_flags(argv) -> Any:
parser = argparse.ArgumentParser(description="Extract a Python wheel.")
Expand Down Expand Up @@ -127,6 +152,46 @@ def parse_flags(argv) -> Any:
help="The output path.",
)

parser.add_argument(
"--patch",
type=Path,
default=[],
action="append",
help="A patch file to apply.",
)

parser.add_argument(
"--patch-arg",
type=str,
default=[],
action="append",
help="An argument for the patch tool when applying the patches.",
)

parser.add_argument(
"--patch-tool",
type=str,
help=(
"The tool from PATH to invoke when applying patches. "
"If set, --patch-tool-target is ignored."
),
)

parser.add_argument(
"--patch-tool-target",
type=Path,
help=(
"The path to the tool to invoke when applying patches. "
"Ignored when --patch-tool is set."
),
)

parser.add_argument(
"--patch-dir",
type=str,
help="The directory from which to invoke the patch tool.",
)

return parser.parse_args(argv[1:])


Expand Down
57 changes: 55 additions & 2 deletions third_party/rules_pycross/pycross/private/wheel_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,29 @@ def _py_wheel_library_impl(ctx):
args = ctx.actions.args().use_param_file("--flagfile=%s")
args.add("--wheel", wheel_file)
args.add("--directory", out.path)
args.add_all(ctx.files.patches, format_each = "--patch=%s")
args.add_all(ctx.attr.patch_args, format_each = "--patch-arg=%s")
args.add("--patch-tool", ctx.attr.patch_tool)
args.add("--patch-dir", ctx.attr.patch_dir)

inputs = [wheel_file]
tools = []
inputs = [wheel_file] + ctx.files.patches
if name_file:
inputs.append(name_file)
args.add("--wheel-name-file", name_file)

if ctx.attr.patch_tool_target:
args.add("--patch-tool-target", ctx.attr.patch_tool_target.files_to_run.executable)
tools.append(ctx.executable.patch_tool_target)

if ctx.attr.enable_implicit_namespace_pkgs:
args.add("--enable-implicit-namespace-pkgs")

ctx.actions.run(
philsc marked this conversation as resolved.
Show resolved Hide resolved
inputs = inputs,
outputs = [out],
executable = ctx.executable._tool,
tools = tools,
arguments = [args],
# Set environment variables to make generated .pyc files reproducible.
env = {
Expand Down Expand Up @@ -103,7 +113,7 @@ def _py_wheel_library_impl(ctx):
),
]

py_wheel_library = rule(
_py_wheel_library = rule(
implementation = _py_wheel_library_impl,
attrs = {
"deps": attr.label_list(
Expand All @@ -119,6 +129,35 @@ and py_test targets must specify either `legacy_create_init=False` or the global
This option is required to support some packages which cannot handle the conversion to pkg-util style.
""",
),
"patch_args": attr.string_list(
default = ["-p0"],
doc =
"The arguments given to the patch tool. Defaults to -p0, " +
"however -p1 will usually be needed for patches generated by " +
"git. If multiple -p arguments are specified, the last one will take effect.",
),
"patch_dir": attr.string(
default = "",
doc = "The directory from which to invoke the patch_tool.",
),
philsc marked this conversation as resolved.
Show resolved Hide resolved
"patch_tool": attr.string(
philsc marked this conversation as resolved.
Show resolved Hide resolved
doc = "The patch(1) utility from the host to use. " +
philsc marked this conversation as resolved.
Show resolved Hide resolved
"If set, overrides `patch_tool_target`. Please note that setting " +
"this means that builds are not completely hermetic.",
),
"patch_tool_target": attr.label(
executable = True,
cfg = "exec",
doc = "The label of the patch(1) utility to use. " +
"Only used if `patch_tool` is not set.",
),
"patches": attr.label_list(
allow_files = True,
default = [],
doc =
"A list of files that are to be applied as patches after " +
"extracting the archive. This will use the patch command line tool.",
),
"python_version": attr.string(
doc = "The python version required for this wheel ('PY2' or 'PY3')",
values = ["PY2", "PY3", ""],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we should probably remove support for PY2, which is probably outside the scope of this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I can submit a follow-up PR.

Expand All @@ -135,3 +174,17 @@ This option is required to support some packages which cannot handle the convers
),
},
)

def py_wheel_library(patches = None, patch_tool = None, patch_tool_target = None, **kwargs):
# If the user has specified patches, but hasn't specified how to apply
# those patches, default to our internally-provided patch binary.
if patches and not patch_tool and not patch_tool_target:
fail("Patches specified, but missing `patch_tool` or `patch_tool_target` attributes. " +
"I.e. unsure how to apply patches.")

philsc marked this conversation as resolved.
Show resolved Hide resolved
_py_wheel_library(
patches = patches,
patch_tool = patch_tool,
patch_tool_target = patch_tool_target,
**kwargs
)