fix(deb): measure Depends: on Wayland and check the closure there (#325) - #327
Merged
Conversation
The .deb's `Depends:` was measured under X11 only (plan 022 used `strace`
under `xvfb-run`) and the closure check is X11-only too, so the gap was
structurally invisible: the X11 EGL platform never calls
`wl_egl_window_create`, so no amount of X11 testing can notice a missing
Wayland library. Wayland is the primary Linux target.
## Depends: += libegl1, libwayland-egl1
Measured over SEVEN configurations, not the four the plan called for:
{Wayland, X11} x {Vulkan ICD, none} x {libEGL reachable, not}.
The third axis is the whole finding. With neither a Vulkan ICD nor
libEGL, wgpu enumerates zero adapters and iced quietly renders on the CPU
via tiny-skia — so the GLES path is never entered and
`libwayland-egl.so.1` is never even probed. All four cells of the planned
matrix pass, none of them loads it, and their union would have kept the
gap open.
The failure, once libEGL is reachable (which on a real desktop it always
is): roost does not degrade, it SIGABRTs (exit 134) inside
`Surface::configure` after eight ENOENT probes for
`libwayland-egl.so.1`. iced's fallback compositor covers compositor
*creation*, not surface configure. Installing `libwayland-egl1` into that
same container is the positive control — it comes up on the GL backend
having opened the library.
So the two entries travel together: adding `libegl1` alone would CREATE
that crash on Wayland rather than prevent it. Nothing was removed; all
eleven existing entries were re-confirmed by trace. `libvulkan1` stays in
Recommends — measured, the app starts, stays up and renders a real frame
with no loader and no ICD. `libdecor` is confirmed NOT needed (zero hits
in all seven traces): sctk-adwaita draws decorations in-process.
## The closure check runs both display servers, compositor OUT of container
`--display x11|wayland|both`, defaulting to both, so the existing CI and
release callers pick up the Wayland leg with no workflow change.
Ubuntu's `weston` itself depends on `libwayland-egl1`, so installing it
beside the package under test would satisfy the very dependency under
test and the negative control would pass on a broken package. The
compositor therefore runs in its own container and only its socket is
shared; the package container installs NOTHING but the .deb. The X11 leg
gets the same treatment — Xvfb with `-ac` so no `xauth` is needed either.
Two assertions were added, and the first is the load-bearing one:
* every dlopened soname must resolve via `ldconfig -p`. This is the
detector. A launch-only check cannot catch this bug at all, because in
a `--no-install-recommends` container the app starts and renders
without either library.
* liveness: the app must still be alive and answering `identify` after a
further 8s, and exit with an expected status. `App::bootstrap` binds
the IPC socket before iced creates the window, so a single `identify`
only proves bootstrap got that far.
Verified in a Linux VM (aarch64): both legs pass in 54s; the negative
control — `libwayland-egl1` stripped from `Depends:`, `libegl1` kept —
reds the Wayland leg with `::MISSING:: libwayland-egl.so.1` and leaves
X11 green, which is the asymmetry the X11-only check could not express.
The `dpkg` state proves it: 136 packages with the dependency, 135
without, `diff` of the sorted lists exactly one line.
## Review findings
Fixed during review (four correctness bugs in the harness, all found by
running it rather than reading it):
* three shell-quoting/`set -e` defects meant the harness could never pass
— the payload's own apostrophes closed the `-c` quote, a `${binary:Package}`
format string expanded on the wrong side, and `wait` under `set -e`
exited 143 on every successful shutdown.
* `LAUNCH_TIMEOUT` wrapped `apt-get` as well as the launch, so a slow
mirror would have been reported as a Depends: diagnosis. Install and
launch are now separate phases.
* the remapped inner-timeout code collided with docker's own exit 125,
letting a daemon error masquerade as an app-launch timeout. It is 122
now, and 125/126/127 are reported as harness failures.
* the soname check was an unanchored `grep` with unescaped dots, so
`libEGL.so.10` would have satisfied `libEGL.so.1`. It is an exact
first-field match now.
Known limit: measured on aarch64. The dlopen sonames come from Rust
source constants that are not `cfg(target_arch)`-gated, so the list
should be arch-invariant, but a confirming amd64 run is cheap and CI will
do it on this PR.
Closes #325.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgxiEBQTqgNjPswKqcs12d
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe Debian closure check now supports X11 and Wayland validation. It starts isolated compositors, checks display-specific shared libraries, verifies application liveness, applies bounded timeouts, and reports distinct failure classes. Package metadata adds ChangesDebian closure validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant verify-deb-closure.sh
participant Display_container
participant Package_container
participant Debian_application
verify-deb-closure.sh->>Display_container: Start Weston or Xvfb
Display_container-->>verify-deb-closure.sh: Report display readiness
verify-deb-closure.sh->>Package_container: Install .deb and inspect sonames
Package_container-->>verify-deb-closure.sh: Return dependency result
verify-deb-closure.sh->>Debian_application: Launch with display environment
Debian_application-->>verify-deb-closure.sh: Remain alive and responsive
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
linux/scripts/verify-deb-closure.sh (1)
139-181: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
COMPOSITOR_TIMEOUTis spent twice per leg.
linux/scripts/verify-deb-closure.sh:139gives the fullCOMPOSITOR_TIMEOUTtodocker run -d(image pull), andlinux/scripts/verify-deb-closure.sh:172then starts a fresh readiness loop of the same duration. Worst case per leg is 600s at the default, and 1200s for--display both. The bound holds, so this is not a hang, but the single variable name reads like a single budget. Consider deriving the readiness deadline from the remaining time, or renaming to make the two-phase budget explicit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@linux/scripts/verify-deb-closure.sh` around lines 139 - 181, Make the compositor startup budget explicit in the flow around docker run and the readiness loop: avoid presenting COMPOSITOR_TIMEOUT as two independent full-duration phases. Either track a shared deadline and have the socket readiness loop use only the remaining time after docker run, or rename/configure the variables to clearly represent separate pull and readiness budgets while preserving the intended total bound.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@linux/scripts/verify-deb-closure.sh`:
- Around line 259-275: Add an early validation near the LAUNCH_TIMEOUT and
ROOST_CLOSURE_LIVENESS_SECONDS definitions that rejects configurations where the
liveness delay can consume the launch timeout’s available headroom, using the
existing timeout values and a clear failure message. Ensure the harness exits
before launching rather than allowing the launch phase to be misreported as a
timeout.
- Around line 326-354: Update cleanup to remove the deterministic package
container named "${run_id}-${leg}-package" in addition to compositor_cid. Ensure
this cleanup runs after DOCKER_TIMEOUT interrupts the docker client so the
daemon-side container and its share_dir bind mount are reaped.
In `@packaging/nfpm.yaml`:
- Around line 36-43: Update the measurement comment’s “first six axes” wording
to refer to the first two axes, and remove the obsolete lead-in on that line
while preserving the separate libEGL-axis explanation.
---
Nitpick comments:
In `@linux/scripts/verify-deb-closure.sh`:
- Around line 139-181: Make the compositor startup budget explicit in the flow
around docker run and the readiness loop: avoid presenting COMPOSITOR_TIMEOUT as
two independent full-duration phases. Either track a shared deadline and have
the socket readiness loop use only the remaining time after docker run, or
rename/configure the variables to clearly represent separate pull and readiness
budgets while preserving the intended total bound.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a24736a7-774b-45e5-9f25-20ca1762b7e5
📒 Files selected for processing (2)
linux/scripts/verify-deb-closure.shpackaging/nfpm.yaml
* `ROOST_CLOSURE_LIVENESS_SECONDS` is spent inside the phase bounded by `LAUNCH_TIMEOUT`, and the two are independent knobs. Raising the first past the second made the launch phase time out and report "the package did not come up" — the opposite of what happened. The combination is now rejected up front. * `timeout` around `docker run` kills the docker CLIENT; the daemon keeps the container and `--rm` never fires. The orphan outlived the script AND kept the shared directory bind-mounted, so the directory cleanup silently failed too. `cleanup` now removes it by its deterministic name. * The nfpm comment said "the first six axes" for a three-axis, eight-cell matrix. Chasing the second one surfaced a third leak the reviewer could not have seen from the diff: **Xvfb chowns its socket directory to root and sets the sticky bit**, so after the X11 leg the host cannot even rmdir the emptied directory. The shared path is now two levels — containers mount only the inner one, so the outer stays ours and is removable once a throwaway container has emptied it as root. Verified in the Linux VM: both legs still pass, and a run now leaves zero temp directories and zero containers behind (previously one directory per X11 leg). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgxiEBQTqgNjPswKqcs12d
This was referenced Aug 10, 2026
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.
Plan 024, PR 2 of 6 — workstream W-A. Closes #325. Follows #326.
The finding is bigger than the issue title
#325says "libwayland-egl1is missing". It is, but so islibegl1, and the twohave to land together — adding
libegl1alone would create a crash rather than preventone. More importantly, the measurement method the plan specified could not have found
either of them.
Seven configurations, not four
The plan pinned a
{Wayland, X11} × {Vulkan ICD, absent}matrix. I ran it. All fourcells pass, and not one of them ever probes
libwayland-egl.so.1— so their union wouldhave reproduced today's list and left the gap wide open.
The reason is a third axis nobody had: with neither a Vulkan ICD nor libEGL, wgpu
enumerates zero adapters and iced falls back to its tiny-skia CPU compositor. The entire
GLES path — and every library it loads — is never entered.
--no-install-recommendsproduces exactly that state, which is why the container check has been green on this bug
since it was written.
M5 is the real bug:
preceded by eight ENOENT probes for
libwayland-egl.so.1. iced's fallback compositorcovers compositor creation — which is why M2/M4 degrade gracefully — but the
wl_egl_windowfailure surfaces later, at surface configure, where there is nofallback. It is a hard abort, not a degraded render. M7 is the positive control: the only
difference from M5 is
apt-get install libwayland-egl1.Screenshot byte counts are a clean renderer fingerprint throughout: 23 249 B for every
GPU-backend leg, 23 146 B for the two tiny-skia legs — all non-trivial, so every "starts"
row above rendered a real frame.
Depends:deltaAdded
libegl1,libwayland-egl1. Removed nothing. All eleven existing entries werere-confirmed by trace.
libvulkan1correctly stays inRecommends(measured: the appstarts, stays up and renders with no loader and no ICD).
libdecoris confirmed NOTneeded — zero hits across all seven traces;
sctk-adwaitadraws decorations in-process.The closure check now runs Wayland, with the compositor outside the container
--display x11|wayland|both, defaulting toboth, soci.yml'siced-releaseandrelease.yml'slinuxjob pick up the Wayland leg with no workflow change.Ubuntu's
westondepends onlibwayland-egl1. Installing it beside the package undertest would satisfy the very dependency being tested and the negative control would pass on
a broken package — so the compositor runs in its own container and only its socket is
shared. The package container installs nothing but the .deb. The X11 leg gets the same
treatment (Xvfb with
-ac, so noxauthin the package container either), which closesthe same latent shape on that side.
Two new assertions, and the first is the one that matters
Soname resolvability via
ldconfig -p— this is the detector. A launch-only checkcannot catch this bug at all: in a
--no-install-recommendscontainer the app starts andrenders fine without either library. The launch is now the backstop, not the detector.
libwayland-egl.so.1is asserted on the Wayland leg only, because that is the truth —which is what makes the negative control asymmetric.
Liveness.
App::bootstrapbinds the IPC socket before iced creates the window, so arenderer/EGL failure that kills the process a moment later can still answer one
identifyinside the poll window. The app must now still be alive and answering after a further 8 s,
and exit with an expected status. Proven non-vacuous with a wrapper binary that binds and
then dies — the check reports
answered identify and then exited (status 9).Verification (Linux VM, aarch64, real
nfpmbuild from this branch)libwayland-egl1stripped,libegl1kept — Wayland leg::MISSING:: libwayland-egl.so.1, exit 3diffof the sorted lists is exactly one line,libwayland-egl1:arm64. Noweston/xvfb/xauthin the package container.libegl1without a vendor implementationshellcheck linux/scripts/*.sh+bash -nclean (CI's exact commands).One risk I chased down before shipping
libegl1is the vendor-neutral loader; I expectedlibegl-mesa0(the actualimplementation) to be a
Recommends, which--no-install-recommendswould drop — leavinga loadable-but-dead libEGL that might reach
Surface::configureand reproduce the verySIGABRT this change prevents. The premise was wrong: on Ubuntu 24.04
libegl1Depends on
libegl-mesa0. I forced the vendor-less state anyway(
dpkg --purge --force-depends libegl-mesa0) and it is also safe —eglInitializefailsduring adapter enumeration, so wgpu reports zero adapters and iced falls back before any
surface exists. The abort needs a working EGL to get as far as surface configure.
Review findings — four correctness bugs, all found by running the harness
apostrophes closed the
-cquote and spilled half the script into the outer shell; a${binary:Package}format string expanded on the wrong side and trippedset -u; andwaitunderset -emeant every successful leg exited 143 into the generic failurepath.
LAUNCH_TIMEOUTwrappedapt-getas well as the launch, so a slow mirror would havebeen reported as a
Depends:diagnosis. Install and launch are separate phases now.error masquerade as an app-launch timeout. It is 122 now; 125/126/127 report as harness
failures.
grepwith unescaped dots —libEGL.so.10would havesatisfied
libEGL.so.1. Exact first-field match now.Accepted risks / limits
cfg(target_arch)-gated, so the list should be arch-invariant — and this PR's CI is anamd64 run of the same check, which is the confirmation.
those packages are justified by the X11 legs and the binary's dlopen table.
mesa-vulkan-drivers/libgl1-mesa-dri's ownDepends:and must not be pinned here./tmpdir per leg used to leak (the compositor's socket is root-owned); cleanup nowborrows root from a throwaway container. Verified: zero leaked dirs after a run.
No impact on
Secrets, privacy. Package size grows by the
libegl1closure, which every desktop alreadyhas installed.
Summary by CodeRabbit
New Features
Bug Fixes