-
Notifications
You must be signed in to change notification settings - Fork 228
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
Autobuild: Combine and simplify Android build scripts #2527
Autobuild: Combine and simplify Android build scripts #2527
Conversation
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.
Thanks! I don't have an android device with me at the moment, but the CI looks ok. See my - partly unrelated - comments below.
pass_artifact_to_job | ||
;; | ||
*) | ||
echo "Unknown stage '${1:-}'" |
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.
Although this would apply to all scripts: We should probably output something like "NOTE: This script is designed to run on GitHub Actions, not locally. Please run the deploy scripts in the respective OS folder"
Related: jamulussoftware#2503 - Move all autobuild/android/* scripts to a single .github/autobuild/android.sh script which is called for the different stages (setup/build/get-artifacts). - Create functions with proper names for larger steps - Inline Github artifact output definition as it's shorter that way - Make shellcheck-clean - Drop wget dependency and use curl for all downloads - Keep variables as local as possible; only export what's necessary - Move further hardcoded versions to variables - Move build output to build/ - Drop dependencies libxkbcommon-x11-0 (unused) and openjdk-8-jre-headless (covered via jdk)
Now that all platforms have been converted to the new scheme (.github/autobuild/*), the autobuild directory only contains the ensure_THIS_JAMULUS_PROJECT_PATH.sh script which is no longer used. Related: jamulussoftware#2521
0097888
to
7803eb5
Compare
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.
Didn't test due to the lack of a real android device but it compiles, so it should be ok?
Edit: Tested via Android in Qemu and it works - even with sound ;-).
Short description of changes
Move all autobuild/android/* scripts to a single
.github/autobuild/android.sh script which is called for the different stages
(setup/build/get-artifacts).
Create functions with proper names for larger steps
Inline Github artifact output definition as it's shorter that way
Make shellcheck-clean
Drop wget dependency and use curl for all downloads
Keep variables as local as possible; only export what's necessary
Move further hardcoded versions to variables
And as this is also the last PR which converts the platform-specific scripts, this includes the now possible cleanup:
Now that all platforms have been converted to the new scheme (.github/autobuild/*), the autobuild directory only contains the
ensure_THIS_JAMULUS_PROJECT_PATH.sh script which is no longer used.
CHANGELOG: Autobuild: Refactored and simplified Android build scripts.
(will be condensed...)
Context: Fixes an issue?
Related: #2521
Does this change need documentation? What needs to be documented and how?
No.
Status of this Pull Request
Ready for review.
What is missing until this pull request can be merged?
Reviews.
Checklist