Support unpacking wheels that contain files with commas in their names

[hoodmane]

I am trying to move Pyodide over to using wheels to distribute packages pyodide/pyodide#2027. After building the wheel, we need to do some postprocessing so we use python -m wheel unpack wheel-name.whl, then rearrange the wheel as appropriate, then pack the wheel again. The PyPa wheel package does not correctly handle commas in file names.

In particular, python -m wheel unpack breaks on the statsmodels wheel because of the following files:

statsmodels/datasets/tests/raw.github.com,vincentarelbundock,Rdatasets,master,csv,car,Duncan.csv.zip
statsmodels/datasets/tests/raw.github.com,vincentarelbundock,Rdatasets,master,datasets.csv.zip
statsmodels/datasets/tests/raw.github.com,vincentarelbundock,Rdatasets,master,doc,car,rst,Duncan.rst.zip

This PR fixes the problem.

[pfmoore]

The correct fix here is probably to use the stdlib csv library - the RECORD file is defined as being csv format, so using the library is far better than hand-parsing the format.

[agronholm]

Yes, I was just thinking the same.

[hoodmane]

Okay, I will do that. This was just the smallest code change that fixes my problem. =)

[hoodmane]

Also, the code coverage report lists line 79 that I added to wheelfile.py as not covered. Did I not add the test correctly? It certainly breaks locally without my change and works with it.

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (main@0acd203). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #427   +/-   ##
=======================================
  Coverage        ?   80.85%           
=======================================
  Files           ?       12           
  Lines           ?      982           
  Branches        ?        0           
=======================================
  Hits            ?      794           
  Misses          ?      188           
  Partials        ?        0           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0acd203...363ba2f. Read the comment docs.

[agronholm]

Looks good, just a couple nits.

[hoodmane]

Thanks for the very prompt review!

[hoodmane]

There's an odd set of test failures. Maybe there was a reason that you weren't using csv here?

[agronholm]

A couple more nits.

[agronholm]

Supremely annoying to have the test suite fail on Python 2.7. I really need to ditch that, but until I get the time, I have to deal with those test failures.

[agronholm]

With that latest change, tests now pass on Python 2.7 but not on Windows. Investigating.

[pfmoore]

It's probably because record should be opened in binary mode. You don't want to wrap a text file object in a TextIOWrapper...

[agronholm]

But it is being opened in binary mode. You can't open files from a ZipFile any other way.

[pfmoore]

Oops sorry. I was reading the code in GitHub and didn’t expand to get enough context. Apologies for the noise.

[pfmoore]

Are you assuming that the default encoding for TextIOWrapper is utf-8? That isn’t always true on Windows, so that might be the issue. (To be clear, this is just something I often see go wrong on windows, I’ve not had time to review the actual test failure. I’ll try to take a proper look later.)

[agronholm]

Are you assuming that the default encoding for TextIOWrapper is utf-8? That isn’t always true on Windows, so that might be the issue. (To be clear, this is just something I often see go wrong on windows, I’ve not had time to review the actual test failure. I’ll try to take a proper look later.)

I suspected that when I saw the failures on Windows, but I haven't had a chance to test that assumption yet. I will look into it later today (unless OP resolves it first).

[agronholm]

Yup, that was it. Thanks @hoodmane !

[hoodmane]

Thanks @agronholm and @pfmoore!