Add CI for example projects#37
Conversation
|
Note that this PR would require that actions be enabled for this repository. |
digitalby
left a comment
digitalby
left a comment
There was a problem hiding this comment.
Nice PR, the install flow mirrors Getting Started nicely and the version-override hooks are clean. Dropping a pile of nits below, nothing blocking, feel free to ignore any that feel unhelpful.
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| #swift_version: ['nightly-main', '6.3'] |
There was a problem hiding this comment.
commented-out nightly matrix is gonna rot if we forget about it (which... we will). either rip it out or promote it to a workflow_dispatch input so you can actually kick off a nightly run without editing the file. leaving it here means in six months someone uncomments it, it doesn't work, and we all pretend we didn't see it
| echo "$HOME/.local/share/swiftly/bin" >> "$GITHUB_PATH" | ||
| "$HOME/.local/share/swiftly/bin/swiftly" --version | ||
| elif [[ "${RUNNER_OS}" == "macOS" ]]; then | ||
| curl -O https://download.swift.org/swiftly/darwin/swiftly.pkg && \ |
There was a problem hiding this comment.
tiny style nit: you've got set -euxo pipefail at the top of this step but the macOS branch still chains everything with && \. under -e the chain is redundant and the linux branch right above doesn't do it, so it reads inconsistent. just newline-separate like the linux block?
| echo "Unknown OS: ${RUNNER_OS}" | ||
| exit 1 | ||
| fi | ||
|
|
There was a problem hiding this comment.
nit: quoting on $GITHUB_ENV / $GITHUB_PATH is inconsistent across the file. some lines quote it, this one (and a couple below) don't. harmless but my eye keeps snagging on it, would just quote everywhere
| - name: Configure Swift Android SDK | ||
| run: | | ||
| set -euo pipefail | ||
| cd $HOME/.swiftpm/swift-sdks/*android*.artifactbundle || cd $HOME/.config/swiftpm/swift-sdks/*android*.artifactbundle || cd $HOME/Library/org.swift.swiftpm/swift-sdks/*android*.artifactbundle |
There was a problem hiding this comment.
this cd glob || cd glob || cd glob chain is a bit of a timebomb. the globs are unquoted so if *android*.artifactbundle ever matches more than one directory, cd gets multiple args and the step dies with a weird error. also if the first path exists but is empty, the fallback never runs.
safer pattern would be something like:
shopt -s nullglob
candidates=(
"$HOME"/.swiftpm/swift-sdks/*android*.artifactbundle
"$HOME"/.config/swiftpm/swift-sdks/*android*.artifactbundle
"$HOME"/Library/org.swift.swiftpm/swift-sdks/*android*.artifactbundle
)
[[ ${#candidates[@]} -gt 0 ]] || { echo "no android SDK bundle found"; exit 1; }
cd "${candidates[0]}"or just parse swift sdk list output since you already call it the step before
| swift sdk list | ||
|
|
||
| # set environment variables used by swift-android.gradle.kts | ||
| echo "SWIFT_VERSION=${swift_install}" >> $GITHUB_ENV |
There was a problem hiding this comment.
worth a one-liner comment here explaining that you're deliberately overwriting the matrix SWIFT_VERSION with the resolved patch version (e.g. 6.3 -> 6.3.1) so gradle picks up the actual bundle dirname. i had to read the jq block twice to convince myself this wasn't a bug. future me will thank present you
| exit 1 | ||
| fi | ||
|
|
||
| - name: Install Android NDK |
There was a problem hiding this comment.
no caching = every run re-downloads the NDK (~1GB) and the android swift SDK. that's a lot of bytes off download.swift.org and a couple of minutes off every CI run across both OSes. actions/cache keyed on ${{ matrix.ndk_version }} and ${{ matrix.swift_version }} would be an easy win, and friendlier to the mirror. fine to land as-is and do it in a follow-up though
| @@ -0,0 +1,211 @@ | |||
| name: ci | |||
There was a problem hiding this comment.
no concurrency: group means rapid pushes to a PR branch queue up N redundant matrix runs. usual pattern for this repo would be something like:
concurrency:
group: ci-${{ github.ref }}
cancel-in-progress: ${{ github.event_name == 'pull_request' }}at the top of the workflow
|
|
||
| - name: Build hello-cpp-swift APK | ||
| run: | | ||
| cd hello-cpp-swift/cpp-lib |
There was a problem hiding this comment.
micro-nit: cd hello-cpp-swift/cpp-lib && ... && cd - would read cleaner as either a (subshell) or a dedicated step with working-directory:. the cd - in particular is the kind of thing that silently breaks if someone adds a command above it that cds somewhere else
| // Swift toolchain version passed to swiftly (e.g. "6.3", "main-snapshot"). | ||
| // Can be overridden via the SWIFT_VERSION environment variable, which is | ||
| // useful for CI matrices that test multiple toolchains. | ||
| var swiftVersion: String = System.getenv("SWIFT_VERSION")?.takeIf { it.isNotEmpty() } ?: "6.3", |
There was a problem hiding this comment.
subtle behavior mismatch with the three groovy files: here you do ?.takeIf { it.isNotEmpty() } ?: "6.3", but the .gradle files do System.getenv("SWIFT_VERSION") ?: "6.3". if someone exports SWIFT_VERSION="" (which CI envs do more often than you'd think), kotlin falls back to 6.3 but groovy keeps the empty string and blows up later in the bundle path. align both sides, prefer the kotlin behavior
| // Swift toolchain version passed to swiftly (e.g. "6.3", "main-snapshot"). | ||
| // Can be overridden via the SWIFT_VERSION environment variable, which is | ||
| // useful for CI matrices that test multiple toolchains. | ||
| def swiftVersion = System.getenv("SWIFT_VERSION") ?: "6.3" |
There was a problem hiding this comment.
not your fault, but worth calling out: this exact 6-line block is now duplicated across three build.gradle files and will drift the next time someone touches it. prime candidate for hoisting into swift-android.gradle.kts (or a root ext { }) so version bumps land in one place. happy to leave as pre-existing debt for a follow-up PR, totally fine to not touch it here
|
@shahmishal Could you turn on actions in the settings for this repo so I can iterate on testing the GitHub workflows? |
2 participants
This PR adds GitHub CI that will install the Swift SDK for Android and build each of the individual projects.
It also lightly modifies the gradle build files to enable overriding the Swift version it uses with
SWIFT_VERSIONandSWIFT_ANDROID_SDK_VERSIONenvironment variables in order to enable building with versions other than the presumed 6.3 (including nightly toolchains, although those aren't currently enabled in the CI).Note that we manually install everything in accordance with the Getting Started guide rather than leaning on the Android SDK support in
github-workflows(swiftlang/github-workflows#172) because it is closer to the experience that an end-user would have, plus this CI supports building on macOS as well as Linux. Also, thegithub-workflowsis designed to build-and-test a normal Swift package against a given SDK, whereas this workflow only builds the Swift package indirectly via the gradle builds.A successful run can be seen at https://github.com/marcprux/swift-android-examples/actions/runs/24172716605
Closes #34