From 10150e519b6683d819c1cfa7035b56a7ad5213fd Mon Sep 17 00:00:00 2001
From: Ignas Anikevicius <240938+aignas@users.noreply.github.com>
Date: Thu, 21 Dec 2023 16:04:29 +0900
Subject: [PATCH] fix(whl_library): actually apply patches and warn if RECORD
 file patch 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@07e0d316a345a3cb2593f98525320590bbc56e30

Related #1631, #1639.
---
 CHANGELOG.md                                  |  6 ++++++
 examples/bzlmod/whl_mods/pip_whl_mods_test.py | 16 ++++++++++++++++
 python/pip_install/pip_repository.bzl         |  2 +-
 python/private/patch_whl.bzl                  | 19 +++++++++++++++++++
 python/private/repack_whl.py                  | 11 +++++++++--
 5 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b524536b8c..ee5421f790 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
 
diff --git a/examples/bzlmod/whl_mods/pip_whl_mods_test.py b/examples/bzlmod/whl_mods/pip_whl_mods_test.py
index a88134b150..3d7d161f1f 100644
--- a/examples/bzlmod/whl_mods/pip_whl_mods_test.py
+++ b/examples/bzlmod/whl_mods/pip_whl_mods_test.py
@@ -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()
diff --git a/python/pip_install/pip_repository.bzl b/python/pip_install/pip_repository.bzl
index dca36ce74c..b897e8c56a 100644
--- a/python/pip_install/pip_repository.bzl
+++ b/python/pip_install/pip_repository.bzl
@@ -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
diff --git a/python/private/patch_whl.bzl b/python/private/patch_whl.bzl
index 24b8a0b565..9e3119f744 100644
--- a/python/private/patch_whl.bzl
+++ b/python/private/patch_whl.bzl
@@ -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,
         ],
@@ -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)
diff --git a/python/private/repack_whl.py b/python/private/repack_whl.py
index 074e30db74..be113ef791 100644
--- a/python/private/repack_whl.py
+++ b/python/private/repack_whl.py
@@ -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,
@@ -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__":