RE-UE4SS icon indicating copy to clipboard operation
RE-UE4SS copied to clipboard

chore: switch to self hosted runner

Open localcc opened this issue 2 months ago • 15 comments

localcc avatar Apr 06 '24 10:04 localcc

The only upside this has over the standard github runner is that the VM localcc is using is much more powerful, so the build should go a lot faster.

I had a look at the standard github runner limits and even when linux port is done there's no way we'd ever even get close to the limits.

Since the xmake merge, we have to manually rerun the release workflow 3-5 times just for it to get past the early EOF error. Using a self-hosted runner does not solve this issue. If this continues, it's going to be really annoying to constantly have to babysit the make release actions for 10-20 minutes just to push it past this stupid issue.

Possible fixes:

  • https://stackoverflow.com/questions/21277806/fatal-early-eof-fatal-index-pack-failed - Lyrth said they tried some of these solutions to no avail
  • https://github.com/actions/checkout/issues/1379#issuecomment-1839483302 - fix that seemed to work for someone here is to use a PAT and switch to http for the fetch, but that's pretty dodgy
  • Switch to a different CI pipeline such as azure or jenkins

Buckminsterfullerene02 avatar Apr 06 '24 12:04 Buckminsterfullerene02

The only upside this has over the standard github runner is that the VM localcc is using is much more powerful, so the build should go a lot faster.

I had a look at the standard github runner limits and even when linux port is done there's no way we'd ever even get close to the limits.

Since the xmake merge, we have to manually rerun the release workflow 3-5 times just for it to get past the early EOF error. Using a self-hosted runner does not solve this issue. If this continues, it's going to be really annoying to constantly have to babysit the make release actions for 10-20 minutes just to push it past this stupid issue.

Possible fixes:

* https://stackoverflow.com/questions/21277806/fatal-early-eof-fatal-index-pack-failed - Lyrth said they tried some of these solutions to no avail

* [Checkout fails on early EOF error. [fetch-pack: invalid index-pack output] actions/checkout#1379 (comment)](https://github.com/actions/checkout/issues/1379#issuecomment-1839483302) - fix that seemed to work for someone here is to use a PAT and switch to http for the fetch, but that's pretty dodgy

* Switch to a different CI pipeline such as azure or jenkins

You sure ? I checked all the failed actions for the windows-runner branch, and none of them failed because of the EOF error. There are two different errors: 1 (latest):

ambiguous argument 'HEAD': unknown revision or path not in the working tree.
The process 'C:\Windows\system32\icacls.exe' failed with exit code 1332

2 (previous runs):

Input 'submodules' not supported when falling back to download using the GitHub REST API. To create a local Git repository instead, add Git 2.18 or higher to the PATH.

UE4SS avatar Apr 06 '24 18:04 UE4SS

The only upside this has over the standard github runner is that the VM localcc is using is much more powerful, so the build should go a lot faster.

I had a look at the standard github runner limits and even when linux port is done there's no way we'd ever even get close to the limits.

Since the xmake merge, we have to manually rerun the release workflow 3-5 times just for it to get past the early EOF error. Using a self-hosted runner does not solve this issue. If this continues, it's going to be really annoying to constantly have to babysit the make release actions for 10-20 minutes just to push it past this stupid issue.

Possible fixes:

* https://stackoverflow.com/questions/21277806/fatal-early-eof-fatal-index-pack-failed - Lyrth said they tried some of these solutions to no avail
* [Checkout fails on early EOF error. [fetch-pack: invalid index-pack output] actions/checkout#1379 (comment)](https://github.com/actions/checkout/issues/1379#issuecomment-1839483302) - fix that seemed to work for someone here is to use a PAT and switch to http for the fetch, but that's pretty dodgy
* Switch to a different CI pipeline such as azure or jenkins

You sure ?

I checked all the failed actions for the windows-runner branch, and none of them failed because of the EOF error.

There are two different errors:

1 (latest):


ambiguous argument 'HEAD': unknown revision or path not in the working tree.

The process 'C:\Windows\system32\icacls.exe' failed with exit code 1332

2 (previous runs):


Input 'submodules' not supported when falling back to download using the GitHub REST API. To create a local Git repository instead, add Git 2.18 or higher to the PATH.

That was an issue in my runner setup, not the problem we're having with GitHub hosted.

localcc avatar Apr 06 '24 18:04 localcc

That was an issue in my runner setup, not the problem we're having with GitHub hosted.

Okay but I'm not seeing any EOF errors here on github from this branch which I assume is self hosted. Have you been seeing this error locally ? I have never seen this error locally myself, which makes me think it's a problem with githubs set up which if true means that we should be able to avoid it if we're self-hosting.

UE4SS avatar Apr 06 '24 19:04 UE4SS

EOF errors occur regardless if it's a self hosted or a GitHub hosted runner

localcc avatar Apr 06 '24 21:04 localcc

EOF errors occur regardless if it's a self hosted or a GitHub hosted runner

But how do you know that this is the case if there's been no such error from your runner ? Is this some sort of known global github error and the runner is locked down in some way even if it's self hosted so you can't fix it ? Otherwise I don't understand why I literally never have this error locally. If this error doesn't happen locally then you'd think if we set the self-hosted runner up the same way we'd also not have the error there.

UE4SS avatar Apr 06 '24 21:04 UE4SS

EOF errors occur regardless if it's a self hosted or a GitHub hosted runner

But how do you know that this is the case if there's been no such error from your runner ? Is this some sort of known global github error and the runner is locked down in some way even if it's self hosted so you can't fix it ? Otherwise I don't understand why I literally never have this error locally. If this error doesn't happen locally then you'd think if we set the self-hosted runner up the same way we'd also not have the error there.

This bug has been reported a few times in the GitHub actions repo https://github.com/actions/checkout/issues/1379

Anyway, can we please just try the org approved PAT and switch to https solution?

Buckminsterfullerene02 avatar Apr 07 '24 03:04 Buckminsterfullerene02

EOF errors occur regardless if it's a self hosted or a GitHub hosted runner

But how do you know that this is the case if there's been no such error from your runner ? Is this some sort of known global github error and the runner is locked down in some way even if it's self hosted so you can't fix it ? Otherwise I don't understand why I literally never have this error locally. If this error doesn't happen locally then you'd think if we set the self-hosted runner up the same way we'd also not have the error there.

This bug has been reported a few times in the GitHub actions repo actions/checkout#1379

I guess I just don't know how the self-hosted runner works. I assumed if you host it yourself, you have full control, like it would just ssh in and execute whatever git commands but evidently it's not the case. My bad.

UE4SS avatar Apr 07 '24 03:04 UE4SS

Anyway, can we please just try the org approved PAT and switch to https solution?

I think this is a good idea, but if it doesn't work then I think we should definitely move away from github actions because CI is too important to be failing like this.

UE4SS avatar Apr 07 '24 04:04 UE4SS

Lyrth tested the PAT idea on their fork and there are zero errors across 5 runs, so it seems to be the fix of the problem. The only thing to note is that the owner of the UEPseudo repo (https://github.com/Re-UE4SS) needs to be the one to make the PAT as needs access to the repo in the absence of the SSH key. So Narknon needs to do that.

I think this PR should be closed as we don't have any need for a self hosted runner anymore, as it provides no extra benefit.

Buckminsterfullerene02 avatar Apr 10 '24 11:04 Buckminsterfullerene02

Lyrth tested the PAT idea on their fork and there are zero errors across 5 runs, so it seems to be the fix of the problem. The only thing to note is that the owner of the UEPseudo repo (https://github.com/Re-UE4SS) needs to be the one to make the PAT as needs access to the repo in the absence of the SSH key. So Narknon needs to do that.

I think this PR should be closed as we don't have any need for a self hosted runner anymore, as it provides no extra benefit.

We should maybe not have only one person with that kind of access to such an important repo, just in case, and didn't you say he was somewhat retired ? So he might be drifting in and out of this place kind of like me, not ideal for the only person with that kind access.

UE4SS avatar Apr 10 '24 11:04 UE4SS

Accidentally keyboard shortcutted this PR closed.

UE4SS avatar Apr 10 '24 11:04 UE4SS

Lyrth tested the PAT idea on their fork and there are zero errors across 5 runs, so it seems to be the fix of the problem. The only thing to note is that the owner of the UEPseudo repo (https://github.com/Re-UE4SS) needs to be the one to make the PAT as needs access to the repo in the absence of the SSH key. So Narknon needs to do that. I think this PR should be closed as we don't have any need for a self hosted runner anymore, as it provides no extra benefit.

We should maybe not have only one person with that kind of access to such an important repo, just in case, and didn't you say he was somewhat retired ? So he might be drifting in and out of this place kind of like me, not ideal for the only person with that kind access.

We've tried all the other solutions found in various places and this is the only solution that actually works consistantly. So unless you'd prefer to stick with manually coaxing the shit through the pipe for every merge (which typically takes between 10 and 20 minutes) or try to find a better solution, I don't see how we have any choice, despite this choice being pretty shitty (thanks github).

We can use classic PATs which can be set to never expire so theoretically once Narknon should never have to mess with it again. But if he does, it's not like he's so far retired (and I doubt he ever will be) that he won't do it. He's active enough that he's been reading all the PRs ~~and complains about how we're ruining ue4ss, I have no clue what that's on about.~~

Security wise, the PAT has access to all private repos under the account, so will just have to treat it like an SSH key, i.e. only org admins can view it and have to treat it like a password. Meaning localcc, truman, narknon and you.

image

Buckminsterfullerene02 avatar Apr 10 '24 11:04 Buckminsterfullerene02

Lyrth tested the PAT idea on their fork and there are zero errors across 5 runs, so it seems to be the fix of the problem. The only thing to note is that the owner of the UEPseudo repo (https://github.com/Re-UE4SS) needs to be the one to make the PAT as needs access to the repo in the absence of the SSH key. So Narknon needs to do that. I think this PR should be closed as we don't have any need for a self hosted runner anymore, as it provides no extra benefit.

We should maybe not have only one person with that kind of access to such an important repo, just in case, and didn't you say he was somewhat retired ? So he might be drifting in and out of this place kind of like me, not ideal for the only person with that kind access.

We've tried all the other solutions found in various places and this is the only solution that actually works consistantly. So unless you'd prefer to stick with manually coaxing the shit through the pipe for every merge (which typically takes between 10 and 20 minutes) or try to find a better solution, I don't see how we have any choice, despite this choice being pretty shitty (thanks github).

We can use classic PATs which can be set to never expire so theoretically once Narknon should never have to mess with it again. But if he does, it's not like he's so far retired (and I doubt he ever will be) that he won't do it. He's active enough that he's been reading all the PRs ~and complains about how we're ruining ue4ss, I have no clue what that's on about.~

Security wise, the PAT has access to all private repos under the account, so will just have to treat it like an SSH key, i.e. only org admins can view it and have to treat it like a password. Meaning localcc, truman, narknon and you.

image

I don't have a problem with your solution, I just didn't like the idea of only person having admin access to the UEPseudo repo as you implied when you said narknon has to do it. Maybe I misunderstood something.

UE4SS avatar Apr 10 '24 11:04 UE4SS

Lyrth tested the PAT idea on their fork and there are zero errors across 5 runs, so it seems to be the fix of the problem. The only thing to note is that the owner of the UEPseudo repo (https://github.com/Re-UE4SS) needs to be the one to make the PAT as needs access to the repo in the absence of the SSH key. So Narknon needs to do that. I think this PR should be closed as we don't have any need for a self hosted runner anymore, as it provides no extra benefit.

We should maybe not have only one person with that kind of access to such an important repo, just in case, and didn't you say he was somewhat retired ? So he might be drifting in and out of this place kind of like me, not ideal for the only person with that kind access.

We've tried all the other solutions found in various places and this is the only solution that actually works consistantly. So unless you'd prefer to stick with manually coaxing the shit through the pipe for every merge (which typically takes between 10 and 20 minutes) or try to find a better solution, I don't see how we have any choice, despite this choice being pretty shitty (thanks github). We can use classic PATs which can be set to never expire so theoretically once Narknon should never have to mess with it again. But if he does, it's not like he's so far retired (and I doubt he ever will be) that he won't do it. He's active enough that he's been reading all the PRs ~and complains about how we're ruining ue4ss, I have no clue what that's on about.~ Security wise, the PAT has access to all private repos under the account, so will just have to treat it like an SSH key, i.e. only org admins can view it and have to treat it like a password. Meaning localcc, truman, narknon and you. image

I don't have a problem with your solution, I just didn't like the idea of only person having admin access to the UEPseudo repo as you implied when you said narknon has to do it. Maybe I misunderstood something.

Ah I misunderstood you. Perhaps it would be a good idea for Narknon to give at least one other person the credentials for that account.

Buckminsterfullerene02 avatar Apr 10 '24 12:04 Buckminsterfullerene02