-
-
Notifications
You must be signed in to change notification settings - Fork 449
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
[FIX-macOS] Wine Downloader fails to extract files #3227
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, this seems to work fine (tested on Linux)
plugins: [decompressTargz(), decompressTarxz()], | ||
strip | ||
}) | ||
if (!isSnap && (path.endsWith('.tar.xz') || path.endsWith('.tar.gz'))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if the second condition makes sense here. decompress
will also only work with these two file types (since we manually pass the two plugins for them to it)
So something like
if (!path.endsWith('.tar.xz') && !path.endsWith('.tar.gz')) {
logError(['Unsupported archive:', path], LogPrefix.Backend)
return
}
if (!isSnap)
// ...
might be more accurate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it fails if we pass a different archive. I just think this way is safer, but might be too much or not needed like you said.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is safer right now, actually. I doubt those plugins are designed to work with archives not intended for them (and them failing might lead to a crash instead of just an error message, as seen here), so bailing out early should be the safer option
Regardless, as far as I know we currently don't feed other archive formats to this function, so for now this doesn't really matter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested on an intel mac and it works fine (it worked fine before too though, the issue was only for M1/2/3 chips)
Co-authored-by: Mathis Dröge <[email protected]>
…er/HeroicGamesLauncher into fix/wine-extract-mac
This fixes #3202
But it also get back to using
tar
on Linux except for Snaps. so if you do not have a mac to test, just test the behavior on linux, it is the same.Use the following Checklist if you have changed something on the Backend or Frontend: