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.25k stars 40 forks source link

Proton-tkg (Wine Master) missing CI builds (too many recent failed builds) #236

Closed sonic2kk closed 1 year ago

sonic2kk commented 1 year ago

Describe the bug
Proton-tkg for some reason seems to be listing builds from the Proton-tkg releases instead of the Wine Master CI.

I believe it's listing the wrong versions as for "Proton-tkg (Wine Master)", __fetch_workflows is returning [] (still testing but I would guess it isn't finding a valid workflow and so is just returning []).

I have a suspicion part of the problem is that a significant amount (though still < 100) of the Wine Master CI builds have failed. Since there is over a full page of failures, I wonder if this is perhaps throwing ProtonUp-Qt off. It can account for failed builds but maybe there's something with the pagination where if a full page of builds fail, it falls over? Not sure yet, I am still investigating :-)

Regular Proton-tkg is unaffected, and Wine-tkg builds for Lutris are unaffected.

This also affects Wine Master builds for #233.

To Reproduce
Steps to reproduce the behavior:

  1. Enable Advanced Mode
  2. Select Steam as the source (or Heroic Proton if using #233)
  3. Click "Add Version" on the main menu
  4. Select "Proton-tkg (Wine Master)"
  5. Listed build versions are incorrect and seem to line up more closely with those on the Proton-tkg Releases page.

Expected behavior
Proton-tkg (Wine Master) should use CI builds, which it did as of about two weeks ago. I used ProtonUp-Qt to download a build based on Wine Master to run Vortex 1.8.0 on Linux (requires Wine Staging 8.4+).

Notably, the build I downloaded coincidentally turned out to be the last passing Wine Master build. Every other build since then has failed.

Screenshots
Note in this screenshot the version of Proton-tkg 7.6 and the version linked to on the Releases page. They appear to line up.

image

Desktop (please complete the following information):

Additional context
This "bug" was in all likelihood not caused by a ProtonUp-Qt code change, but something that changed upstream which broke compatibility.

Terminal output
N/A

sonic2kk commented 1 year ago

Narrowed down the issue to this line in __fetch_workflows:

tags.extend(str(run['id']) for run in self.rs.get(workflow["url"] + "/runs").json()["workflow_runs"] if run['conclusion'] == "success")

I would guess whatever is returned from that API call is only returning the last X workflows, and since they all fail, tags is [].

sonic2kk commented 1 year ago

That endpoint only returns the last 30 by default, but we can pass it a per_page. Changing the line to something like this will work:

tags.extend(str(run['id']) for run in self.rs.get(workflow["url"] + f"/runs?per_page={str(count)}").json()["workflow_runs"] if run['conclusion'] == "success")

The "issue" with this is that it does slightly increase the load time for Proton-tkg. The "solution" for that though would be to restrict the count based on the parameter given to __fetch_workflows.

sonic2kk commented 1 year ago

Having got a slightly better understanding of the problem here, any program that does anything similar to ProtonUp-Qt for CI builds (i.e. fetching the last X passed builds) would run into this problem eventually if there were enough failed builds for a workflow. Even if they were to get the last 1,000,000 builds (silly example), if the last 1,000,000 builds failed then it would have this issue.

So I don't think this is a ProtonUp-Qt bug, it might've fit more in the "Enhancement" category (something like "Attempt to get more CI builds").