fix(py_wheel): Quote wheel RECORD file entry elements if needed#2269
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
The change in general LGTM, but the CI failure is a little weird. Don't have to look into this in-depth now though. |
kcon-stackav
force-pushed
the
kevin/fix-pip-override-for-commas-in-filenames
branch
2 times, most recently
from
f3642a7 to
1717016
Compare
kcon-stackav
force-pushed
the
kevin/fix-pip-override-for-commas-in-filenames
branch
from
1717016 to
1c77342
Compare
kcon-stackav
changed the title
|
Could you please add a unit test in the I am not sure this is |
kcon-stackav
force-pushed
the
kevin/fix-pip-override-for-commas-in-filenames
branch
from
1c77342 to
dd5112c
Compare
kcon-stackav
changed the title
kcon-stackav
force-pushed
the
kevin/fix-pip-override-for-commas-in-filenames
branch
from
dd5112c to
5bfa599
Compare
aignas
left a comment
aignas
left a comment
There was a problem hiding this comment.
LGTM, thank you for adding the tests.
When patching a single file in Pytorch, repack_whl.py will print over 20k lines of RECORD.patch. The reason is that the original RECORD has all filenames quoted for some reason, but the automatically generated one quotes only when required (such as commas in file names, see #2269). This PR refactors the wheelmaker.py to still use the csv.writer to auto-quote, but adds an additional detection for forced quote usage. This makes the RECORD.patch match intuitive expectations of what could change. There are some relevant existing tests in examples/wheel/wheel_test.py and tests/whl_filegroup/extract_wheel_files_test.py --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Ignas Anikevicius <[email protected]>
2 participants
The
RECORDfile written when wheels are patched usingpip.override()is not quoting filenames as needed, so wheels that (unfortunately) include files whose name contain commas would be written in such a way that https://pypi.org/project/installer/ would fail to parse them, resulting in an error like:This PR fixes that by using
csvto read and writeRECORDfile entries which takes care of quoting elements of record entries as needed. See PEP376 for more info about theRECORDfile format here: https://peps.python.org/pep-0376/#recordFixes #2261