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: [Actions] release workflow for both releases and nightlies #8371

Merged
merged 6 commits into from Dec 19, 2020

Conversation

TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Dec 12, 2020

Fixes #8330
Fixes #8309

Depends on OpenTTD/workflows#10

A nightly run looks something like this:
https://github.com/TrueBrain/OpenTTD/actions/runs/420435699

A release run something like this:
https://github.com/TrueBrain/OpenTTD/actions/runs/420414947
(I tagged a fictional version in my own fork; you can ignore that completely)


Targets currently not included

  • ARM version of Mac OS. This is basically done and ready to ship, but we will wait till after this PR.
  • ARM version of Windows. This requires a few more patches before it compiles successful. The rest of the workflow is prepared for this.
  • Emscripten. This requires review of a few PRs, but otherwise this workflow is prepared to accept that too.
  • AppImage. There is a PR to introduces most basics of AppImage to the repo; it needs integration with the release workflow in such way we minimize diskspace and still have a workable image.

Choices made

  • NSIS is only running for releases, not for nightlies.
  • .deb builds are only made for releases, not for nightlies.
  • Universal MacOS builds will be a PR further down the line, if it turns out we want them / our user-base wants them.
  • We might attach our binaries to GitHub Releases for releases, but not right now. This will be an additional PR.

@glx22
Copy link
Contributor

glx22 commented Dec 12, 2020

I'll look at OPTION_TOOLS_ONLY

@TrueBrain TrueBrain force-pushed the release-workflow branch 2 times, most recently from 628a813 to e126cd9 Compare December 13, 2020 21:51
@LordAro
Copy link
Member

LordAro commented Dec 13, 2020

My particular answers for To Consider:

  • Do we need -dbg.deb?
    Yes - matches having PDB on Windows, and good for consistency
  • Do we want Debian builds? (we now only do Ubuntu)
    Yes
  • Do we want NSIS runs for every nightly, or only for releases (like it is now).
    IMO no
  • Do we want .deb builds for every nightly, or only for releases (like it is now).
    IMO no
  • Do we want Mac OS universal builds?
    Yes
  • Should we attach the binaries to GitHub Releases too? And if so, including the checksums? And if so, all three (MD5, SHA-1, SHA-256), or only SHA-256?
    IMO no to all. No point mixing where people get downloads from.

@orudge
Copy link
Contributor

orudge commented Dec 13, 2020

I'll cover the macOS ARM builds and Universal builds, but will likely wait for this PR to be merged first.

@michicc
Copy link
Member

michicc commented Dec 13, 2020

Attaching binaries to GitHub for releases is probably useful as there will be people that look for them on the GitHub Release tab first. I don't think having nightlies here as well makes sense, will just clutter the list way too much.

@TrueBrain
Copy link
Member Author

Attaching binaries to GitHub for releases is probably useful as there will be people that look for them on the GitHub Release tab first. I don't think having nightlies here as well makes sense, will just clutter the list way too much.

I tempt to agree with you. And yes, I meant for releases only :D Oops .. should have been more clear :P For nightlies it is not even possible.
But for now I will not implement it; we can add it if we like it in a later PR :)

Look at me, scoping the PR :D W00p!

@TrueBrain TrueBrain force-pushed the release-workflow branch 4 times, most recently from fc8b750 to 7bafbb6 Compare December 14, 2020 00:15
@TrueBrain
Copy link
Member Author

Unsure if .github/changelog.sh is a good location for that script ... but I also didn't find a better one. Suggestions are welcome!

@TrueBrain TrueBrain marked this pull request as ready for review December 14, 2020 08:53
@TrueBrain
Copy link
Member Author

Fixed .deb dependencies, and postponed -dbg.deb (as it seems CPack doesn't support this currently for applications; only for "components"). So this is now ready for review!

@TrueBrain TrueBrain force-pushed the release-workflow branch 2 times, most recently from 7169bfe to a7977fd Compare December 14, 2020 09:45
@TrueBrain TrueBrain added candidate: yes This Pull Request is a candidate for being merged size: large This Pull Request is large in size; review will take a while size: small This Pull Request is small, and should be relative easy to process and removed size: large This Pull Request is large in size; review will take a while labels Dec 14, 2020
Copy link
Member

@LordAro LordAro left a comment

Choose a reason for hiding this comment

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

More questions than problems

Comment on lines +4 to +12
workflow_dispatch:
inputs:
ref:
description: 'Ref to build (for Pull Requests, use refs/pull/NNN/head)'
required: true
repository_dispatch:
# client_payload should be the same as the inputs for workflow_dispatch.
types:
- Build*
Copy link
Member

Choose a reason for hiding this comment

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

I don't quite understand what these are for, can you explain?
Relatedly, why are the nightlies not triggered with a cron, e.g. https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-syntax-for-github-actions#onschedule ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Like I don't know about schedule .. pfft :P See OpenTTD/workflows#10 :)

workflow_dispatch means we have a new button in the GitHub interface that allows us to make a binary release of anything, anytime. This makes rebuilding easier, but also building PRs possible.

repository_dispatch is similar, but for automated triggers. A while ago we decided to move all those workflows into https://github.com/OpenTTD/workflows . This has mostly to do with forks. If you fork this repo, these days by default schedules are disabled; but this was not always the case. Additionally, if you enable it once for what-ever reason, it remains on for 90 days. So that is pretty noisy. So we decided to move all the schedule stuff to https://github.com/OpenTTD/workflows, as it is very unlikely anyone would fork that.

Additionally, the schedule code is not straightforward. It does not support timezones, but also it cannot check if there was anything new. That complexity is now all in OpenTTD/workflows#10, as it really has nothing to do with OpenTTD.

In https://github.com/OpenTTD/workflows you will find more lovely glue-code we use for our infra :) It also triggers eints commits, publishes the docs, rebuilds the website, etc :)

Comment on lines +30 to +51
- name: Checkout (Release)
if: github.event_name == 'release'
uses: actions/checkout@v2
with:
# We generate a changelog; for this we need the full git log.
fetch-depth: 0

- name: Checkout (Manual)
if: github.event_name == 'workflow_dispatch'
uses: actions/checkout@v2
with:
ref: ${{ github.event.inputs.ref }}
# We generate a changelog; for this we need the full git log.
fetch-depth: 0

- name: Checkout (Trigger)
if: github.event_name == 'repository_dispatch'
uses: actions/checkout@v2
with:
ref: ${{ github.event.client_payload.ref }}
# We generate a changelog; for this we need the full git log.
fetch-depth: 0
Copy link
Member

Choose a reason for hiding this comment

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

Would it be possible to set a 'ref' variable to use rather than duplicating this 3 times?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, kind of. But it would require a bash rule with the if logic in there to deduce which ref to use. The downside of that is, it is really easy to fuck that up. This now is trivially correct, and is why I prefer it. It does require a step extra .. but as you can see in a step later where we figure out the branch-name .. that code is not pretty, and really easy to mess up :) (which I also did, as it turns out :P)

So if you don't mind, I prefer this way. If you do mind, I can rewrite it .. but no guarantees :D

(I hope GitHub comes with some updates soon that I can also remove the other bash blob, and integrate it into these blobs .. but the actions/checkout needs some changes for that :D).

.github/workflows/release.yml Outdated Show resolved Hide resolved
TrueBrain and others added 6 commits December 18, 2020 14:36
Co-authored-by: Owen Rudge <owen@owenrudge.net>
This in preparation for other architectures, like arm64.
This has several ways of being triggered:
- When creating a new release via the GitHub interface. Fully
  automated that will produce new binaries, upload them, and it
  will even update the website to tell about the new version.
- When triggered in an automated way from OpenTTD/workflows to
  start a nightly.
- Manually via the Release workflow, which accepts branches,
  Pull Requests and tags to build.

Rerunning a job is safe and should be without issues. Everything
retriggers and updates what-ever might have been broken. In fact,
except for dates, it should produce identical results.

Co-authored-by: Charles Pigott <charlespigott@googlemail.com>
Azure Pipelines has build our releases for two years now, but we
are finally switching to GitHub Actions. This to bring the full
workflow to a single place, making it easier for people to see
what is going on and how to influence the process.
Copy link
Member

@LordAro LordAro left a comment

Choose a reason for hiding this comment

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

About time we had working nightlies again

Copy link
Contributor

@glx22 glx22 left a comment

Choose a reason for hiding this comment

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

Ok for me too.

@TrueBrain TrueBrain merged commit de61413 into OpenTTD:master Dec 19, 2020
@TrueBrain TrueBrain deleted the release-workflow branch December 19, 2020 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
candidate: yes This Pull Request is a candidate for being merged size: small This Pull Request is small, and should be relative easy to process
Projects
None yet
5 participants