Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: Fix tests with 7zip disabled #344

Merged
merged 1 commit into from
Jan 9, 2024
Merged

Conversation

ribalda
Copy link
Contributor

@ribalda ribalda commented Jan 9, 2024

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

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

Signed-off-by: Ricardo Ribalda <ricardo@ribalda.com>
@genivia-inc
Copy link
Member

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.

@genivia-inc genivia-inc merged commit 66c3879 into Genivia:master Jan 9, 2024
3 checks passed
@ribalda
Copy link
Contributor Author

ribalda commented Jan 9, 2024 via email

@genivia-inc
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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 added a commit that referenced this pull request Jan 10, 2024
- PR #344
- PR #343
- fix 7zip search on 32 bit systems when option -M (or -t setting -M) is used
@genivia-inc
Copy link
Member

genivia-inc commented Jan 10, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants