-
-
Notifications
You must be signed in to change notification settings - Fork 254
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 build image function to GitHub Actions #1008
Conversation
@ecdye Wait, there are a few things to consider.
...this is why I did it with a manual trigger. |
@holgerfriedrich To address your concerns, For open source repos, there is no limit on storage. Storage is only charged on private repos so we are fine in that regard. I can look into caching but I do not believe that downloads from source are a huge issue with our scope. If that is a real concern then I can look into making PRs a manual start . |
0b8c7b8
to
c464155
Compare
@holgerfriedrich Caching is possible and is now implemented and working, that should take care of all of your concerns? Any other issues before merging you have? And for more info on GitHub policy read https://help.github.com/en/actions/configuring-and-managing-workflows/persisting-workflow-data-using-artifacts and https://help.github.com/en/actions/configuring-and-managing-workflows/caching-dependencies-to-speed-up-workflows |
@ecdye good to know the GitHub policies on storage. Not sure yet which is the best approach to trigger. I do not like the idea that all of our commits and all PRs create images. This will waste space and computing power. If we trigger only on master and if build.bash is modified, we will not have an chance to test on PRs before merge.... |
I don't feel that it is really a waste of space as GitHub prunes it and takes care of. I am under the belief that because it is a service they offer and we will routinely make use of it, it is not an issue. It also is a part of our CI as it tests that what changes we make allow the image to build and then us to test on real HW. That is my opinion. What are your thoughts @mstormi? |
Nice try! But if you don't get that to work, it's fine to just place the .deb's somewhere and install during the target's installation run. |
@holgerfriedrich > but failed with dpkg install in mounted target image :-( apt-get install --download-only PKGNAME # mind that's assuming you run on the same architecture as the target does Then just have the system itself do the usual apt-get install on first time installation. It should use the pre-cached pkg. |
9a615f6
to
d78eda4
Compare
@mstormi @holgerfriedrich
I did not, however test offline install as it won't work quite right without #984 being finished. |
Just to let you know about the new GitHub functionality: https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/ |
That will be amazing if it works, my one concern is that I don't know if it will let people without write access trigger them. (i.e. me). Lets test and find out, as that is a much more ideal solution. |
Hard to tell if it works until is has been merged because the other test cases cause it to trigger. We may have to wait and see if it works as intended but it should. |
c870231
to
2a76f95
Compare
Mostly because those are the only ones that we have previoously enforced shellcheck on. If I were to do them all I would probably need to fix several issues and that is not really inside the scope of this PR. |
I was basically referring to the one you just introduced. But if this is a problem, don't mind to leave it out for now. |
The 'Build Image' action was primarily written by Holger Friedrich and then edited for better functionality in the openHABian core repo. This also contains other changes to the CI setup, such as moving the ShellCheck checks out of Travis and instead enforcing them in GitHub Actions. Along with theses changes, some improvements to the build script and install output were also made. These changes to the build script include using cURL over wget for downloading needed files and also cloning the openHABian repo in the build process rather than the install process. This change was made because the repo must be present for offline install to work, and as such it makes sense to provide it with the image to reduce points of failure on install. Fixes openhab#991 Co-authored-by: Holger Friedrich <[email protected]> Co-authored-by: Markus Storm <[email protected]> Signed-off-by: Ethan Dye <[email protected]>
Done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Let's see how it works.
@mstormi ready to merge?
The 'Build Image' action was primarily written by Holger Friedrich and
then edited for better functionality in the openHABian core repo.
This also contains other changes to the CI setup, such as moving the
ShellCheck checks out of Travis and instead enforcing them in GitHub
Actions.
Along with theses changes, some improvements to the build script and
install output were also made. These changes to the build script include
using cURL over wget for downloading needed files and also cloning the
openHABian repo in the build process rather than the install process.
This change was made because the repo must be present for offline
install to work, and as such it makes sense to provide it with the image
to reduce points of failure on install.
Fixes #991
Co-authored-by: Holger Friedrich [email protected]
Co-authored-by: Markus Storm [email protected]
Signed-off-by: Ethan Dye [email protected]