tests: Fix tests with 7zip disabled

Genivia / ugrep

NEW ugrep 6.5: a more powerful, ultra fast, user-friendly, compatible grep. Includes a TUI, Google-like Boolean search with AND/OR/NOT, fuzzy search, hexdumps, searches (nested) archives (zip, 7z, tar, pax, cpio), compressed files (gz, Z, bz2, lzma, xz, lz4, zstd, brotli), pdfs, docs, and more

https://ugrep.com

BSD 3-Clause "New" or "Revised" License

2.57k stars 109 forks source link

tests: Fix tests with 7zip disabled #344

Closed ribalda closed 8 months ago

ribalda commented 8 months ago

If 7zip is not built we cannot check the .7z file

genivia-inc commented 8 months ago

Whoops. The 7z test was added later. Forgot to include a check for when 7zip is explicitly disabled, like for the other cases. 7zip was a bit of a pain to support and its internals aren't great. Got tired of this and wanted to move on. Will do better next time.

ribalda commented 8 months ago

Np . Thanks for your fast work to merge this.

Just fyi, I have disabled 7zip in arm, PowerPC and x32, they are failing to pass the test:

https://salsa.debian.org/ribalda/ugrep/-/commit/2201e17ce3b5da4e56c9c9b05161680727d0a099

There are some patches from Debian that might be useful for you... https://salsa.debian.org/debian/7zip/-/tree/master/debian/patches?ref_type=heads

Regards!

On Tue, 9 Jan 2024, 16:06 Dr. Robert van Engelen, @.***> wrote:

Merged #344 https://github.com/Genivia/ugrep/pull/344 into master.

— Reply to this email directly, view it on GitHub https://github.com/Genivia/ugrep/pull/344#event-11431607161, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEJJGZMMT24263LERPKSZDYNVMGRAVCNFSM6AAAAABBS72E4KVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJRGQZTCNRQG4YTMMI . You are receiving this because you authored the thread.Message ID: @.***>

genivia-inc commented 8 months ago

Thank you for sharing the link to the patches. There is only one C file that to which a patch can be applied C/7zCrc.c. The other patches apply to files I'm not using. I'm not enabling assembly code compilation nor crypto functions, which we don't need. This won't fix the problem with PPC and ARM32.

I wrote a Makefile.am based on the C/7zip_gcc_c.mak by looking into:

7zip_gcc_c.mak
var_clang.mak
var_clang_arm64.mak
var_clang_x64.mak
var_clang_x86.mak
var_gcc.mak
var_gcc_arm64.mak
var_gcc_x64.mak
var_gcc_x86.mak
var_mac_arm64.mak
var_mac_x64.mak

All of these system-specific makefiles add defines that we do not use for this part of 7zip LZMA SDK.

We will need to dig a bit deeper to find out how to support PPC and ARM32.

genivia-inc commented 8 months ago

I'm able to replicate the problem on an RPi ARM, so I'm on it to figure this out. Unfortunately, GDB doesn't tell me anything specific (stack corrupt?).

genivia-inc commented 8 months ago

Found the problem. The problem is a combination of 7zip LZMA SDK's internals and the way I've implemented option -M to match "magic bytes" (file types etc). This match is simpler than a full search of a compressed file or archive. This is when the LZMA SDK code breaks on 32 bit systems.

genivia-inc commented 8 months ago

Released 4.5.2 with the updates. I've successfully tested ugrep on RPi ARM 32 bit and Windows x86 32 bit to check if the 7zip fix works, which I had no doubt should work fine now.