Fix seven findings in the AppImage and Windows build scripts - #33
Merged
Conversation
Both Windows specs decoded droidwebdisplay.ico.base64 into packaging/windows/droidwebdisplay.ico, a path that is not gitignored. Every Windows build therefore left an untracked binary in the checkout, so `git status` came back dirty and a routine `git add -A` would commit a generated artifact. The icon now lands in PyInstaller's workpath (./build/..., already covered by the `**/build/` rule), falling back to ROOT/build when workpath is absent. The tracked .base64 remains the single source. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XfT5Z3Z24bjC8qtqERGpMQ
The Windows job checks PE FileVersion and ProductVersion against the VERSION file and fails the build on a mismatch. Linux had no equivalent, yet release.yml renames DroidWebDisplay-ci-linux-x86_64.AppImage to DroidWebDisplay-v<version>-linux-x86_64.AppImage and publishes it, so the filename asserted a version nothing had verified and its hash went into SHA256SUMS.txt on that basis. The new step extracts the built AppImage, compares the VERSION it actually bundles against the tree, and confirms adb is present -- matching what the Windows job already does for its two packages. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XfT5Z3Z24bjC8qtqERGpMQ
DroidWebDisplay.spec set upx=True on both EXE and COLLECT while both Windows specs use upx=False. upx is not installed on the Linux runner -- it appears in neither apt-get list -- so PyInstaller logged that UPX was unavailable and carried on. The setting was therefore untested configuration that would begin compressing binaries unreviewed the moment upx landed on an image, and UPX-compressing Qt binaries is a known cause of corrupt executables and antivirus false positives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XfT5Z3Z24bjC8qtqERGpMQ
droidwebdisplay.desktop.in carried no Icon key and install.sh never placed droidwebdisplay.svg on the icon search path, so a system install produced a launcher entry with a generic placeholder. The AppImage build meanwhile inlines its own desktop file that does set Icon=droidwebdisplay -- two definitions of the same entry, and the installed one was the poorer. The template now sets Icon=droidwebdisplay, install.sh copies the SVG into XDG hicolor/scalable/apps and refreshes the icon cache when the tool exists, and uninstall.sh removes it again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XfT5Z3Z24bjC8qtqERGpMQ
DroidWebDisplay.spec carried an `if sys.platform == "win32"` branch that CI never reaches -- Windows builds use the two dedicated specs. That branch produced a onefile exe with no icon and no VS version resource, which is precisely the binary ci.yml's PE metadata check exists to reject, so anyone running this spec on Windows got a package that would fail the gate for reasons the spec itself caused. It now refuses to run on Windows and names the specs to use instead. The platform-conditional adb name list and the duplicated EXE block collapse to the single Linux form. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XfT5Z3Z24bjC8qtqERGpMQ
Both specs did `int(part) for part in VERSION.split(".")`, so a VERSION of
0.11.8-rc.1 raised ValueError and failed the Windows build outright. This
project has already published -rc tags (v0.11.2-rc.1, stable-v0.11.2-rc.3),
so that is a shape it uses -- it simply has not reached the VERSION file yet.
The numeric VS_FIXEDFILEINFO tuple now comes from the leading MAJOR.MINOR.PATCH
and ignores any suffix, while the FileVersion and ProductVersion strings keep
the full VERSION text. A value that does not start with three numbers is still
rejected.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XfT5Z3Z24bjC8qtqERGpMQ
The Linux, Windows onefile and Windows onedir specs repeated the bundled-data list, the ADB discovery and the hidden-import computation verbatim -- 339 lines of which roughly 90% was duplicated. Adding a bundled file meant editing three places, and missing one produced a package broken on a single platform only. packaging/pyinstaller/_dwd_common.py now owns those inputs plus the VERSION parsing and the Windows icon decode. Each spec keeps only what genuinely differs: its platform guard, and its EXE/COLLECT shape. Spec files are exec'd rather than imported, so they reach the module through SPECPATH on sys.path; nothing in it is imported by the application. Verified by evaluating all three specs against a stubbed PyInstaller, which reproduces the pre-refactor inputs exactly: Linux gets 10 datas, 1 binary, no icon or version resource, COLLECT DroidWebDisplay; Windows onefile gets 10 datas, 3 binaries, icon and version, no COLLECT; Windows onedir the same plus COLLECT DroidWebDisplayWindowsOnedir. The Linux spec still refuses to run on win32. Added tests/packaging/test_pyinstaller_spec_inputs.py, which executes the module rather than grepping the specs -- a spec that stops bundling a file fails there instead of passing a source-text match. It skips when PyInstaller is absent, since only the packaging jobs install it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XfT5Z3Z24bjC8qtqERGpMQ
CI caught both. Neither is a behaviour regression; both are the same pattern
this codebase keeps hitting, where a test asserts source text and so breaks on
a refactor that changed nothing a user can observe.
test_packaging.py::test_windows_package_is_windowed_desktop_host read the
win32 branch of DroidWebDisplay.spec and asserted console=False inside it.
That branch was dead -- CI never reached it, and it built an exe with no icon
and no version resource, exactly what the PE metadata check rejects. Removing
it broke this test, which is to say the test existed to protect it. It now
asserts the property it is named for against the two specs Windows actually
builds, and that the Linux spec redirects Windows builds to them.
test_windows_package_hardening.py asserted the literal
`with_suffix(".ico.base64")` appeared in each Windows spec. That pinned how
the icon path was spelled rather than that the icon comes from the tracked
base64 source. It now asserts the specs delegate to common.windows_icon, that
the shared module names the .base64 source, and that neither spec writes back
to packaging/windows/droidwebdisplay.ico -- which is the actual property worth
protecting, since that path is not gitignored.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XfT5Z3Z24bjC8qtqERGpMQ
…setup form
Reported: first-run setup correctly shows the full form with PIN and Confirm
PIN, but locking afterwards showed that same setup form rather than a simple
unlock prompt.
The gate already renders two modes -- #renderGate(configured) sets the title,
the submit label, and hides the Confirm PIN row -- but the
droidwebdisplay-auth-required listener only did:
this.elements.gate.hidden = false;
It never re-rendered, so the gate reappeared in whatever mode it was last
drawn. After first-run setup that is setup mode, leaving "Create bridge PIN"
and a visible Confirm PIN box on screen for what is actually a login. The
submit handler reads #status.configured, which is true by then, so it did
perform a login -- the form simply described the wrong operation. A fresh page
load looked right, because ensureAuthenticated() renders from a fresh status;
only a lock within the same page session showed it.
The listener now re-reads /api/v1/auth/status and re-renders from it. The
server is the authority on whether a PIN exists, so this is also correct after
a PIN change or a revocation from another browser. If the status request fails
the last known value is used, so the user is never left without a way back in,
and if it reports the session is somehow still valid the app is shown instead.
Guarded against re-entry: several in-flight requests can each answer 401 at
once, and #renderGate clears the PIN input, so a burst would wipe the field
under someone already typing. The gate re-renders only when it is currently
hidden.
authStatus is a public request, so its own 401 cannot re-dispatch
droidwebdisplay-auth-required; there is no event loop here.
No node here, so dist/assets/auth-controller.js was hand-applied and the
manifest regenerated. The new layout test asserts the listener delegates, that
the handler re-reads status and re-renders, that the re-entry guard is present,
and that the old unhide-only listener is gone. Not verified in a browser: the
lock-and-reopen sequence itself.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XfT5Z3Z24bjC8qtqERGpMQ
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.
Review of the packaging path — the three PyInstaller specs, the AppImage assembly in
ci.yml, and the Linux installer. Seven findings, one commit each.1. Every Windows build left an untracked binary in the repo
Both Windows specs decoded
droidwebdisplay.ico.base64intopackaging/windows/droidwebdisplay.ico— a path not gitignored.git statuscame back dirty after any build, and a routinegit add -Awould commit a generated artifact. The icon now lands in PyInstaller'sworkpath(already covered by**/build/).2. The AppImage's version was never verified
The Windows job checks PE
FileVersion/ProductVersionagainstVERSIONand fails on mismatch. Linux had no equivalent — yetrelease.ymlrenamesDroidWebDisplay-ci-linux-x86_64.AppImagetoDroidWebDisplay-v<version>-linux-x86_64.AppImageand publishes it, so the filename asserted a version nothing had checked and its hash went intoSHA256SUMS.txton that basis. A new step extracts the built AppImage and compares theVERSIONit actually bundles.3.
upx=Trueon Linux was dead config on a live hazardupxis installed on neither runner, so PyInstaller logged "UPX is not available" and carried on — the setting was untested, and would have activated unreviewed the momentupxappeared on an image. UPX-compressing Qt binaries is a known cause of corrupt executables and AV false positives. Both Windows specs already usedupx=False.4. The installed Linux desktop entry had no icon
droidwebdisplay.desktop.incarried noIcon=key andinstall.shnever put the SVG on the icon search path, so a system install showed a generic placeholder — while the AppImage inlines its own desktop file that does setIcon=droidwebdisplay. Two definitions, and the installed one was the poorer.install.shnow installs the SVG into XDGhicolor/scalable/apps;uninstall.shremoves it.5. The Linux spec silently built a worse Windows exe
DroidWebDisplay.spechad anif sys.platform == "win32"branch CI never reaches, producing an exe with no icon and no version resource — precisely whatci.yml's PE check exists to reject. It now refuses to run on Windows and names the correct specs.6. Three specs, ~90% duplicated
The bundled-data list, ADB discovery and hidden-import computation were repeated verbatim across all three.
_dwd_common.pynow owns them; each spec keeps only its platform guard and its EXE/COLLECT shape. 339 lines → 345 across four files, but the duplication is gone.Verified by evaluating all three specs against a stubbed PyInstaller, reproducing pre-refactor inputs exactly: Linux 10 datas / 1 binary / no icon /
COLLECT DroidWebDisplay; Windows onefile 10 datas / 3 binaries / icon + version / no COLLECT; onedir the same plusCOLLECT DroidWebDisplayWindowsOnedir.7. A prerelease
VERSIONcrashed the Windows buildint(part) for part in VERSION.split(".")raises on0.11.8-rc.1. This project has published-rctags, so it's a shape it uses — it just hasn't reachedVERSIONyet. The numericVS_FIXEDFILEINFOtuple now takes the leadingMAJOR.MINOR.PATCHwhile the version strings keep the full text.Verification
No PyInstaller, node or pytest in the authoring environment, so the packages were not built here. All three specs parse, the stubbed evaluation matches prior behaviour, and both shell scripts pass
bash -n.Added
tests/packaging/test_pyinstaller_spec_inputs.py, which executes the shared module rather than grepping the specs — a spec that stops bundling a file fails there instead of passing a source-text match. It skips where PyInstaller isn't installed.CI is the real test here: it builds the AppImage and both Windows packages, and now verifies the AppImage version too.
🤖 Generated with Claude Code
https://claude.ai/code/session_01XfT5Z3Z24bjC8qtqERGpMQ