chore(unrar_sys): upgrade unrar source code version to 7.0.9

[ttys3]

update to https://www.rarlab.com/rar/unrarsrc-7.0.9.tar.gz

update 20240401:

I think we need some macro to avoid SSE on old macOS machines.

the error :

ld: warning: object file (/Users/runner/work/unrar.rs/unrar.rs/target/debug/deps/libunrar_sys-04ff564a1c542626.rlib(01e88f00c6928482-rarvm.o)) was built for newer macOS version (13.1) than being linked (12.0)

ld: warning: object file (/Users/runner/work/unrar.rs/unrar.rs/target/debug/deps/libunrar_sys-482deafb002ad51f.rlib(01e88f00c6928482-rarvm.o)) was built for newer macOS version (12.1) than being linked (10.8)

is confusing, but not the root cause.

https://github.com/tensorflow/tensorflow/issues/7223

https://github.com/tensorflow/tensorflow/pull/7241/files

according to https://github.com/randombit/botan/issues/797#issuecomment-271115382

Clang defines both GNUC and GNUG to trick either (or both) glibc and libstdc++

which caused call to __builtin_cpu_supports triggered the ___cpu_model undefined error:

= note: Undefined symbols for architecture x86_64:
            "___cpu_model", referenced from:
                Rijndael::Init(bool, unsigned char const*, unsigned int, unsigned char const*) in libunrar_sys-04ff564a1c54[26](https://github.com/ttys3/unrar.rs/actions/runs/8509450473/job/23304997946#step:5:27)26.rlib(01e88f00c6928482-rijndael.o)
          ld: symbol(s) not found for architecture x86_64

[ttys3]

updated, PTAL

[muja]

Hey, thank you for your work. Did you follow the steps in unrar_sys's README, specifically applying the patches from the list?

[ttys3]

Yes. I do follow the docs. Currently the Mac tests fail. That's strange.

I use the ./upgrade.sh script to update the tarball

[ttys3]

the CI failure is related to old version llvm.

I think we need some macro to avoid SSE on old macOS machines.

https://github.com/tensorflow/tensorflow/issues/7223

https://github.com/tensorflow/tensorflow/pull/7241/files

[ttys3]

@muja the tests should OK now, PTAL

[ttys3]

@muja ping

[muja]

PTAL?

[ttys3]

PTAL?

Please take another look

[muja]

Hey @ttys3, I've been in "hiatus" and had a look at the PR. I have a few comments/questions

1. Why did you change the OS X target from 10.8 to 10.12?
2. Why were the patches necessary?
3. One of the commit hashes (5cf..) in the patches.txt is invalid, I believe due to a force-push?
4. In the meantime, rarlab has released 7.0.9 -- it probably makes sense to upgrade to that.

[ttys3]

Hey @ttys3, I've been in "hiatus" and had a look at the PR. I have a few comments/questions

1. Why did you change the OS X target from 10.8 to 10.12?

2. Why were the patches necessary?

3. One of the commit hashes (5cf..) in the patches.txt is invalid, I believe due to a force-push?

4. In the meantime, rarlab has released 7.0.9 -- it probably makes sense to upgrade to that.

5. Why did you change the OS X target from 10.8 to 10.12?

see https://github.com/muja/unrar.rs/pull/48/commits/19ef5b8cf7bf3cceab4b81cf08921502ceafcc98

the commit message described this:

avoid "macOS deployment target (10.8) too low, it will be increased" warning according to https://blog.rust-lang.org/2023/09/25/Increasing-Apple-Version-Requirements.html The new minimum versions are now: macOS: 10.12 Sierra (First released 2016)

2. Why were the patches necessary?

all the patches is in order to fix issued introduced by new version of rar, but the issues mostly related to aarch64 or musl libc or macOS

each patch has a related commit.

3. One of the commit hashes (5cf..) in the patches.txt is invalid, I believe due to a force-push?

now fixed

4. In the meantime, rarlab has released 7.0.9 -- it probably makes sense to upgrade to that.

done.

[muja]

Thank you so much, looks good, I'll merge this but before I'd like to change some commit types. Since most of the fixes here are related to regressions directly introduced by the upgrade, I wouldn't tag them as such. The fix type is for bugs that were fixed since previous releases of the crate (since they will appear as such in the changelog). I have also noticed some additional conventional-commit style messages in the body. This interestingly isn't allowed according to the spec.

We can also squash the update commits to reduce noise.

Finally, I would completely omit the patches.txt from this PR. Since commit hashes are prone to change, I will simply update the patches.txt directly on master after the PR gets merged

To make this easier for you I've prepared a little script which should make everything work:

First, you need to reset to origin/master (important, make sure your dir is not dirty):

git fetch https://github.com/muja/unrar.rs master
git status --porcelain | grep -v '^??' >/dev/null && echo "git repository must not be dirty" || git reset --hard 7fc7b3121261e8a105569607b9217fa9fa643dbe

Then run the following commands, they should fix the commits:

git cherry-pick 1f12c48 # chore(unrar_sys): upgrade unrar source code version to 7.0.7
git cherry-pick --no-commit a8fa134 # chore: update to latest UnRAR 7.0.9
git cherry-pick --no-commit 0975ac2 # test: fix dll version tests
git commit --amend -m 'chore(dll): upgrade unrar source code version to 7.0.9'
git cherry-pick 94c0d58 # fix: avoid call to __builtin_cpu_supports triggered the ___cpu_model undefined error on macOS
git commit --amend -F- <<EOF
chore(dll): fix build by avoiding call to __builtin_cpu_supports

This fixes the ___cpu_model undefined error on macOS
EOF
git cherry-pick 19ef5b8 # ci: increase the minimum version for macOS to 10.12 Sierra
git cherry-pick a6de4bf # fix: fix aarch64 and arm "'ptrdiff_t' was not declared in this scope" error
git commit --amend -m 'chore(dll): fix aarch64 and arm "ptrdiff_t was not declared in this scope" error'
git cherry-pick a446748 # ci: add macOS 13 support
cat <<'EOF'
ci: add macOS 13 support

macOS 14 (Sonoma) is generally available and the `latest` macOS runner image
but currently we can not have macos-14 due to build error: error <cstddef> tried including <stddef.h> but didn't find libc++'s <stddef.h> header.

Refs: https://github.blog/changelog/2024-04-01-macos-14-sonoma-is-generally-available-and-the-latest-macos-runner-image/
Refs: https://github.com/actions/runner-images?tab=readme-ov-file#available-images
EOF

git cherry-pick a6e6b32 # docs: add RAR 5.0 archive format doc

Finally, force push to your branch: git push -f

After that, I'll merge this in :)

[muja]

Or if you don't want to bother, I created a sister PR with those exact changes: #53. I can merge that if you want (unfortunately I cannot edit your PR)

[ttys3]

@muja OK, I can do this.

The reason I didn't respond immediately is because I discovered an issue with CI. Since GitHub has removed macOS 11, we should support versions 13 and 14 instead.

I think we can temporarily remove macOS 11 in this submission to ensure the CI passes smoothly. We can address the support issues for macOS 13 and 14 in a subsequent PR.

[ttys3]

see https://github.blog/changelog/2024-05-20-actions-upcoming-changes-to-github-hosted-macos-runners/#macos-11-deprecation-and-removal

[ttys3]

done.

@muja I removed macOS 11 since it block the CI (commit https://github.com/muja/unrar.rs/pull/48/commits/f9aeaed0223961610f4938895dc26bfb9768b090)

[ttys3]

@muja just removed a patch, which is not required for macOS >= 12

and added macOS 14 for CI

according to https://github.com/actions/runner-images#available-images free macOS 14 runner is always arm64, no x86-64 machine.

[ttys3]

now that we only need this patch https://github.com/muja/unrar.rs/pull/48/commits/22c548029ebfa3c36d18f8cbf270e44804c88e77 for macOS 12 and macOS 13

I have confirmed that macOS 14 does not have issue on this.

[ttys3]

since macOS 11 support has been removed (by github action), we do not need the __need_ptrdiff_t patch workaround anymore (I have removed the commit and force pushed).

in another way, I found that the __need_ptrdiff_t hack patch will cause problem under macOS 14:

warning: unrar_sys@0.3.1: In file included from vendor/unrar/strlist.cpp:1:
warning: unrar_sys@0.3.1: In file included from vendor/unrar/rar.hpp:6:
warning: unrar_sys@0.3.1: In file included from vendor/unrar/os.hpp:15:
warning: unrar_sys@0.3.1: In file included from /Applications/Xcode_15.4.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/new:93:
warning: unrar_sys@0.3.1: In file included from /Applications/Xcode_15.4.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__type_traits/alignment_of.h:14:
warning: unrar_sys@0.3.1: /Applications/Xcode_15.4.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/cstddef:46:5: error: <cstddef> tried including <stddef.h> but didn't find libc++'s <stddef.h> header.           This usually means that your header search paths are not configured properly.           The header search paths should contain the C++ Standard Library headers before           any C Standard Library, and you are probably using compiler flags that make that           not be the case.
warning: unrar_sys@0.3.1: #   error <cstddef> tried including <stddef.h> but didn't find libc++'s <stddef.h> header. \
warning: unrar_sys@0.3.1:     ^
warning: unrar_sys@0.3.1: /Applications/Xcode_15.4.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/cstddef:59:9: error: no member named 'nullptr_t' in the global namespace
warning: unrar_sys@0.3.1: using ::nullptr_t;
warning: unrar_sys@0.3.1:       ~~^

[muja]

The reason OS X 10.8 was added in the first place was because of #29. @IohannRabeson needed that target. That was however almost 1.5 years ago, so maybe their requirement has since changed? @IohannRabeson maybe you can comment on whether it would be detrimental for you if we removed it.

Removing it seems like a sensible way forward, especially given that it is increasingly challenging to even find ways to test/support it. However there may be benefit to have at least a patch release with OS X 10.8 working, and immediately removing support for it afterwards. This way we could officially say that 0.5.4 is the last release that supports OS X 10.8

I wouldn't worry about the missing pipeline run, I can merge it even without it.

[ttys3]

@muja currenly, setting a lower MACOSX_DEPLOYMENT_TARGET is ok. but I do not have macOS 11 env. so I do not know if the code compiles under macOS 11

or, maybe, we can merge the PR now. if macOS 11 users has problem, fire another bug.

[IohannRabeson]

Hi, no it would not be detrimental if you remove the support of OSX 10.8.

Le lun. 26 août 2024 à 07:18, ttys3 @.***> a écrit :

@muja https://github.com/muja currenly, setting a lower MACOSX_DEPLOYMENT_TARGET is ok. but I do not have macOS 11 env. so I do not know if the code compiles under macOS 11

or, maybe, we can merge the PR now. if macOS 11 users has problem, fire another bug.

— Reply to this email directly, view it on GitHub https://github.com/muja/unrar.rs/pull/48#issuecomment-2309963652, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFHJDCJYBIVTDUTFVYZFTTZTMFJRAVCNFSM6AAAAABFFGHSS2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBZHE3DGNRVGI . You are receiving this because you were mentioned.Message ID: <muja/unrar. @.***>

[ttys3]

Hi, no it would not be detrimental if you remove the support of OSX 10.8. Le lun. 26 août 2024 à 07:18, ttys3 @.> a écrit : … @muja https://github.com/muja currenly, setting a lower MACOSX_DEPLOYMENT_TARGET is ok. but I do not have macOS 11 env. so I do not know if the code compiles under macOS 11 or, maybe, we can merge the PR now. if macOS 11 users has problem, fire another bug. — Reply to this email directly, view it on GitHub <#48 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFHJDCJYBIVTDUTFVYZFTTZTMFJRAVCNFSM6AAAAABFFGHSS2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBZHE3DGNRVGI . You are receiving this because you were mentioned.Message ID: <muja/unrar. @.>

since github action already removed macOS 11 support, I think we have nothing to do.

support a legacy OS will took extra time, I do not think I can do this. I wouldn't object though if someone else could take on that responsibility and set up additional CI processes for outdated OSes to support those.

[muja]

@ttys3 it's okay, nobody expected you to support the legacy OS. The idea was to take your patch from March/April which you added for OS X 10.8 and release one patch version with it, communicating broadly that that is the last version where OS X 10.8 is supported. However, there isn't a reason for that anymore. Thank you for your contribution