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

GitHub workflows #75

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

GitHub workflows #75

wants to merge 20 commits into from

Conversation

h0tw1r3
Copy link
Contributor

@h0tw1r3 h0tw1r3 commented May 15, 2023

I was using this on my personal fork for awhile now (syncing with svn).

Creates artifacts for each of the supported platforms.

It doesn't contain a workflow for releases, but that should be really easy.

@trzy
Copy link
Owner

trzy commented May 23, 2023

I need to get around to this soon. Sorry for the delay. I'm really unfamiliar with the GitHub infrastructure around this so I need to devote some time to testing it. Will try this weekend but if not, will be in a few weeks as a conference is coming up.

@h0tw1r3
Copy link
Contributor Author

h0tw1r3 commented May 23, 2023

Here's what the workflows looks like in action:
https://github.com/h0tw1r3/model3emu/actions/runs/5060971437

@trzy
Copy link
Owner

trzy commented Jun 22, 2023

Ok. I need to get on this. Maybe I'll try tomorrow. How does this differ from the other PR from December that is still outstanding? If I understand correctly, you have Linux, Windows, and MacOS, and it looks like you are generating artifacts, too. Is the best way to test to fork the repo privately and edit there?

DirtBagXon referenced this pull request in DirtBagXon/model3emu-code-sinden Nov 5, 2023
@h0tw1r3
Copy link
Contributor Author

h0tw1r3 commented Nov 12, 2023

@trzy most convenient way to test is to add a commit to this branch, and open a pull request to your personal fork. You should then see a PR tests workflow under Actions.

@h0tw1r3
Copy link
Contributor Author

h0tw1r3 commented Nov 13, 2023

Also added a workflow dispatch trigger. Merge into the primary branch of your personal repo to test the ability to trigger the workflow ad-hoc.

@trzy
Copy link
Owner

trzy commented Dec 18, 2023

Ok, I really need to get on this and test it out. Can you give me an overview here? If it produces build artifacts that are identical to what I produce with my build bot (including properly packaging the ZIP file with all the required subdirectories, README.txt, LICENSE.txt, etc.), then we could really use this to distribute binaries.

@h0tw1r3
Copy link
Contributor Author

h0tw1r3 commented Dec 19, 2023

@trzy Yes, that was my intention. The builds contain everything you put in your releases.

@WingofaGriffin
Copy link

@h0tw1r3 I also tried making actions to automate it ( as seen in the draft PR ), but ran into errors with it not being able to find SDL2.h as described here: #135

How were you able to get around these errors, and do you have a link to your fork so I can test the binaries out?

@h0tw1r3 h0tw1r3 force-pushed the github-workflows branch 2 times, most recently from 55f9f96 to 1e60f08 Compare March 1, 2024 20:15
@h0tw1r3
Copy link
Contributor Author

h0tw1r3 commented Mar 1, 2024

@h0tw1r3 I also tried making actions to automate it ( as seen in the draft PR ), but ran into errors with it not being able to find SDL2.h as described here: #135

How were you able to get around these errors, and do you have a link to your fork so I can test the binaries out?

Opened this almost 10 months ago, I don't recall how I got around any errors, or that there were errors.

https://github.com/h0tw1r3/model3emu/actions/

@WingofaGriffin
Copy link

Opened this almost 10 months ago, I don't recall how I got around any errors, or that there were errors.

https://github.com/h0tw1r3/model3emu/actions/

Thanks. Taking a look, it seems like the solution for MacOS was to manually download and direct the makefile to SDL2 files, which feels a bit hacky to me, as the version of SDL2 is hard coded into the action. I'm curious if we can direct it to the homebrew installation we can perform with actions to something like /opt/homebrew/include/SDL2 which I've tried but failed at.

Additionally, upon downloading the artifact, it looks like you're zipping the tarball somehow. This seems redundant.

I'm not running a Windows machine right now to validate that build, but this does look like a better more robust implementation than mine otherwise. Good work.

.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
Makefiles/Makefile.OSX Show resolved Hide resolved
@trzy
Copy link
Owner

trzy commented Mar 6, 2024

Sorry, haven't been following along. What is the current state of this? I know there are some macOS Makefile issues apparently wrt SDL2. I'd like to try to find time to tackle this in the next week and get it merged in.

@h0tw1r3
Copy link
Contributor Author

h0tw1r3 commented Mar 6, 2024

Sorry, haven't been following along. What is the current state of this? I know there are some macOS Makefile issues apparently wrt SDL2.

This PR fixes the issues building on OSX with SDL2 framework.

I'd like to try to find time to tackle this in the next week and get it merged in.

👍 I am going to make one more change, refactor the current workflow into a template, and create two workflows that use it, one for CI and another for releases.

Makefiles/Makefile.Win32 Outdated Show resolved Hide resolved
…rmodel from being rebuilt each time because the object directory timestamp changes during builds
@trzy
Copy link
Owner

trzy commented Apr 5, 2024

Ok fixed it. The problem is that directories cannot be ordinary dependencies, their timestamp is always updated as object files are written to them. So when you build again, the directory is seen to have changed, and everything gets rebuilt needlessly. The solution was to make $(OBJ_DIR) an "order-only dependency" by putting it to the right of a '|' separator.

trzy and others added 4 commits April 5, 2024 15:40
* make Frameworks order only dep
* use file name variable in target rules
* osx build with latest sdl2 frameworks like CI
* remove macos sdl lookup workflow steps
@h0tw1r3
Copy link
Contributor Author

h0tw1r3 commented Apr 7, 2024

@trzy Cherry-picked your changes onto the PR.
Cleaned up some of the Framework targets and updated the action workflows to support downloading frameworks with make.
Also added support for downloading the latest version of the frameworks (as the action workflows do) when the source is clean.
Added a release workflow that doesn't do much for now other than create artifacts for each tested platform from the master branch.

Copy link
Owner

@trzy trzy left a comment

Choose a reason for hiding this comment

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

Looks like everything is in place. I just need to test this out before approving for merge. Will try to give it a spin tonight. My understanding is that I have to make a private fork and test there?

.github/workflows/_artifact.yml Outdated Show resolved Hide resolved
Makefiles/Makefile.OSX Outdated Show resolved Hide resolved
@h0tw1r3 h0tw1r3 force-pushed the github-workflows branch 2 times, most recently from 118887d to 27d14dc Compare April 11, 2024 06:52
@h0tw1r3 h0tw1r3 requested a review from trzy April 11, 2024 06:56
@h0tw1r3
Copy link
Contributor Author

h0tw1r3 commented Apr 11, 2024

Looks like everything is in place. I just need to test this out before approving for merge. Will try to give it a spin tonight. My understanding is that I have to make a private fork and test there?

No need to test on private fork. Workflows are successfully running here now.

@h0tw1r3
Copy link
Contributor Author

h0tw1r3 commented Apr 11, 2024

  • Moved release package creation from the github action workflow to make. Easier to test locally. Doesn't support windows command prompt. Assumed that was acceptable as the build bot script didn't either. Use make pkg
  • Fixed a make parallel bug when using the release target

@h0tw1r3
Copy link
Contributor Author

h0tw1r3 commented Apr 11, 2024

Seems that parallel builds fail sometimes.
https://github.com/trzy/Supermodel/actions/runs/8645740818/job/23703542520#step:5:193

Disabled parallel make in workflow job for now.

@trzy
Copy link
Owner

trzy commented Apr 11, 2024

  • Moved release package creation from the github action workflow to make. Easier to test locally. Doesn't support windows command prompt. Assumed that was acceptable as the build bot script didn't either. Use make pkg

I don't understand why the Musashi dependency causes this problem. I've looked at the rules countless times and it seems like the dependencies are all set up correctly and make should be able to resolve them. My build bot also avoids parallel builds for this reason. I can think of a fix for this but let's not worry about it for now -- serial builds are fine.

Copy link
Owner

@trzy trzy left a comment

Choose a reason for hiding this comment

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

I think this is just about ready to go. Couple nit picks/questions:

  1. When this gets merged to master, the artifacts will be visible on the project page, right? If so, I'm wondering about the naming of the artifacts. When I go to "Actions" and view the builds now, they look like this:

image

Can we make these labels more consistent with the actual archive filenames (that is, including the git revision) and ensure everything is lowercased? Or am I misunderstanding how they will be displayed?

I do see in the .yml file that you are using the runner platform and arch, so if there is no built-in way to change the case, don't worry about. Maybe then change the 'supermodel-' bit to 'Supermodel-'?

  1. I noticed Supermodel builds now fail on MSYS on my local machine with some linker errors involving -lmingw32, etc. Yet these work in command prompt. Strange. No need to fix this, I'll eventually look into it myself, but I don't fully understand what's going on. I might have just screwed up my environment variables at some point or something.

@h0tw1r3
Copy link
Contributor Author

h0tw1r3 commented Apr 11, 2024

Added changelog generator to make pkg

@h0tw1r3
Copy link
Contributor Author

h0tw1r3 commented Apr 11, 2024

I think this is just about ready to go. Couple nit picks/questions:

  1. When this gets merged to master, the artifacts will be visible on the project page, right? If so, I'm wondering about the naming of the artifacts. When I go to "Actions" and view the builds now, they look like this:

Job artifacts are simply the results of a job.

To compare it to the current build-bot process, there are separate functions/steps that build, generate a changelog, stage the release, and then upload to supermodel3.com via sftp.
What's missing from these github workflows are the equivalent of the upload (eg. publish a release).

We need to discuss how releases are created. https://github.com/trzy/Supermodel/releases
It's fairly straight-forward to add a step to the workflow, but most projects use tags to trigger the creation of a new release. Right now there are no tags. Would it be possible to start using tags, ideally with semantic versioning?

In lieu of figuring that out, I'm trying to work out a way to publish releases that use the same 0.3a-git- format that's been historically used by the project.

Can we make these labels more consistent with the actual archive filenames (that is, including the git revision) and ensure everything is lowercased? Or am I misunderstanding how they will be displayed?

The artifact names can be changed, but they are not intended to be used for releases. If you download one, you'll see it's double archived. This is a limitation (intended feature) of the artifacts system, job artifacts are always wrapped in a zip file. I believe Gitlab does it the same way, in that artifacts are intended to be used by other jobs/steps in a standard way. Eg. the release step would extract the artifact, make any final changes (code signing, permissions, etc), bundle up with an installer, and then publish the release.

I do see in the .yml file that you are using the runner platform and arch, so if there is no built-in way to change the case, don't worry about. Maybe then change the 'supermodel-' bit to 'Supermodel-'?

It's simple enough to make the final release archive file name match the current conventions.

@trzy
Copy link
Owner

trzy commented Apr 11, 2024

Sorry for my slowness, this GitHub CI/release stuff is still very unfamiliar to me, so I'm not sure what is required of the release process.

Semantic versioning is a possibility and has obvious advantages (particularly in making it easy to compare versions chronologically).

OTOH, given the light touch administration of this project, providing builds off of git commits (and SVN commits before that) has proven to work really well for us.

Would it be possible to do something like a weekly build (provided there was a commit) trigger?

If not, I suppose we can add tags for semantic version numbers and perhaps make the package flow create a version number from both the tag and the date? E.g.: 0.3.1-20240411 ?

How does the release process work in GitHub? Is it just a matter of a contributor running a "git tag" and then the workflow triggers? I assume there is some way to restrict permissions for who can add tags.

@h0tw1r3
Copy link
Contributor Author

h0tw1r3 commented Apr 11, 2024

Would it be possible to do something like a weekly build (provided there was a commit) trigger?

For sure. A scheduled workflow trigger sounds like it would be perfect for automatic releases.

Basically whatever cadence you want (weekly, daily, etc), the workflow would:

  • check if there are new commits on the primary branch (master) since the last release
  • if true, build a new artifact
  • if successful, create a new release.

How does the release process work in GitHub? Is it just a matter of a contributor running a "git tag" and then the workflow triggers? I assume there is some way to restrict permissions for who can add tags.

Yes, It can also be triggered on a tag and tagging is restricted to contributors by default. You can further restrict it using protection rules.

Based on your questions / feedback, I envisioning a dual release mechanism.

  • Maintainer can tag "milestones" as they see fit, which generate a new release, eg. 0.4.0.
  • Interim auto-scheduled releases based on commits added to the primary branch. 0.4.0+<Number of commits since last tagged release>. This would be similar to a "nightly". Optionally these could be cleaned up automatically after a new tagged release is made.

If that seems interesting, I'll whip something up on my fork for you to review.

@trzy
Copy link
Owner

trzy commented Apr 14, 2024

I think auto-scheduled releases based on commits would be awesome. It could be like a nightly or weekly build. Would it be possible to do it once per period (day, week, whatever) and tag it with date rather than the number of commits?

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.

3 participants