Skip to content
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

QVM: Fix bitrot and add CI. #350

Merged
merged 8 commits into from
Jul 13, 2024
Merged

QVM: Fix bitrot and add CI. #350

merged 8 commits into from
Jul 13, 2024

Conversation

dsvensson
Copy link
Collaborator

No description provided.

@dsvensson
Copy link
Collaborator Author

dsvensson commented Jun 10, 2024

Not sure how you want to do the CI. Would be nice if ktx also produced GH downloads for all builds, PRs and all, also in fork repos. It's free and convenient. Example of this running can be found here: https://flagzone.nittionio.nu/ (dev-tools -> application -> clear site data if it b0rks due to stale cache). Ext functions can be verified by dropping a map in the browser with alpha for example, this will launch that map with ktx active.

@ciscon
Copy link
Collaborator

ciscon commented Jun 10, 2024

if you're logged in you can download the artifacts from the action.

@dsvensson
Copy link
Collaborator Author

dsvensson commented Jun 10, 2024

yeah, but I'm thinking of this if: github.repository == 'QW-Group/ktx' in build-targets.yml and build-targets-snapshot.yml. It only makes sense for the release workflow.

@ciscon
Copy link
Collaborator

ciscon commented Jun 10, 2024

the snapshots get pushed, but yeah definitely the targets one should be everything, on ezquake it's: on: [push, pull_request, workflow_dispatch]

@dsvensson
Copy link
Collaborator Author

dsvensson commented Jun 10, 2024

Example of extension syscall use, trap_SetExtField that sets alpha value for a brush entity.
Skärmavbild 2024-06-10 kl  20 12 02

Open https://flagzone.nittionio.nu, shift+ESC for console, /rctf3_b2 to preload map into browser, ESC to menu, then new multiplayer game and /rctf3_b2 to load that map with ktx qvm ... doesn't properly load the server config atm so /exec ftesrv.cfg and /rctf3_b2 again to fully load ktx, walk off an edge a few times to get a good spawn as I don't seem to have ctf+hook enabled there and then you should be able to walk to place of screenshots.

@ciscon
Copy link
Collaborator

ciscon commented Jun 10, 2024

the other problem is just that we don't have an upload artifact in the target one (or the others, though they have a different purpose anyway). i was going to combine all of them into one and just have different logic paths as right now we're doing the same work multiple times but have yet to get to it.

src/g_syscalls_extra.c Outdated Show resolved Hide resolved
@dsvensson
Copy link
Collaborator Author

If you don't have time to integrate the CI changes properly in the other builds feel free to suggest how you want it done and I'll fix it. Would be great if the bitrot could be stopped once and for all before more comes creeping in.

@ciscon
Copy link
Collaborator

ciscon commented Jun 19, 2024

the goal would be to have a single workflow that always runs the logic in the current build-target.yml (but then uploads those as artifacts) and then just like the other separate steps currently have push those as snapshots if it's a push on qw-group/ktx and do a release with them if it's a release. i was going to reorganize it so there was one main file with the logic and three separate ones to be included with the actual steps, but all in one file would also be just fine as once it's deduplicated it really won't be much longer than any one of those files currently (and we'll only be building the once). testing it all by using the fork's information for the release part (and even the snapshot stuff, though it'll end up failing of course) should be simple enough, there'll just have to be storing the assets for use in the other stages and renaming etc to make it all come out the way it currently does, but definitely doable.

@dsvensson
Copy link
Collaborator Author

dsvensson commented Jul 5, 2024

https://builds.quakeworld.nu/ktx is not even up, why not just go with github artifacts? I have a decent combination of the workflows that I could push here, and perhaps you could transition to just going with github releases to avoid the mess of having to scp stuff elsewhere? With that in place you could just go to the tags view in GitHub, and press "Draft new release" and it would generate release notes for you, and you push release and a tag will be created, and a build could be triggered that attaches the build artifacts to the release.

@ciscon
Copy link
Collaborator

ciscon commented Jul 5, 2024

that's exactly what i'm saying, the artifacts are only created for one of the pipelines right now and not reused- they should be created regardless and then reused depending on X for either pushing snapshots and/or release instead of rebuilding things. we do want a place for the snapshot stuff though and not just do drafts/releases for every single build, that doesn't mean there shouldn't be artifacts attached to the build though.

@dsvensson
Copy link
Collaborator Author

dsvensson commented Jul 5, 2024

I guess the scp stuff could remain. I'll hack up something that works with the github release flow, and that could also copy the release artifacts elsewhere. Realized I can tag to my heart's content in my fork and try things out :) The use of the word "draft" is not "draft release" but just the github phrasing of generating release notes that the release person can adapt, but you get a nice list of changes in the editor for free.

@ciscon
Copy link
Collaborator

ciscon commented Jul 5, 2024

it's actually pretty simple, we've just got it all divided up because it was done multiple times- always do the build creating artifacts, then only do the snapshot push of those artifacts (fiddled with for naming etc) if it's the right repo/branch, and only do the push of the artifacts to release on release/published.

that's fine, that'd just mean you'd always do "that" part regardless of published i suppose.

@dsvensson
Copy link
Collaborator Author

dsvensson commented Jul 6, 2024

Almost done now, releases end up at your equivalence to:

https://github.com/qw-ctf/ktx/releases

...like other GitHub projects. On that page you see a 'Draft a new release' (or if you're repo owner at least) that gives you this:
image

After entering a new release you generate release notes that will auto-populate based on pull requests merged since last release.

This will trigger the build including the upload job that will attach release artifacts to the release, as well as upload to builds.quakeworld.nu.

Here's the current incarnation:
https://github.com/qw-ctf/ktx/blob/qvm/.github/workflows/main.yml

Think I just need to place the final if: guards, and perhaps trim some noisy output... and invert the sftp branches lol..

Sample run:
https://github.com/qw-ctf/ktx/actions/runs/9820691450

@ciscon
Copy link
Collaborator

ciscon commented Jul 6, 2024

i haven't looked at it yet but does it push a snapshot when it's merged into master on qw-group or after a release is tagged?

@dsvensson
Copy link
Collaborator Author

dsvensson commented Jul 6, 2024

When the if: guards are finalized it will push snapshots on qw-group/ktx master pushes, and releases on release events.

@vikpe vikpe removed their request for review July 9, 2024 15:54
@dsvensson
Copy link
Collaborator Author

@qqshka @tcsabina Added build timeout per feedback from @vikpe, and @ciscon was the one suggesting the CI changes, so do you have any further comments?

@ciscon
Copy link
Collaborator

ciscon commented Jul 10, 2024

@qqshka @tcsabina Added build timeout per feedback from @vikpe, and @ciscon was the one suggesting the CI changes, so do you have any further comments?

i do not, anybody else?

@tcsabina
Copy link
Collaborator

@qqshka @tcsabina Added build timeout per feedback from @vikpe, and @ciscon was the one suggesting the CI changes, so do you have any further comments?

Hi Nano,

let me get back to you within a few days about this CI/CD change...

@dsvensson
Copy link
Collaborator Author

dsvensson commented Jul 12, 2024

Verified that https://builds.quakeworld.nu/ktx/snapshots/ is populated correctly by temporarily allowing uploads of branch test.

Don't think there's any need to test release path specifically as it uses the same code.

Here's what it looks like when building in my branch where it just echoes the result:
https://github.com/qw-ctf/ktx/actions/runs/9820556161/job/27115280956

...which also attached the artifacts to the GH release:
https://github.com/qw-ctf/ktx/releases

@ciscon
Copy link
Collaborator

ciscon commented Jul 12, 2024

looks good to me

@dsvensson dsvensson merged commit 6861422 into QW-Group:master Jul 13, 2024
8 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.

None yet

3 participants