Skip to content

GE-Proton: Refactor to use fetch_project_release_data #524

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

Merged

Conversation

sonic2kk
Copy link
Contributor

@sonic2kk sonic2kk commented Apr 9, 2025

Overview

This PR implements a long-standing TODO item of mine, which is to implement fetching checksum data on fetch_project_release_data. This allows ctmods that need can validate checksums (currently only GE-Proton and Proton-CachyOS) to migrate to using fetch_project_release_data without losing the checksum functionality.

The main benefit here is that we can continue to bring, and to reduce duplication in child classes.

As an aside, I did test with Proton-CachyOS and it can be refactored to use this method as well (we can use checksum_suffix=f'{arch}.sha512sum' so that it gets the checksum for the correct release asset even), and it is compatible with the changes in #523. So this additional functionality in fetch_project_release_data` should be flexible enough for general use, I didn't want to implement something that just worked based on how GE-Proton might use it but how any ctmod might use it.

Implementation

util.py

A few small changes were needed in fetch_project_release_data .

We need to check all release assets now, so we can't break sadly, since we don't know if the asset we want will be before or after the main asset. Otherwise, the logic to fetch the asset we want is the same.

To fetch the checksum, we take a new optional parameter to fetch_project_release_data called checksum_suffix. Note that checksum_suffix may not simply be a checksum type, but could include extra information to differentiate it from other checksum assets in the same release. This allows us to specify which checksum we want, as a release may have multiple checksums in its release assets. Proton-CachyOS is a good example, since it has multiple checksums in a release that are both .sha512sum.

There is an argument to be made that this may not be sufficient for more complex release asset names, and a regex may be more appropriate, but the same argument could be made for the release_format parameter where it may not be sufficient to differentiate releases. I think broadly this should be fine and generally, checksums by convention are named with the same name as the file they are hashed from but with a different suffix (i.e. archive.tar.gz would have a checksum of archive.sha512sum). So unless there comes a point where release_format is not sufficient, then we should be fine with checksum_suffix as a generic string.

If checksum_suffix is passed to fetch_project_release_data, and if our values dictionary does not already have a checksum found inside of it (i.e. we have not found our desired checksum asset yet), then we want to find of the current asset has a checksum URL on it. To do this, we can re-use get_download_url_from_asset with all of the same arguments that we give to it above to get the archive asset URL (i.e. the tarfile asset for GE-Proton) except that we want to find the download URL for the asset with the release_format of checksum_suffix. If this gives us back a non-empty string we can assume that we found a release asset with the checksum_suffix that we expect, so we can assume that that is a checksum asset and we can insert its URL into our values dictionary under values['checksum'].

Note that if checksum_suffix is not generic enough, then the first found instance of an asset matching this string will be included. This is how it would work before though when used inside of __fetch_github_data, so it's just a matter of usage. In later iterations of the loop, even if another asset matching our checksum asset is found, it will not be overwritten, since we don't go through the logic of calling get_download_url_from_asset if our values dict already has a checksum key (i.e. if we already found a checksum earlier, don't try to find one again).

With all of that, fetch_project_release_data can return a checksum in its values if we specify a checksum_suffix to look for, which allows Ctmods like GE-Proton to use this method and still get back a checksum.

GE-Proton Ctmod

The only change needed inside of GE-Proton is to replace all of our logic inside of __fetch_github_data with a call to fetch_project_release_data. The only missing functionality in that util function before preventing it from being used in the GE-Proton ctmod was that it could not return a checksum.

We pass fetch_project_release_data a checksum_suffix of .sha512sum. This is very broad, but this matches what we were passing to GE-Proton before, so I kept the same checksum name check here too.

That is the only change needed to the GE-Proton ctmod to get it over to using the fetch_project_release_data util function, and now as a result, GE-Proton is fully refactored to using our util functions and doesn't have any network API calls to manage itself!

Concerns

I am making an assumption here that checksum assets on GitLab will also use the url key the same way they do for all other release assets. We do not currently have any Ctmods using GitLab that verify a checksum, and I do not know any GitLab projects off-hand to check that have checksums in their assets to see what they return. This assumption may turn out to be wrong, and we may need to refactor in future if that is the case. But I think it is reasonably sane to assume that all release assets on GitLab will use the same key on the API, regardless of whether they're release tarballs or checksum files or something else :-)

Also, this PR does modify fetch_project_release_data which used by other ctmods. More testing may be warranted to ensure nothing has broken :-) In my tests things seem fine, but it's late and I did not exhaustively test yet.

Future Work

Since the logic in this PR to fetching checksums in this PR is not limited to sha512, in future we may want to make the way the GE-Proton ctmod verifies hashes a bit more generic. We may want to extract the logic that does the checks out of get_tool and into a single method that can be overridden by child classes, if ctmods that subclass GE-Proton were to use other checksums.

If necessary, some of the functionality here could be broken out into a separate util file (something like hashutils.py), which might allow other ctmods with checksums to more easily integrate without having to copy around boilerplate code.


Hope that explains the rationale for the changes made here well enough. More Ctmod refactoring! One day I'll realise my goal of a base Ctmod class 😉

Thanks!

@sonic2kk sonic2kk force-pushed the geproton-use-fetch-project-release--data branch from a651cba to 4c4b7f1 Compare May 7, 2025 17:12
@sonic2kk
Copy link
Contributor Author

sonic2kk commented May 7, 2025

Rebased onto main, verified that it still downloads and extracts GE-Proton successfully.

@DavidoTek DavidoTek merged commit 00d2bd8 into DavidoTek:main May 23, 2025
2 checks passed
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