Use Helper Methods for ctmod Extraction

[sonic2kk]

This PR is a little bit of a proof-of-concept for an idea I've had for a while, which is to try and simplify the extraction logic in ctmods.

No screenshots for this one, this is all code refactor :-)

Background

Right now, extraction in ctmods uses a lot of repeated code. Some of the extraction logic is pretty complex, like when handling .tar.zst files. Actually, that specific case has been bugging me for a while. Everytime I look at that logic it seems very convoluted to me, so I wanted to try improving it. That gradually developed into "what if we had re-usable methods for extracting ctmods" - And thus, my idea of adding a few helper extraction methods was born. I finally made good on my threats of trying to improve this!

The tl;dr here is that extraction logic for ctmods is now in separate helper util functions, which should make the code more readable by abstracting away the detail of how ctmods are extracted. The benefit of this is especially apparent for Proton-tkg, whch imo is now much more readable than the nasty convoluted mess I wrote before.

Improvements

We have three main extraction functions here: Extract zip, Extract tar, and Extract tar.zst. Each of these functions takes an archive path and an extraction path. The functions call another helper function to ensure the archive itself exists, as well as the base directory for the archive to be extracted to.

The extraction functions will try and catch most reasonable errors that will occur and print those out. This will hopefully help make ctmod extraction more robust should anything go horribly wrong, as well as help with potentially displaying some of these errors in future.

If extraction succeeds, we return True inside of the try block. Therefore we should never leave the try block if everything succeeds, and we can assume any code executed outside of it means extraction failed, so we return False. We can use this in ctmods to return False if extraction failed, similar to how we use __download.

Extraction Utility Functions

Zip Extraction

The zip extraction, aside from catching exceptions and doing some more checks, is not really any different or majorly improved on its own from the current behaviour we have.

Tar Extraction

The tar extractor can take an optional mode parameter since some tars need this to extract properly (i.e. xz).

Zst Extraction

The .tar.zst extractor is a huge simplification over what we previously had. This method is used for Pron-tkg and helps a lot with readability imo. Aside from the variables used to set the paths (which could be hardcoded in method calls, but in the interest of readability I used separate variables), the extraction logic in the ctmod is now just 7 lines! (4 for extraction, 3 for removing dotfiles). If you look at the diff for this ctmod, there was a lot of very convoluted context managers and file I/O stuff which I hated working with - Trying to work on this for #238 was painful, I felt I could've handled this extraction logic much better back in #178.

I also had a good excuse to use some neat tricks like the Walrus operator 😄

The end result of this should be more readable code for Proton-tkg's extraction, not just for .tar.zst files either but for how we handle the different archive types!

Refactoring

While I was poking around in some ctmods, I tried to rename some variables and remove some unnecessary ones, once again for readability. Some variables were renamed for consistency with other ctmods (see the vkd3d ctmods and d8vk).

Performance

Since before extraction we do checks to ensure various files and folders exists, there could be a performance implication here, but it is hopefully not too big of a deal. Extraction itself is probably the biggest bottleneck for large archives (like Proton-tkg Wine Master). Smaller archives like vkd3d still download super fast.

Work in Progress

The diff for this PR is already pretty big, so for now I only touched a handful of ctmods. And if this is going to be extended for more ctmods, it's only going to get bigger. I'm opening this PR as-is because I want some feedback on whether it's worth continuing, and if so which improvements could be made. The goal is for readability and reuse, so any feedback that will help with this is much appreciated!

If the feedback is positive, I can tackle more ctmods. I just haven't yet in the interest of getting an opinion on what is currently implemented, and the PR is up to give an idea of what kind of readability improvements I'm going for 🙂

---

It may be possible to further improve the idea of this PR by making some of the code even more modular where possible. For example, some of the ctmods have very repetitive download logic (see get_tool for vkd3dlutris, vkd3dproton, and d8vk). But that could go in a separate PR I think, as this PR is already a pretty significant refactor.

Thanks :-)

[DavidoTek]

The tl;dr here is that extraction logic for ctmods is now in separate helper util functions

Very good. That makes everything clean again. It wasn't obvious what's happening when looking at the code before.

[...] Extract zip, Extract tar, and Extract tar.zst. [...] The functions call another helper function to ensure the archive itself exists [...]

I feel like the code is getting much more organized lately, following various software engineering practices like the single responsability principle here.
That makes maintanance much easier, thanks! 😄

there could be a performance implication here, but it is hopefully not too big of a deal

I took a quick lock at the code, I don't think there will be much of an impact.

making some of the code even more modular where possible [...] But that could go in a separate PR [...]

Yeah, I guess there is still much that could be cleaned up in the future. But still very good progress

---

I will review/test the changes in the next days. Looks like another merge to me 😄

Thanks!

[sonic2kk]

I should note I haven't updated all the ctmods yet, I have a commit locally I think that I haven't pushed yet which updates another one.

I can update more ctmods in this PR or a separate one, in the interest of making review easier (I know there's already a lot here, I usually try to keep PRs small/focused). But the idea should be the same between ctmods :-)

[DavidoTek]

I can update more ctmods in this PR or a separate one, in the interest of making review easier.

I guess if the changes are very similar in all ctmods, we can do it in this PR.

[sonic2kk]

Just got a little busier in my personal life but wanted to say I'm going to try and make time for this in the next few days :-)

[DavidoTek]

No worries, there's no hurry. I guess, it's a hobby project, also.

[sonic2kk]

Pushed two commits, one for Luxtorpeda that I committed a while ago but didn't get a chance to push, and one just now for DXVK. The commit for DXVK includes a small change to extract_tar, where now we can just pass the mode like mode='xz', because having to specify r: didn't feel right to me. It is still possible to use r:xz, as the function will just append r: to mode if it isn't present.

[sonic2kk]

In the commit to update Boxtron, I created a new utility function to write the tool version. It isn't strictly necessary but since it is a repeated action I think it's slightly cleaner to use a util function :-)

[sonic2kk]

Continuing to work through refactoring the ctmods to use the new extraction. Something I have noticed with is perhaps a benefit of doing this is that there are several sections of repeated logic. Some tools such as Roberta and Luxtorpeda for example are very similar in their get_tool logic (though they fetch releases etc differently).

On the other hand, as a result of this change, it has become apparent to me that DXVK and DXVK Async are... identical now? It looks as though aside from the URLs that they are the same. The way releases are fetched are the same (packages are the same, both same tar format, both same extraction directories). It's likely for a separate PR but it may be possible to take a similar approach with DXVK that we do with the TkG Proton builds, where DXVK Async inherits from the DXVK ctmod.

Combining the ctmods like this would make it a little bit easier to bring DXVK and DXVK Async to Heroic, since we would only need to add the extraction directory checking (which we have from D8VK) to one ctmod.

D8VK may not be an outlier here as it is fetched from nightly.link as a zip file. It may be possible to shoehorn D8VK into a compatible ctmod but it may be best as a standalone ctmod, at least for now; DXVK and DXVK Async seem to be the same from my scanning. Though D8VK and DXVK Nightly may have some overlap. It may be possible to combine DXVK and DXVK Nightly, and from there it would probably be possible to have D8VK meshed in as well. That way DXVK Async, DXVK Nightly, and D8VK could all be subclasses of the DXVK ctmod.

Just something I wanted to mention :-)

---

Since my last comment, I have updated (and tested) the following ctmods:

• Northstar Proton (Steam)
• Roberta (Steam)
• DXVK Async (Lutris)

I think it is making this more readable already 😄 I'll try to get through as many of the ctmods as I can. The only one I will have some difficulty testing is SteamTinkerLaunch. Currently I don't have my laptop handy (which I use to test ProtonUp-Qt STL installs, I do the development work on my PC with a make install installation). STL could be left alone for now and perhaps revisited separately.

Apart from that I'll take a look at the remaining ctmods and hopefully wrap up soon 👍

[sonic2kk]

Okay, aside from SteamTinkerLaunch, that should be all the ctmods updated 🥳

There was a util method that never ended up getting used, extract_zst, so I removed that.

Aside from further testing and review, I noticed the checksum, er, "checks", were seemingly repeated across GE-Proton, Wine-GE, and Lutris-Wine. It may be possible to create a util method to remove the repetition here as well. Of course that broadens the scope of this PR slightly, so it could always wait and go in a separate PR :-)

[DavidoTek]

I did quite some testing, things seem to work fine.
It took some time but can be merged now :)