qmc2 / qmc2-mame-fe

QMC2 - M.A.M.E. Catalog / Launcher II
40 stars 3 forks source link

Update to build with modern minizip-ng #3

Closed belegdol closed 2 years ago

belegdol commented 2 years ago

This should only be merged once the bundled minizip copy is updated.

qmc2 commented 2 years ago

I'd be glad to pull that request, but how about the bundled minizip? That must be "up to date" as well. Otherwise we would be using only external libraries, which I don't like to do. Would you do that as well, that is, exchange the bundled minizip with a new version?

belegdol commented 2 years ago

I can give it try, but I am not sure how you picked which files are included in the bundled copy. It does not seem to be the full minizip source.

qmc2 commented 2 years ago

No, it's not the full source, but we can change that.

qmc2 commented 2 years ago

Oh, and the changes are also required for qmc2-arcade!

EDIT: Ah, okay, you did that already.

belegdol commented 2 years ago

Good point. I will try to get this in a more complete state.

qmc2 commented 2 years ago

Just for your info: We seem to have a major issue with mz_compat.* at least. I can somehow only decompress one file if I'm lucky, the rest gives "MZ_PARAM_ERROR" on the internal mz_zip_entry_read_open(). I have no idea why...

belegdol commented 2 years ago

How can I reproduce this? I have been running qmc2 0.195 patched to use system minizip 3.0 for years without problems. Maybe some more files need to be added?

qmc2 commented 2 years ago

If I knew... I came to checking this after a user told me (unfortunately not here) that with current QMC2 he isn't able to see his "resource files (cabinets, controllers, flyers etc.)" which are zipped, while the version from before (0.242) did show them. I then tried it myself and he is right.

No, I don't think you need to add more files. It happens "for file number 2 and above" here.

qmc2 commented 2 years ago

I've "repacked" them (= made a new zip) and some files are readable from within the zip, but most are not. It's really strange.

belegdol commented 2 years ago

Apologies, I will try to have a look today or tomorrow. The first thing to check would be to see if system minizip is affected. If not, the new function calls use several constants. If I missed a file in which these are defined, this could explain the issue.

qmc2 commented 2 years ago

No reason to apology. I was surprised myself... yeah, if I find something earlier I'll write here.

belegdol commented 2 years ago

I can reproduce this. With system minizip-3.0.2 the problem goes away. So it is either a 3.0.5-specific issue, or I have indeed missed some required files.

qmc2 commented 2 years ago

Okay, then let's see..,

qmc2 commented 2 years ago

BTW, regarding "missing files"... it's probably best to include all .c / .h files because I don't know which ones I'd need for Windows (probably soon) and macOS (later).

belegdol commented 2 years ago

System-wide minizip-3.0.5 works as well. So it does indeed look that I have missed some files.

qmc2 commented 2 years ago

Yeah. So it's not the way we use it (old style aka "compat") at least. Nice.

belegdol commented 2 years ago

I am afraid fixing this might be beyond my capabilities. I tried debugging in qt creator but since I do not know C++ this not really getting anywhere. What I have noticed is that the imageFile variable passed to unzOpenCurrentFile in line 328 of imagewidget.cpp is 8 digits long when using builtin minizip, but 14 digits long when using the system one.

qmc2 commented 2 years ago

Ok. I'll try to do it myself then, probably tomorrow.

With digits you mean bytes? Because imageFile is of type unzFile and this is a C 'struct'. Anyway, an interesting observation.

belegdol commented 2 years ago

I mean that in the expression evaluator, qt creator shows e.g. 44040224 with builtin minizip and e.g. 9479658776979 with system one. Might be nothing though.

qmc2 commented 2 years ago

Ah, yes.

qmc2 commented 2 years ago

I got it!! We have to define HAVE_ZLIB and ZLIB_COMPAT at build time, this brings up a new dependency - mz_strm_zlib.c. Adding this file as well and the issue gets solved.

I will check a few things and commit my changes after it.

qmc2 commented 2 years ago

This now also builds on Windows :)! With the usual MAME tools and an additional webkit package...

belegdol commented 2 years ago

Nice, defines were the things I was going to check next. For the record, system minizip on Fedora is built with following defines:

-DHAVE_BZIP2 -DHAVE_ICONV -DHAVE_INTTYPES_H -DHAVE_LZMA -DHAVE_PKCRYPT -DHAVE_STDINT_H -DHAVE_WZAES -DHAVE_ZLIB -DHAVE_ZSTD -DLZMA_API_STATIC -DMZ_EXPORTS -DMZ_ZIP_SIGNING -DZLIB_COMPAT -D_POSIX_C_SOURCE=200112L
qmc2 commented 2 years ago

That's not everything... I just found out that the "ROMAlyzer" somehow gets this on checksum scanning:

...
17:03:57.359: scan started for file '/home/games/mame/roms/3dobios.zip'
17:03:57.367: ZIP scan: member '' from archive '/home/games/mame/roms/3dobios.zip' has SHA-1 '3c912300775d1ad730dc35757e279c274c0acaad' and CRC '58242cee'
17:03:57.373: ZIP scan: member '' from archive '/home/games/mame/roms/3dobios.zip' has SHA-1 'c4a2e5336f77fb5f743de1eea2cda43675ee2de7' and CRC 'b6f5028b'
17:03:57.379: ZIP scan: member '' from archive '/home/games/mame/roms/3dobios.zip' has SHA-1 '34bf189111295f74d7b7dfc1f304d98b8d36325a' and CRC 'c8c8ff89'
17:03:57.385: ZIP scan: member '' from archive '/home/games/mame/roms/3dobios.zip' has SHA-1 'b01c53da256dde43ffec4ad3fc3adfa8d635e943' and CRC 'd5cbc509'
17:03:57.385: database update: an object with SHA-1 '3c912300775d1ad730dc35757e279c274c0acaad' and CRC '58242cee' already exists in the database, member '' from archive '/home/games/mame/roms/3dobios.zip' ignored
17:03:57.385: database update: an object with SHA-1 'c4a2e5336f77fb5f743de1eea2cda43675ee2de7' and CRC 'b6f5028b' already exists in the database, member '' from archive '/home/games/mame/roms/3dobios.zip' ignored
17:03:57.385: database update: an object with SHA-1 '34bf189111295f74d7b7dfc1f304d98b8d36325a' and CRC 'c8c8ff89' already exists in the database, member '' from archive '/home/games/mame/roms/3dobios.zip' ignored
17:03:57.385: database update: an object with SHA-1 'b01c53da256dde43ffec4ad3fc3adfa8d635e943' and CRC 'd5cbc509' already exists in the database, member '' from archive '/home/games/mame/roms/3dobios.zip' ignored
17:03:57.385: scan finished for file '/home/games/mame/roms/3dobios.zip'
...

Something for tomorrow :).

qmc2 commented 2 years ago

I opened https://github.com/qmc2/qmc2-mame-fe/issues/7 so it doesn't get lost.