fornjot icon indicating copy to clipboard operation
fornjot copied to clipboard

All binaries built by CD workflow are labeled as release binaries

Open hannobraun opened this issue 3 years ago • 1 comments

As of #868, we now have a --version argument that displays the version, and makes a distinction between release and development versions. Unfortunately all binaries built by the CD workflow are labeled as release binaries for the purpose of the --version flag. This leads those binaries that aren't promoted to release binaries to display a wrong version.

The way it works, is that on every push to main, the CD workflow is started, and builds binaries: https://github.com/hannobraun/Fornjot/blob/016d5d2b2ff2dd33888c1b25d38d47c1f2c4749d/.github/workflows/cd.yml#L23-L77

The flag that marks the binaries as official release binaries is wrongly set for that whole workflow: https://github.com/hannobraun/Fornjot/blob/016d5d2b2ff2dd33888c1b25d38d47c1f2c4749d/.github/workflows/cd.yml#L14-L16

After all binaries are built, the release job starts, and will publish a release (using those binaries), if that's what is supposed to happen. Whether it's supposed to happen, is determined by the Operator | Deduce step: https://github.com/hannobraun/Fornjot/blob/016d5d2b2ff2dd33888c1b25d38d47c1f2c4749d/.github/workflows/cd.yml#L101-L109

At this point, the binaries have already been built.

I see two ways to address this:

  1. Move the Operator | Deduce step to a separate job that runs before the binaries are built. Only set FJ_OFFICIAL_RELEASE, if it detects that a release should happen.
  2. Don't build binaries at all, unless a release is detected. We have weekly releases now, so maybe we don't need the per-build binaries anymore?

I think I'd prefer solution 1, but I'd accept solution 2.

Labeling as https://github.com/hannobraun/Fornjot/labels/good%20first%20issue, since this only requires knowledge of GitHub Actions to address, not any deeper understanding of Fornjot in particular.

hannobraun avatar Jul 28 '22 11:07 hannobraun

@hendrikmaus has left a comment in #850 with a proposal on how to approach a solution to this issue.

hannobraun avatar Aug 08 '22 14:08 hannobraun

I'd like to work on that issue, but need to clarify some things first.

We have two flags:

  1. FJ_OFFICIAL_RELEASE (currently hardcoded to 1) which tells whether the release is official (e.g. production one) or not (e.g. development one).
  2. release-detected (wrapped as the Outputs::ReleaseDetected) set by the release operator (a.k.a. Operator | Deduce) which tells us whether we should trigger a release or not.

But... these two are the same flags, aren't they? If so, I've created a draft PR that unifies these two.

❗Please note, that I did not test this code against GH Actions.

kopackiw avatar Nov 08 '22 01:11 kopackiw

Thanks for the contribution @kopackiw. It is the way to go on this one. I left a review for you with a couple of comments. Let me know if there are any questions, I am happy to answer them.

hendrikmaus avatar Nov 08 '22 08:11 hendrikmaus