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

Add windows experimental release action #48705

Merged

Conversation

reasonablytall
Copy link
Contributor

Summary

Build "Adds a CI action to build and release for Windows"

Purpose of change

The Jenkins-based experimental build-server has been broken for the past two weeks or so (#48642) making it hard for windows/CDDA-Game-Launcher users to get a copy of the latest experimental. This PR creates a Github Action that (on every push to master) creates an experimental release, builds the game for windows x32+x64, and adds the artifacts to the release.

NOTE: For some reason the artifacts produced by this build process are half the size (37MB vs 65MB) of those produced by the jenkins build server (though they still work!). Maybe I'm missing translations or something? I'd appreciate comments on that :)

Describe the solution

This creates a Github Action with two jobs:

  1. Create a release
  2. Build CDDA for Windows and upload it to the new release(matrix)
    • windows-tiles-x32
    • windows-tiles-x64

I'm mostly cargo-culting the build from COMPILING.md#cross-compile-to-windows-from-linux.

The release is tagged with experimental-BUILDNO, and the artifacts and release title include the build number as well. That build number will start at 1 the first time it's run on this repo, but I could add an offset if we'd like to match the previous experimental build numbers.

Linux build jobs can easily be added in future PRs. They would look very similar to the Windows job, just with different build steps.

Happy to take any requests, nits, wording changes, etc -- big or small!

Describe alternatives you've considered

If @narc0tiq manages to get the build-server running again, that'd work fine. I hacked this PR together on a whim when I tried to play the latest experimental but couldn't so I wouldn't mind either way if these changes are accepted :) Do what's best for the project!

Testing

I've run the build process and tested that the game runs for both artifacts.

Here's what a created release looks like:
image

And here's what the action looks like:
image

@actual-nh
Copy link
Contributor

@Maleclypse: Could you trigger these workflows? Thanks!

@actual-nh actual-nh added (P2 - High) High priority (for ex. important bugfixes) <Bugfix> This is a fix for a bug (or closes open issue) Code: Build Issues regarding different builds and build environments OS: Windows Issues related to Windows operating system labels Apr 29, 2021
@actual-nh actual-nh requested review from BrettDong and narc0tiq April 29, 2021 22:47
@actual-nh
Copy link
Contributor

Hmm. The actions aren't showing up, probably because they're supposed to trigger on a push, not a PR. If testing using the PR itself is desirable, then temporarily adding PRs to what can activate it may be needed.

@reasonablytall
Copy link
Contributor Author

Hmm. The actions aren't showing up, probably because they're supposed to trigger on a push, not a PR. If testing using the PR itself is desirable, then temporarily adding PRs to what can activate it may be needed.

Yep they're triggered to only run on push to master. To test locally you can push this commit to the master on your fork, but I've already run this SHA on my fork: https://github.com/reasonablytall/Cataclysm-DDA/actions/runs/796389473

I'm happy to push to master on my fork whenever I make changes and drop a link to the result in a comment. I'm not sure whether it would be good to temporarily add PRs as a triggering event, as that would create releases on this repo.

@actual-nh actual-nh added the Organization General development organization issues label Apr 29, 2021
@BrettDong
Copy link
Member

Is it possible to set a "wait" period so for example when 10 pull requests are merged into master (effectively 10 pushes) sequentially in 20 minutes, there will be only one build triggered rather than 10 builds?

@reasonablytall
Copy link
Contributor Author

Is it possible to set a "wait" period so for example when 10 pull requests are merged into master (effectively 10 pushes) sequentially in 20 minutes, there will be only one build triggered rather than 10 builds?

I could add a concurrency field on the workflow, which would prevent more than one instance of the workflow running at once. It cancels pending workflows when a new one is triggered, so in a burst of 10 commits/pushes only the first and last commits would get builds.

@actual-nh
Copy link
Contributor

I could add a concurrency field on the workflow, which would prevent more than one instance of the workflow running at once. It cancels pending workflows when a new one is triggered, so in a burst of 10 commits/pushes only the first and last commits would get builds.

Would the first commit actually get a full build if using cancel-in-progress: true?

@reasonablytall
Copy link
Contributor Author

Would the first commit actually get a full build if using cancel-in-progress: true?

No, but with the way it's currently implemented this action wouldn't be good to cancel (it would leave behind an empty release) so I wouldn't set cancel-in-progress: true.

@BrettDong
Copy link
Member

BrettDong commented Apr 30, 2021

How about splitting it into two workflows where the first workflow is triggered by push event to master branch and then makes a tag, and the second workflow is triggered by a new tag and do the build? Because in the future we may also add workflows to build for macOS, Android, etc. So a tagging workflow determines the version number and triggers builds for all platforms, which may be in separate workflows

@jbytheway
Copy link
Contributor

Does it run the tests? Would we want it to? I think the Jenkins build bot does, but I'm not certain.

@BrettDong
Copy link
Member

Does it run the tests? Would we want it to? I think the Jenkins build bot does, but I'm not certain.

Jenkins build bot doesn't build and run tests nowadays (see narc0tiq/catalysm-mainline-build#8)

@actual-nh
Copy link
Contributor

How about splitting it into two workflows where the first workflow is triggered by push event to master branch and then makes a tag, and the second workflow is triggered by a new tag and do the build? Because in the future we may also add workflows to build for macOS, Android, etc. So a tagging workflow determines the version number and triggers builds for all platforms, which may be in separate workflows

The tagging one could have a built-in pause (1 minute? 5 minutes?) and concurrency with cancel-in-progress true, so as to cancel it if another push happened in the meantime.

@ZhilkinSerg
Copy link
Contributor

Is it possible to set a "wait" period so for example when 10 pull requests are merged into master (effectively 10 pushes) sequentially in 20 minutes, there will be only one build triggered rather than 10 builds?

Better yet we trigger build only once a day if there are new commits since last build

@BrettDong
Copy link
Member

Better yet we trigger build only once a day if there are new commits since last build

Then it'd be better to make the version string the date of build rather than a sequence number so people won't be confused with the old Jenkins build version scheme.

@kevingranade
Copy link
Member

I think we'll want to switch this to using timestamps at some point instead of a series, but let's just get this in and see how it goes for now.

@kevingranade kevingranade merged commit 07c58d9 into CleverRaven:master May 5, 2021
commit url: https://github.com/${{ github.repository }}/commit/${{ github.sha }}
EOL

make -j$((`nproc`+0)) CROSS="${PLATFORM}" TILES=1 SOUND=1 RELEASE=1 LOCALIZE=1 BACKTRACE=0 PCH=0 bindist
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lang folder is missing in the output binary compared to Jenkins builds. You probably need to add LANGUAGES=all here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah nice -- I have that change testing here. Once it looks good I'll make open a new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) Code: Build Issues regarding different builds and build environments Organization General development organization issues OS: Windows Issues related to Windows operating system (P2 - High) High priority (for ex. important bugfixes)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants