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

Add partial (listing and decompressing only) support for RAR #529

Merged
merged 3 commits into from
Nov 15, 2023

Conversation

lmkra
Copy link
Contributor

@lmkra lmkra commented Sep 26, 2023

Attempt to address #152 using https://github.com/muja/unrar.rs crate.

CHANGELOG.md Outdated Show resolved Hide resolved
@lmkra
Copy link
Contributor Author

lmkra commented Oct 1, 2023

ARMv7 CI failure seems to be an upstream issue: muja/unrar.rs#35

@marcospb19
Copy link
Member

marcospb19 commented Oct 1, 2023

Thanks for the follow ups!

I temporarily disabled ARMv7 to see if CI succeeds in other targets, it's failing on Windows, let me know whether you know how to fix it.

@lmkra
Copy link
Contributor Author

lmkra commented Oct 3, 2023

MSVC target on Windows should work fine, it's only MinGW one that fails.
I've proposed a fix for that: muja/unrar.rs#37

In the meantime I've temporarily switched used unrar.rs version to my own fork, which contains all of upstream-proposed fixes.

@lmkra
Copy link
Contributor Author

lmkra commented Oct 4, 2023

Situation now seems to be that:

  • x86_64-pc-windows-gnu target builds correctly
  • ...but integrations tests fail due to ouch binary being dynamically linked to libstdc++-6.dll, which is provided by MinGW compiler and is not found in standard Windows shell search path.

While setting up $PATH variable to MinGW install directory or shipping aforementioned library together with ouch.exe could be considered as a workarounds, but I think that better solution would be to statically link against libstdc++ in ouch executable.
Just need to find out how to convince cargo to do it...

@marcospb19
Copy link
Member

Honestly, I'm absolutely clueless 😄 .

@lmkra
Copy link
Contributor Author

lmkra commented Oct 5, 2023

Seems to be working now 😃

Copy link
Member

@marcospb19 marcospb19 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be working now 😃

Amazing!

I think it's better not to merge right now because it would block new Ouch releases from happening, I can only publish a version on crates.io when all dependencies are fetched from crates.io itself, I guess we'll have to wait for a new unrar release (but we don't know when that comes out).

Does that make sense to you? 🤔

@lmkra
Copy link
Contributor Author

lmkra commented Oct 8, 2023

Thanks!

And yes, that makes perfect sense to me 👍

@lmkra
Copy link
Contributor Author

lmkra commented Nov 12, 2023

New unrar has finally landed on crates.io :)
Therefore I've rebased this PR against current master branch and removed all temporary patching.

@marcospb19
Copy link
Member

That's awesome! Thank you for keeping an eye on that.

Cargo.toml Outdated Show resolved Hide resolved
@lmkra lmkra force-pushed the unrar-support branch 2 times, most recently from 9e11383 to b8583d7 Compare November 12, 2023 18:13
@lmkra lmkra requested a review from marcospb19 November 12, 2023 18:17
Copy link
Member

@marcospb19 marcospb19 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@marcospb19 marcospb19 merged commit 1aba1a2 into ouch-org:main Nov 15, 2023
12 checks passed
@lmkra lmkra deleted the unrar-support branch November 15, 2023 12:00
@marcospb19 marcospb19 mentioned this pull request Oct 22, 2024
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.

2 participants