Skip to content

Commit

Permalink
fix(whl_library): actually apply patches and warn if RECORD file patc…
Browse files Browse the repository at this point in the history
…h is needed (#1637)

Before this PR there was a typo, that was actually causing the
patching function to not use the provided patches. With this change
we are finally correctly doing it. After fixing this bug I noticed
that `repository_ctx.patch` results in files with `CRLF` on Windows,
which made me make the `RECORD` mismatch to be a warning rather than
a hard failure to make the CI happy and allow users on Windows to
patch wheels but see a warning if they have a multi-platform bazel
setup.

The `CRLF` endings on Windows issue is fixed in
bazelbuild/bazel@07e0d31

Related #1631, #1639.
  • Loading branch information
aignas authored Dec 21, 2023
1 parent 83e9255 commit 10150e5
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 3 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ A brief description of the categories of changes:
specifying a local system interpreter.
* (bzlmod pip.parse) Requirements files with duplicate entries for the same
package (e.g. one for the package, one for an extra) now work.
* (whl_library) Actually use the provided patches to patch the whl_library.
On Windows the patching may result in files with CRLF line endings, as a result
the RECORD file consistency requirement is lifted and now a warning is emitted
instead with a location to the patch that could be used to silence the warning.
Copy the patch to your workspace and add it to the list if patches for the wheel
file if you decide to do so.

### Added

Expand Down
16 changes: 16 additions & 0 deletions examples/bzlmod/whl_mods/pip_whl_mods_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,22 @@ def test_extra(self):
content = generated_file.read_text().rstrip()
self.assertEqual(content, "Hello world from requests")

def test_patches(self):
current_wheel_version = "2.25.1"

# This test verifies that the patches are applied to the wheel.
r = runfiles.Create()
metadata_path = "{}/site-packages/requests-{}.dist-info/METADATA".format(
self._requests_pkg_dir,
current_wheel_version,
)

metadata = Path(r.Rlocation(metadata_path))
self.assertIn(
"Summary: Python HTTP for Humans. Patched.",
metadata.read_text().splitlines(),
)


if __name__ == "__main__":
unittest.main()
2 changes: 1 addition & 1 deletion python/pip_install/pip_repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,7 @@ def _whl_library_impl(rctx):

if rctx.attr.whl_patches:
patches = {}
for patch_file, json_args in patches.items():
for patch_file, json_args in rctx.attr.whl_patches.items():
patch_dst = struct(**json.decode(json_args))
if whl_path.basename in patch_dst.whls:
patches[patch_file] = patch_dst.patch_strip
Expand Down
19 changes: 19 additions & 0 deletions python/private/patch_whl.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,15 @@ def patch_whl(rctx, *, python_interpreter, whl_path, patches, **kwargs):
parsed_whl.platform_tag,
]))

record_patch = rctx.path("RECORD.patch")

result = rctx.execute(
[
python_interpreter,
"-m",
"python.private.repack_whl",
"--record-patch",
record_patch,
whl_input,
whl_patched,
],
Expand All @@ -97,4 +101,19 @@ def patch_whl(rctx, *, python_interpreter, whl_path, patches, **kwargs):
),
)

if record_patch.exists:
record_patch_contents = rctx.read(record_patch)
warning_msg = """WARNING: the resultant RECORD file of the patch wheel is different
If you are patching on Windows, you may see this warning because of
a known issue (bazelbuild/rules_python#1639) with file endings.
If you would like to silence the warning, you can apply the patch that is stored in
{record_patch}. The contents of the file are below:
{record_patch_contents}""".format(
record_patch = record_patch,
record_patch_contents = record_patch_contents,
)
print(warning_msg) # buildifier: disable=print

return rctx.path(whl_patched)
11 changes: 9 additions & 2 deletions python/private/repack_whl.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ def main(sys_argv):
type=pathlib.Path,
help="The original wheel file that we have patched.",
)
parser.add_argument(
"--record-patch",
type=pathlib.Path,
help="The output path that we are going to write the RECORD file patch to.",
)
parser.add_argument(
"output",
type=pathlib.Path,
Expand Down Expand Up @@ -163,8 +168,10 @@ def main(sys_argv):
got_record,
out.distinfo_path("RECORD"),
)
logging.exception(f"Please also patch the RECORD file with:\n{record_diff}")
return 1
args.record_patch.write_text(record_diff)
logging.warning(
f"Please apply patch to the RECORD file ({args.record_patch}):\n{record_diff}"
)


if __name__ == "__main__":
Expand Down

0 comments on commit 10150e5

Please sign in to comment.