8000 fix(whl_library): actually apply patches and warn if RECORD file patc… · tanan/rules_python@10150e5 · GitHub
[go: up one dir, main page]
Skip to content

Commit 10150e5

Browse files
aignasaignas
authored
fix(whl_library): actually apply patches and warn if RECORD file patch is needed (bazel-contrib#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 bazel-contrib#1631, bazel-contrib#1639.
1 parent 83e9255 commit 10150e5

File tree

5 files changed

+51
-3
lines changed

5 files changed

+51
-3
lines changed

‎CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,12 @@ A brief description of the categories of changes:
4646
specifying a local system interpreter.
4747
* (bzlmod pip.parse) Requirements files with duplicate entries for the same
4848
package (e.g. one for the package, one for an extra) now work.
49+
* (whl_library) Actually use the provided patches to patch the whl_library.
50+
On Windows the patching may result in files with CRLF line endings, as a result
51+
the RECORD file consistency requirement is lifted and now a warning is emitted
52+
instead with a location to the patch that could be used to silence the warning.
53+
Copy the patch to your workspace and add it to the list if patches for the wheel
54+
file if you decide to do so.
4955

5056
### Added
5157

‎examples/bzlmod/whl_mods/pip_whl_mods_test.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,22 @@ def test_extra(self):
130130
content = generated_file.read_text().rstrip()
131131
self.assertEqual(content, "Hello world from requests")
132132

133+
def test_patches(self):
134+
current_wheel_version = "2.25.1"
135+
136+
# This test verifies that the patches are applied to the wheel.
137+
r = runfiles.Create()
138+
metadata_path = "{}/site-packages/requests-{}.dist-info/METADATA".format(
139+
self._requests_pkg_dir,
140+
current_wheel_version,
141+
)
142+
143+
metadata = Path(r.Rlocation(metadata_path))
144+
self.assertIn(
145+
"Summary: Python HTTP for Humans. Patched.",
146+
metadata.read_text().splitlines(),
147+
)
148+
133149

134150
if __name__ == "__main__":
135151
unittest.main()

‎python/pip_install/pip_repository.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -729,7 +729,7 @@ def _whl_library_impl(rctx):
729729

730730
if rctx.attr.whl_patches:
731731
patches = {}
732-
for patch_file, json_args in patches.items():
732+
for patch_file, json_args in rctx.attr.whl_patches.items():
733733
patch_dst = struct(**json.decode(json_args))
734734
if whl_path.basename in patch_dst.whls:
735735
patches[patch_file] = patch_dst.patch_strip

‎python/private/patch_whl.bzl

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,15 @@ def patch_whl(rctx, *, python_interpreter, whl_path, patches, **kwargs):
7373
parsed_whl.platform_tag,
7474
]))
7575

76+
record_patch = rctx.path("RECORD.patch")
77+
7678
result = rctx.execute(
7779
[
7880
python_interpreter,
7981
"-m",
8082
"python.private.repack_whl",
83+
"--record-patch",
84+
record_patch,
8185
whl_input,
8286
whl_patched,
8387
],
@@ -97,4 +101,19 @@ def patch_whl(rctx, *, python_interpreter, whl_path, patches, **kwargs):
97101
),
98102
)
99103

104+
if record_patch.exists:
105+
record_patch_contents = rctx.read(record_patch)
106+
warning_msg = """WARNING: the resultant RECORD file of the patch wheel is different
107+
108+
If you are patching on Windows, you may see this warning because of
109+
a known issue (bazelbuild/rules_python#1639) with file endings.
110+
111+
If you would like to silence the warning, you can apply the patch that is stored in
112+
{record_patch}. The contents of the file are below:
113+
{record_patch_contents}""".format(
114+
record_patch = record_patch,
115+
record_patch_contents = record_patch_contents,
116+
)
117+
print(warning_msg) # buildifier: disable=print
118+
100119
return rctx.path(whl_patched)

‎python/private/repack_whl.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,11 @@ def main(sys_argv):
113113
type=pathlib.Path,
114114
help="The original wheel file that we have patched.",
115115
)
116+
parser.add_argument(
117+
"--record-patch",
118+
type=pathlib.Path,
119+
help="The output path that we are going to write the RECORD file patch to.",
120+
)
116121
parser.add_argument(
117122
"output",
118123
type=pathlib.Path,
@@ -163,8 +168,10 @@ def main(sys_argv):
163168
got_record,
164169
out.distinfo_path("RECORD"),
165170
)
166-
logging.exception(f"Please also patch the RECORD file with:\n{record_diff}")
167-
return 1
171+
args.record_patch.write_text(record_diff)
172+
logging.warning(
173+
f"Please apply patch to the RECORD file ({args.record_patch}):\n{record_diff}"
174+
)
168175

169176

170177
if __name__ == "__main__":

0 commit comments

Comments
 (0)
0