search

DavidoTek / ProtonUp-Qt

Install and manage GE-Proton, Luxtorpeda & more for Steam and Wine-GE & more for Lutris with this graphical user interface.
https://davidotek.github.io/protonup-qt
GNU General Public License v3.0
1.17k stars 39 forks source link

(v2.8.1 fix) Default tar extract mode to 'r:', remove successfully extracted tkg archives #267

Closed sonic2kk closed 12 months ago

sonic2kk commented 12 months ago

Fixes two issues I (introduced and) found while testing the v2.8.1 Flatpak. They are not specific to Flatpak, I just found them during testing flathub/net.davidotek.pupgui2#18.

extract_tar doesn't work for regular .tar files

This is due to a bug where the default mode is r and gets garbled into r:r. This produces the error "unknown compression type 'r'". This happens because if not mode.startswith('r:') then we set it to r:{mode}. When the mode is left to the default parameter, which is r, this gets garbled to r:r (because {mode} is r). Having r on its own is also a valid argument to tarfile.extractall, but changing the default parameter to r: is very "elegant" and doesn't require any refactoring.

The fix for this bug is to set the default parameter to r:. Setting r:* is also valid. When having the default as r:, the startswith check will pass and we won't garble the mode parameter.

This is a regression from #250, and is the main "urgent" regression as it breaks Proton-tkg.

Incorrect Archive Globbing in Temp Dir

If there were two, say, tar.zst files in the temp dir, then the wrong one may be extracted. Since we just "glob" for the extension and assume there will only be one. To fix this, we clean up archives after extraction with a remove_if_exists. I considered updating the extraction functions to do this automatically, or take a parameter to do this, but I figured that might not be very useful since this behaviour is specific to Proton-tkg and its flavours (since we don't glob elsewhere afaik).

I ran into this issue when testing different Tkg flavours, and noticed the incorrect archive was being extracted. Existing archives will be cleaned up when ProtonUp-Qt is closed, as the temp dir is cleared.

This regression is possibly from #238, I am not sure. Since this is more of an edge case and has a workaround, this one is a bit less "urgent" but I fixed it here anyway.

Not sure if this warrants another release, since Proton-tkg is broken on v2.8.1 AppImage/source. I have no preference either way :smile:

DavidoTek commented 12 months ago

Interesting. Not sure how I missed that. I think I mixed up GE-Proton and Proton-Tkg while testing, the same happened to me while writing the release notes :facepalm:

sonic2kk commented 12 months ago

Too many Protons if you ask me :sweat_smile: And not enough wine to manage them all :wine_glass: