clalancette / pycdlib

Python library to read and write ISOs
GNU Lesser General Public License v2.1
147 stars 38 forks source link

Fix error while reading directory with shift_jis encoded name #124

Open Darkhood148 opened 7 months ago

Darkhood148 commented 7 months ago

This PR aims to fix the issue described here: https://github.com/clalancette/pycdlib/issues/109. There seems to be a default encoding of utf-8 at many places which causes an error if shift_jis is required. I have fixed it by adding another parameter for encoding while keeping its default value to utf-8 to avoid changing the definition everywhere.

Darkhood148 commented 7 months ago

@clalancette Can you look into it please?

clalancette commented 7 months ago

@clalancette Can you look into it please?

Sorry for the delay here.

The idea in general here is fine. But this ends up failing many of the builtin tests as-is; you can see that both in the CI jobs, and if you run make tests locally. Once all of the tests are passing, we can consider putting this in.

Darkhood148 commented 7 months ago

@clalancette I have made the changes. Some tests are still failing but those tests are also failing in the master branch which makes me believe that those might be related to another issue unrelated to my PR. With that in mind, could you please review it again?

clalancette commented 7 months ago

@clalancette I have made the changes. Some tests are still failing but those tests are also failing in the master branch which makes me believe that those might be related to another issue unrelated to my PR. With that in mind, could you please review it again?

As far as I know, there are no tests failing in the master branch currently, at least on Fedora. Also if you rebase this onto the latest, we'll be able to run full CI on Ubuntu 22.04, which is also passing.

Darkhood148 commented 7 months ago

@clalancette I have rebased my commit on the latest commits. Although, still the exactly 8 tests are failing on my machine (Ubuntu 22.04). Tests that fail on master branch:

FAILED tests/integration/test_hybrid.py::test_hybrid_sevendeepdirs - assert 2 == 3
FAILED tests/integration/test_parse.py::test_parse_rr_symlink_broken - FileNotFoundError: [Errno 2] No such file or directory: '/tmp/pytest-of-darkhood148/pytest-3/test_parse_rr_symlink_broken0/...
FAILED tests/integration/test_parse.py::test_parse_rr_deep_dir - assert 3 == 4
FAILED tests/integration/test_parse.py::test_parse_rr_deep - assert 3 == 4
FAILED tests/integration/test_parse.py::test_parse_rr_deep2 - assert 3 == 4
FAILED tests/integration/test_parse.py::test_parse_rr_joliet_deep - assert 3 == 4
FAILED tests/integration/test_parse.py::test_parse_rr_deeper_dir - assert 3 == 5
FAILED tests/integration/test_parse.py::test_parse_rr_hidden_relocated - assert 3 == 4

Tests that fail on my branch:

FAILED tests/integration/test_hybrid.py::test_hybrid_sevendeepdirs - assert 2 == 3
FAILED tests/integration/test_parse.py::test_parse_rr_symlink_broken - FileNotFoundError: [Errno 2] No such file or directory: '/tmp/pytest-of-darkhood148/pytest-3/test_parse_rr_symlink_broken0/...
FAILED tests/integration/test_parse.py::test_parse_rr_deep_dir - assert 3 == 4
FAILED tests/integration/test_parse.py::test_parse_rr_deep - assert 3 == 4
FAILED tests/integration/test_parse.py::test_parse_rr_deep2 - assert 3 == 4
FAILED tests/integration/test_parse.py::test_parse_rr_joliet_deep - assert 3 == 4
FAILED tests/integration/test_parse.py::test_parse_rr_deeper_dir - assert 3 == 5
FAILED tests/integration/test_parse.py::test_parse_rr_hidden_relocated - assert 3 == 4

No new tests are failing on my branch

Darkhood148 commented 6 months ago

Heyy @clalancette . Could you look into this?