Extract the preview-emulator setup into a composite action - #22
Merged
Conversation
The two emulator-preview-* jobs each carried their own copy of the same setup: enable KVM, refresh cmdline-tools, install the ps16k image, create the AVD (with the 30-line hand-written-ini fallback), install emulators;latest, and install libpulse0. The copies had already started to drift -- the multi-run one had lost three explanatory comments and the "AVD root ini" / "preview emulator version" diagnostics -- and #21 widened the gap by rewriting shutdown logic in only one of them. Move all of it to .github/actions/preview-emulator, parameterised by api-level / target / abi / avd-name / device / ram-size / cores / disk-size so a second image can be added without another copy. ci.yml loses 153 lines. The action also owns the console auth token workaround from #21, which means the other preview job's "Stop emulator" step (adb emu kill) stops being a silent no-op too. That step now defers to an existing token instead of overwriting it, so once a preview build ships the proposed upstream fix that creates the file itself, the workaround becomes inert and can simply be deleted. Only the two non-blocking preview jobs are touched; build, test, smoke and android-cli-experiment are byte-identical. Verified by rendering the action's AVD step with its default inputs and running it against a sandbox HOME: the generated config.ini is byte-identical to what the inline version produced. (That check also caught dead code carried over from the inline copies -- a sed meant to strip heredoc indentation that never matched anything, since the YAML block scalar had already stripped it.)
Composite action steps must declare `shell: bash`, which GitHub runs as `bash --noprofile --norc -e -o pipefail`. The inline workflow steps this action replaced had no `shell:` key, so they got the workflow default `bash -e` -- without pipefail. Under pipefail, `yes | sdkmanager ... > /dev/null` fails: yes takes SIGPIPE when sdkmanager exits and the pipeline reports 141. Both preview jobs died on the first install. Turn pipefail off for the three steps that pipe into sdkmanager/avdmanager. That restores the previous semantics exactly -- the pipeline reports the last command's status, so a genuine sdkmanager failure still fails the step; only the SIGPIPE from yes is ignored. The remaining pipeline in the action (echo | sudo tee) is safe: tee consumes its input fully, and a tee failure propagating is the behaviour we want.
jpcottin
added a commit
that referenced
this pull request
Jul 26, 2026
setsid(2) runs asynchronously in the child, so for a brief window after backgrounding the launcher its pid still reports the *shell's* process group. Reading the pgid exactly once loses that race intermittently: the guard then sees EMU_PGID == SELF_PGID, correctly refuses to group-kill our own step, and silently drops to the single-pid fallback -- so the process group scoping added in #21 was not reliably active. Observed on the runner in 1 of 4 boot cycles (#22's run); 0 of 4 in #21's, which is why it was missed. Consequence was mild -- 'adb emu kill' is the primary shutdown path and the fallback still terminates the launcher -- but the scoping is the whole point of that change. Poll until the pgid settles instead, giving up only if the emulator dies or after 10s. The first iteration is identical to the previous single read, so this can only ever do better. Also log the resolved pgid, so a future run shows whether isolation actually happened rather than staying silent. The failure cannot be reproduced on an unloaded dev machine (0 losses in 40 trials); forcing the timing -- a child that lingers in our process group for 300ms before setsid -- the old single read loses 10/10 and the polling version loses 0/10.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Both
emulator-preview-*jobs carried their own copy of the same setup — enable KVM, refresh cmdline-tools, install the ps16k image, create the AVD (including the ~30-line hand-written-ini fallback), installemulators;latest, installlibpulse0.The copies had already drifted: the multi-run one had lost three explanatory comments and both the
AVD root iniandpreview emulator versiondiagnostics. #21 widened the gap further by rewriting shutdown logic in only one of the two.What changed
New
.github/actions/preview-emulator, parameterised byapi-level/target/abi/avd-name/device/ram-size/cores/disk-size, so another image can be added without a third copy.ci.ymlloses 153 lines and both jobs reduce to oneuses:.The action also owns the console auth token workaround from #21. Two consequences:
Stop emulatorstep (adb emu kill) stops being a silent no-op — it had the same unauthenticated-console problem, just without anything depending on it.Scope
Only the two non-blocking preview jobs are touched.
build,test,smokeandandroid-cli-experimentparse byte-identical tomain— verified by diffing the parsed YAML job objects, not just eyeballing.Verification
Composite actions can't run locally, so the parts that could be checked were:
HOME. The generatedconfig.iniis byte-identical to what the inline version produced.sedintended to strip heredoc indentation that never matched anything, because the YAML block scalar had already stripped it. Dropped rather than carried forward.runsteps inci.ymland all 6 in the action passbash -n.The real test is this CI run; both jobs remain
continue-on-error, so a mistake here can't gate anything.