Add Workflow Action to build and upload webapp bundle on Release #2161

AlvinSchiller · 2023-12-13T19:11:08Z

Added a workflow action that builds and uploads the WebApp tar.gz bundle.
This workflow is triggered on publishing a release and the bundle will be added as an asset to this release.
The version is based on the given releases tag (webapp-<tag_name>.tar.gz).
Every release on any Branch is supported. Changed the format of the version extra to build a SemanticVersion compatible string.
Draft releases are NOT supported and will not trigger this (github trigger restriction).

~~Demo Release: https://github.com/AlvinSchiller/RPi-Jukebox-RFID/releases/tag/v3.1000.10000-alpha.2~~
~~Demo Run: https://github.com/AlvinSchiller/RPi-Jukebox-RFID/actions/runs/7196517075~~

~~Changes to come:~~

~~- [ ] Add fallback for WebApp installation option if current version bundle is not available~~

~~Important notes:~~
For the next release the version.py must be changed on the develop branch before the merge. Therefor after this is done CI Tests will fail (and after the merge also an installation of the main branch) cause the needed webapp bundle is not yet available without a published release.

~~So changing version.py should be done right before the merge. Also merging to main and publishing a release should be done right after another, to avoid such "errors".~~

Updated process: see #2161 (comment)

coveralls · 2023-12-13T19:13:28Z

Pull Request Test Coverage Report for Build 7235131391

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

4 of 5 (80.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-14.0%) to 38.384%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/jukebox/jukebox/version.py	4	5	80.0%

Totals
Change from base Build 7174343890:	-14.0%
Covered Lines:	418
Relevant Lines:	1089

💛 - Coveralls

pabera · 2023-12-13T20:31:27Z

Looks good so far!

Add fallback for WebApp installation option if current version bundle is not available

Why wouldn't it be available if this workflow is automatically triggered? Are you referring to a time where the release is out but the action failed and didn't produce an webapp artifact?

AlvinSchiller · 2023-12-13T21:10:16Z

Looks good so far!

Add fallback for WebApp installation option if current version bundle is not available

Why wouldn't it be available if this workflow is automatically triggered? Are you referring to a time where the release is out but the action failed and didn't produce an webapp artifact?

Come to think about it again, im not quite sure anymore xD
I thought of a lot of usecases while getting this to work, but for main this shouldn't actually be a case, if the release is correctly created.

AlvinSchiller · 2023-12-13T21:10:49Z

Here are some of the thoughts i had (feedback appreciated):

I had the idea (and it would actually be the right thing to do from a versioning aspect) that after a release, the version on develop will be raised (so currently on develop would be v3.5.0-alpha).
But this would straight break the current installation CI for the download webapp case and it would need to be adapted somehow (or only be run on main?).

Thinking further on this there could be a Prerelease (as draft releases are not supported) for the latest develop version (-alpha.x) to also supply a webapp bundle, so no inital build is needed.

I think i was still somewhere in this scenarios, when i added the part mentioned above.
Its probaly just to much hastle for now to change this, also given it doesn't add a benefit apart of lesser build time and a more clear version number.
And if its something to implement it should be another PR.

pabera · 2023-12-14T11:25:11Z

I had the idea (and it would actually be the right thing to do from a versioning aspect) that after a release, the version on develop will be raised (so currently on develop would be v3.5.0-alpha).

I like the idea, but I am not sure how this could be properly handled. We would have to tag every commit to f3/develop with the recent version tag? Else, how would github now where to to which release it should upload the artifects to? It's typically solved through nightly builds rather than an actual version. But I am not sure if that's supported. (https://dev.to/derlin/how-to-create-nightly-releases-with-github-actions-fpp or https://github.com/knightlyjs/knightly)

AlvinSchiller · 2023-12-14T14:03:39Z

Yes, thats the tricky part and im also unsure how this can be done best. Also all steps have to be done automatically, otherwise its just not feasible.

I also see the options you described.

We Tag every new PR Merge commit on future3/develop (e.g. v3.5.0-alpha.x) and create a PreRelease to upload the bundle to. Also there would be the need to update the version.py to "point" to the correct release/tag for installation.

Positive: every tag-version has its distinctable bundle and on installation this can be used and fits the state of the sources.
Negative: it causes a lot of prereleases, tags and addidtional commits. there could/should be a purge mechanism to delete outdated prereleases and tags, but thats even more complexity.

Much like described in https://dev.to/derlin/how-to-create-nightly-releases-with-github-actions-fpp we just supply the latest bundle build for future3/develop.
This would need only one PreRelease with a tag on the new version commit (e.g v3.5.0-alpha) which is created on develop after merging to main. version.py doesn't have to be updated after this, until the next release to main.
Then every commit to future3/develop which affects the webapp triggers the action and uploads and overwrites the bundle.

Positive: Its a lot less complex and the webapp bundle only gets touched if there are changes.
Negative: The bundle is not distinctable, its just latest and greatest. This could cause problems if installations are done from an older (incompatible) commit, e.g. from fork branches that haven't updated to latest develop yet.
This is actually the current situation with the main bundle for "non-production" branches and why local installation is the default.

Ideas to deal with the negatives of 2, which i think are pretty serious and could cause more unnecessary bugreports.

a. Only supply the bundle if installed from the main repo, otherwise local build is needed. Again current behavior of "non-production" branches

Positive: see 2 + no negatives of 2.
Negative: Not useable for forks (but can be worked around with env overrides)

b. Don't upload latest and greatest, but "version" the bundle with the corresponding commit hash (e.g. webapp-v3.5.0-alpha.89078bdf4c19cd08f8ec11c806428da34db43190.tar.gz). This way the bundles are distinctable and the needed bundle could be calculate from git history (find the last shared commit with the main repo branch: git merge-base upstream/future3). Every Commit on future3/develop has to build the webapp though!
Still local builds are mandatory if changes are made to the webapp on the fork branch (or this can also be calculated from git history?). Complexity is pretty high again.

Positive: Also usable on forks.
Negative: A lot more complex. Still can cause incompatible webapp sources to be deployed on forked branches with custom changes.

If we want to add the this "develop version" feature, personally i would suggest to go with case 2.a.
It has a medium complexity and a quite low overhead. To support this only minor changes to the installation process would be needed, cause the behavior is already known for "non-production" branches.
Also if someone wants to use the bundle on a forked branch it can be worked around with env overrides (CI workflow curently uses this to test download)

pabera · 2023-12-14T16:26:57Z

While I agree with all your points, you are taking a sledgehammer to crack a nut. Let's not overthink this.
We can expect a few things. For Builders, the recommendation is to use main and this branch always has a proper build. I would never expect someone to use develop for a production Phoniebox. And if you do, you do at it at your own risk.

And if someone still wants to use develop, there is a high chance they use the latest commit (and also get the latest knighly build, see last paragraph). -> 99.9% covered.

Ideas to deal with the negatives of 2, which i think are pretty serious and could cause more unnecessary bugreports.

I consider this an extreme edge case and therefore don't think this is very serious nor will it cause many bug reports.

And yes, for people that want to build their own branch, building the webapp locally remains the first choice. It should be the default use case. The download is just there because some Pi's just don't have the memory to compile a NodeJS app (up to Pi 3b+, Zero2 etc)

Last but not least, for the main repo, we could always trigger a webapp build after a commit which is saved with webapp-build-COMMITHASH.tar.gz which we can then reference in the install script.

Updating version.py regularly does not scale. Updating tags to the next commit doesn't scale either. I also don't recognize this behaviour from other projects, tbh.

AlvinSchiller · 2023-12-14T23:11:37Z

Updating version.py regularly does not scale. Updating tags to the next commit doesn't scale either. I also don't recognize this behaviour from other projects, tbh.

Absolutly agree to this. It was just an option in the line of thoughts, but nothing you would really want to do.

We can expect a few things. For Builders, the recommendation is to use main and this branch always has a proper build. I would never expect someone to use develop for a production Phoniebox. And if you do, you do at it at your own risk.

And if someone still wants to use develop, there is a high chance they use the latest commit (and also get the latest knighly build, see last paragraph). -> 99.9% covered.

You are right. That should be the part we should care about most and what point 2.a. is also about.
To be honest: the other options where just putting my thoughts somewhere and to not miss an important part. It surely would be well overdesigned for the simple feature we want and the usecases given (like you said "a sledgehammer to crack a nut" 😆 ).

So i think we are pretty close to a process definition.

Automation:

Every push to develop triggers an action to build and upload the webapp.
The action
- builds the webapp and creates a bundle webapp-build-COMMITHASH.tar.gz
- tries to find a release matching the current version from version.py (e.g. v3.5.0-alpha)
  - if no such release is found it creates one
- the bundle will be uploaded to the release

Installation:

The installation process will check if a bundle for the current version and commit hash is available.
- if true: download and deploy the bundle
- if false: show the user a message that no prebuild bundle is available and the webapp needs to be build locally.

Should this process also be applied to the main branch? This would automate the release creation and make the overall process a lot easier. Only the updates of the release description would be necessary afterwards.

pabera · 2023-12-15T07:03:17Z

Process looks good to me 👍

tries to find a release matching the current version from version.py (e.g. v3.5.0-alpha)

Do I understand this correctly: After a merge to main, we must update version.py first?

Should this process also be applied to the main branch?

I believe yes. But here, I would like the build to not have the commit hash as part of the name :-)

AlvinSchiller · 2023-12-15T07:39:42Z

I believe yes. But here, I would like the build to not have the commit hash as part of the name :-)

Ok, always questionable to do the same thing but different, but should be possible without to much hassle ^^
We would not have the case that more then one bundle is needed right? Latest and greatest for one version should be fine on main?

Do I understand this correctly: After a merge to main, we must update version.py first?

I would suggest, changing the version BFEORE the merge (the last commit). Just like its done at the moment.
So there are no diffs on main and develop.
After the merge, the version on develop has to be changed again to the next "develop version".
If the version would be changed on main (release version) and also on develop (next develop version) there would be a merge conflict for the next release.

But that bring up another point. if the version is changed on develop, this must not trigger the creation of a new release, as it would tag the wrong commit. This has to be implemented in the action (ignore the run if on develop branch, an version has no identifier). This would mean between the version changes before and after the merge to main, this action wont run on the develop branch. But i think that should be fine.

pabera · 2023-12-15T07:43:34Z

We would not have the case that more then one bundle is needed right? Latest and greatest for one version should be fine on main?

I am not too bound to this. Let's keep it simple, even if it means have have commit hashs in the name.

The rest all makes sense. One last question though. Will every commit trigger a new build even though the webapp code has not changed? I guess it would, right?

AlvinSchiller · 2023-12-15T07:58:25Z

I am not too bound to this. Let's keep it simple, even if it means have have commit hashs in the name.

👍 We can see how it works out, and if we want to adjust that later on.

One last question though. Will every commit trigger a new build even though the webapp code has not changed? I guess it would, right?

I would say yes, as it makes the process less complex. Every commit has its own bundle, so we can just use the current commit hash. The bundle has currently a size of 1.3 MB so gathering every commit bundle in the release shouldn't be a problem to soon, even with a few hundred commits.
Otherwise there has to be a logic to find the last commit with changes on the webapp and use that commit hash to get the correct bundle.

remove old workflow

add download for latest bundle: used if no bundle for the commit is found and force download is set.

changed order: kiosk_mode and node option change message to better reflect the behavior. move local build finmessage to the 'yes' case: Download is forced and node is not installed. the message is irritating

AlvinSchiller · 2023-12-16T22:53:07Z

Implemented Process

Automation

Every push to future3/main or future3/develop triggers an action to build and upload the webapp to a release
The action
- checks if it should run
  - only on the MainRepo "MiczFlor"
  - only for branches future3/main or future3/develop
  - on future3/main only if the version in version.py is a release version*
  - on future3/develop only if the version in version.py is a prerelease version*
- builds the webapp and creates a bundle webapp-build-COMMITHASH.tar.gz
  - the bundle is saved as artifact on the job with a 5 days retention, in case its needed for manual deployment to the release or else
- tries to find a release matching the current version from version.py
  - if no such release is found one is created (Release for future3/main with generated releasenotes, Prerelease for future3/develop)
- uploads the bundle as webapp-build-COMMITHASH.tar.gz and also as webapp-build-latest.tar.gz (see installation)
- the release is updated, but name and body will not be overwritten.

* version string are validated against Semantic Versioning format
e.g. release: 1.0.0, 3.5.0, 100.4.50000+metadata
e.g. prerelease: 1.0.0-alpha, 3.5.0-whatsoever.12, 100.4.50000-identifier.12+metadata

Installation

The installation options to choose are not changed (description was updated though to better reflect the behavior).
The setup process will check if a bundle for the current version and commit hash is available
- if true: download and deploy the bundle
- if false:
  - check if download is forced (ENABLE_WEBAPP_PROD_DOWNLOAD == true) and if a webapp-build-latest.tar.gz bundle for the current version is available
    - if true: download and deploy the bundle (possible stale and incomaptible, just like before). Used e.g. for CI and possible fork installations.
    - if false: the installation is aborted, as the webapp download was chosen but no prebuild bundle for download was found

Demo

Test Branch with current changes (demo versionnumber 3.999999.999999-alpha)
Pull Request to merge changes. Action is not triggerd.
Merge on AlvinSchiller/future3/develop results in Action run and creation of Tag and Prerelease v3.999999.999999-alpha
New Commit on AlvinSchiller/future3/develop results in Action run and adds another bundle to existing Prerelease v3.999999.999999-alpha
Pull Request to merge changes to AlvinSchiller/future3/main. Action is not triggerd (don't know why the last run from the commit is shown in checks)
Merge on AlvinSchiller/future3/main results in Action run but cancels due to incorrect release version number
New Commit on AlvinSchiller/future3/develop to change version number to release results in Action run but cancels due to incorrect prerelease version number
Pull Request to merge changes to AlvinSchiller/future3/main. Action is not triggerd (don't know why the last run from the commit is shown in checks)
Merge on AlvinSchiller/future3/main results in Action run and creation of Tag and Release v3.999999.999999 including releasenotes
New Commit on AlvinSchiller/future3/main results in Action run and adds another bundle to existing Release v3.999999.999999
New Commit on AlvinSchiller/future3/main to change version number to next bugfix release results in Action run and creation of Tag and Release v3.999999.1000000 including releasenotes

installation/routines/customize_options.sh

src/jukebox/jukebox/version.py

.github/workflows/bundle_webapp_and_release_v3.yml

AlvinSchiller · 2023-12-17T06:12:37Z

The CI with the webapp download fails as there is currently no prerelease for develop with a latest bundle. So after the merge it will succeed.

s-martin · 2023-12-17T19:50:54Z

Does it make sense to use the short hashes in the file name of the web app package?

feels easier to handle :)

pabera · 2023-12-17T20:42:19Z

I am late to the party now.

If possible, let's reduce the commit hash length to 7 digits, it's a common thing to do: A short git commit hash is an abbreviation of the hash to the first 7 characters, it is almost certainly unique within a repository and git will increase the number of characters used if it is not.

installation/routines/customize_options.sh

AlvinSchiller · 2023-12-17T22:28:23Z

Does it make sense to use the short hashes in the file name of the web app package?
feels easier to handle :)

I didn't use the short version as it wasn't easier to begin with :)
It needs manually fiddling with the hashes and thinking about possible problems on collisions ^^
And as we just have it as a filename, that someone will hardly ever use manually, i didn't see it as a problem.

If possible, let's reduce the commit hash length to 7 digits, it's a common thing to do: A short git commit hash is an abbreviation of the hash to the first 7 characters, it is almost certainly unique within a repository and git will increase the number of characters used if it is not.

Although common some problems with duplication are reported.
Git will increase the number of characters, but thats only true for future access. And we can't use this automatic mechanism as we have fixed length stored hash, which we have to match.
It most likely will never hit though, as the duplication would need to be in the same version, but in the unlikely event it breaks the installation or has unexpected side effects.

If we really shorten the length, i would go for some more characters like 12 -15, or so. Its still a lot less, but should more ensure uniqueness.

AlvinSchiller · 2023-12-19T11:04:12Z

@s-martin @pabera
Change to shorter hashes? If yes, how short?

pabera · 2023-12-19T11:06:21Z

I believe 7 chars is the typical length, it's what I read

AlvinSchiller · 2023-12-19T11:48:44Z

Thas what a read regarding the default length and its uniquness.

How much of a git sha is generally considered necessary to uniquely identify a change in a given codebase?

https://stackoverflow.com/a/18134919
https://stackoverflow.com/a/21015031

git-config.txt-coreabbrev

core.abbrev

the length object names are abbreviated to. If unspecified or set to "auto", an appropriate value is computed based on the approximate number of packed objects in your repository, which hopefully is enough for abbreviated object names to stay unique for some time. If set to "no", no abbreviation is made and the object names are shown in their full length. The minimum length is 4.

Git-Tools-Revision-Selection

Short SHA-1
...
Generally, eight to ten characters are more than enough to be unique within a project. For example, as of February 2019, the Linux kernel (which is a fairly sizable project) has over 875,000 commits and almost seven million objects in its object database, with no two objects whose SHA-1s are identical in the first 12 characters.

Maybe just go for 10 so no one has to touch this very soon? ^^

…zFlor#2161) * fix editorconfig for yml files * add workflow for webapp build on release * filter paths for CI installation worlflow * abort installation if needed download failed * fix SemVer definition * add workflow for webapp bundle build and releases. remove old workflow * add main to version.py * fix prebuild webapp bundle download add download for latest bundle: used if no bundle for the commit is found and force download is set. * changes to webapp build option changed order: kiosk_mode and node option change message to better reflect the behavior. move local build finmessage to the 'yes' case: Download is forced and node is not installed. the message is irritating * add checks for correct version for branch * activate official repo check * set next develop version * Update message * make webapp downloads on forks possible * bugfix elif * add abort if sources failed to load * Updated WEBAPP NODE message * change filename to short hash (10 chars) * add semver ref to version.py

AlvinSchiller added 5 commits December 13, 2023 15:14

fix editorconfig for yml files

00360fc

add workflow for webapp build on release

fee072f

filter paths for CI installation worlflow

8457c8a

abort installation if needed download failed

1644d27

fix SemVer definition

e0d2403

AlvinSchiller requested review from pabera and s-martin December 13, 2023 19:13

s-martin approved these changes Dec 13, 2023

View reviewed changes

AlvinSchiller added 7 commits December 16, 2023 22:03

add workflow for webapp bundle build and releases.

00ba9c8

remove old workflow

add main to version.py

68ece5a

fix prebuild webapp bundle download

61bab75

add download for latest bundle: used if no bundle for the commit is found and force download is set.

changes to webapp build option

9379f1f

changed order: kiosk_mode and node option change message to better reflect the behavior. move local build finmessage to the 'yes' case: Download is forced and node is not installed. the message is irritating

add checks for correct version for branch

97e2a50

activate official repo check

6d760b9

set next develop version

cd4702a

s-martin reviewed Dec 16, 2023

View reviewed changes

installation/routines/customize_options.sh Outdated Show resolved Hide resolved

s-martin reviewed Dec 16, 2023

View reviewed changes

src/jukebox/jukebox/version.py Show resolved Hide resolved

s-martin reviewed Dec 16, 2023

View reviewed changes

.github/workflows/bundle_webapp_and_release_v3.yml Show resolved Hide resolved

s-martin approved these changes Dec 16, 2023

View reviewed changes

s-martin reviewed Dec 16, 2023

View reviewed changes

.github/workflows/bundle_webapp_and_release_v3.yml Show resolved Hide resolved

AlvinSchiller added 2 commits December 17, 2023 00:11

Update message

904f211

make webapp downloads on forks possible

2f57174

AlvinSchiller marked this pull request as ready for review December 16, 2023 23:28

AlvinSchiller added 2 commits December 17, 2023 01:47

bugfix elif

02ece37

add abort if sources failed to load

0cbf61e

AlvinSchiller force-pushed the future3/feature/action_webapp branch from 1a190eb to 0cbf61e Compare December 17, 2023 00:52

pabera reviewed Dec 17, 2023

View reviewed changes

installation/routines/customize_options.sh Outdated Show resolved Hide resolved

Updated WEBAPP NODE message

a955a59

pabera approved these changes Dec 17, 2023

View reviewed changes

AlvinSchiller added 2 commits December 19, 2023 17:17

change filename to short hash (10 chars)

a44e944

add semver ref to version.py

e5d3319

s-martin approved these changes Dec 19, 2023

View reviewed changes

AlvinSchiller merged commit 9e65de4 into MiczFlor:future3/develop Dec 19, 2023
14 of 18 checks passed

AlvinSchiller deleted the future3/feature/action_webapp branch December 19, 2023 21:30

pabera added this to the v3.5 milestone Jan 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Workflow Action to build and upload webapp bundle on Release #2161

Add Workflow Action to build and upload webapp bundle on Release #2161

AlvinSchiller commented Dec 13, 2023 •

edited

Loading

coveralls commented Dec 13, 2023 •

edited

Loading

pabera commented Dec 13, 2023

AlvinSchiller commented Dec 13, 2023

AlvinSchiller commented Dec 13, 2023 •

edited

Loading

pabera commented Dec 14, 2023

AlvinSchiller commented Dec 14, 2023

pabera commented Dec 14, 2023 •

edited

Loading

AlvinSchiller commented Dec 14, 2023

pabera commented Dec 15, 2023 •

edited

Loading

AlvinSchiller commented Dec 15, 2023

pabera commented Dec 15, 2023

AlvinSchiller commented Dec 15, 2023

AlvinSchiller commented Dec 16, 2023

AlvinSchiller commented Dec 17, 2023

s-martin commented Dec 17, 2023

pabera commented Dec 17, 2023

AlvinSchiller commented Dec 17, 2023 •

edited

Loading

AlvinSchiller commented Dec 19, 2023

pabera commented Dec 19, 2023

AlvinSchiller commented Dec 19, 2023

Add Workflow Action to build and upload webapp bundle on Release #2161

Add Workflow Action to build and upload webapp bundle on Release #2161

Conversation

AlvinSchiller commented Dec 13, 2023 • edited Loading

coveralls commented Dec 13, 2023 • edited Loading

Pull Request Test Coverage Report for Build 7235131391

Warning: This coverage report may be inaccurate.

💛 - Coveralls

pabera commented Dec 13, 2023

AlvinSchiller commented Dec 13, 2023

AlvinSchiller commented Dec 13, 2023 • edited Loading

pabera commented Dec 14, 2023

AlvinSchiller commented Dec 14, 2023

pabera commented Dec 14, 2023 • edited Loading

AlvinSchiller commented Dec 14, 2023

Automation:

Installation:

pabera commented Dec 15, 2023 • edited Loading

AlvinSchiller commented Dec 15, 2023

pabera commented Dec 15, 2023

AlvinSchiller commented Dec 15, 2023

AlvinSchiller commented Dec 16, 2023

Implemented Process

Automation

Installation

Demo

AlvinSchiller commented Dec 17, 2023

s-martin commented Dec 17, 2023

pabera commented Dec 17, 2023

AlvinSchiller commented Dec 17, 2023 • edited Loading

AlvinSchiller commented Dec 19, 2023

pabera commented Dec 19, 2023

AlvinSchiller commented Dec 19, 2023

AlvinSchiller commented Dec 13, 2023 •

edited

Loading

coveralls commented Dec 13, 2023 •

edited

Loading

AlvinSchiller commented Dec 13, 2023 •

edited

Loading

pabera commented Dec 14, 2023 •

edited

Loading

pabera commented Dec 15, 2023 •

edited

Loading

AlvinSchiller commented Dec 17, 2023 •

edited

Loading