fix(desktop): resolve the packed app by its configured product name - #85960
fix(desktop): resolve the packed app by its configured product name#85960vadimcomanescu wants to merge 1 commit into
Conversation
135e7d3 to
b08ea0e
Compare
|
Amended the branch (force-push, no reviews yet) to close the one rough edge the original body flagged, so nobody has to read a caveat to explain a red test.
The test now compares by file identity instead of by string: resolved = cli_main._desktop_packaged_executable(desktop_dir)
assert resolved is not None
assert resolved.samefile(exe)
assert resolved.parent == exe.parent
assert resolved.name.lower() == "aurora"Production code is untouched by the amendment — the probe order in Evidence, same machine:
The body has been updated to describe this final state. |
fix(desktop): resolve the packed app by its configured product name
|
What does this PR do?
The desktop build/update/uninstall chain looks for the app it just built under hardcoded stock names —
Hermes.app,Hermes.exe,hermes— instead of the name electron-builder was configured to write. Any build whosebuild.productName/build.executableNamediffer from the stock values packs successfully and is then reported as not built:hermes desktop --build-onlytreats the rebuild as having produced no launchable app, exits nonzero, and the updater rolls back to the previous version.The two halves of the same feature disagree about what the app is called:
build.productName/build.executableName(apps/desktop/package.json:172-186).apps/desktop/scripts/before-pack.mjs:121computes the exe name as`${context.packager?.appInfo?.productFilename || 'Hermes'}.exe`before preserving the rollback tree.One changed string in
package.jsontherefore turns one-click update into a permanent rollback loop, and it surfaces as "the update did not apply", which points nowhere nearpackage.json. Who hits it: anyone testing a renamed build locally, distro packagers who set a different product name, OEM/white-label builds, and downstream redistributions.This is a field report, not a hypothetical: it was hit on a renamed downstream build, where the first one-click update run aborted exactly here. Credit where it is due — the rollback path did its job and kept the machine on the previous working build with an honest dialog, which is why this is a fixable annoyance and not a brick.
Reproduction on a clean checkout:
scripts/install.shreports the same non-event asDesktop build completed but no app was found under <dir>/release/, andinstall.ps1asDesktop build completed but no Hermes.exe was found …— three different messages for one cause: the app is there, under a different name.This PR does not rename anything or add a branding surface. It reads the two keys electron-builder already consumes, at the places that look for electron-builder's output. Stock builds are unaffected byte-for-byte: the stock names remain the fallback whenever
package.jsonis missing, unreadable, or unparseable.Related Issue
No existing issue covers this. Searched open and closed issues and PRs for
productName,Hermes.exe, and packaged-app resolution first — the nearby hits (#53040 before-pack cleanup, #37762 universal DMG) are different failures in the same area, and neither touches name resolution. Happy to file a tracker entry if you would rather have one to link.Type of Change
Changes Made
hermes_cli/main.py_desktop_packaged_executable()_desktop_product_names()helper; all three platform branches build their candidate list from ithermes_cli/main.py_ensure_desktop_exe_launchable(),_desktop_macos_relaunchable_fixup()hermes_cli/gui_uninstall.pydesktop_userdata_dir(),packaged_gui_app_paths()userDatadirectory derive from the same product namescripts/desktop-update/posix.shmac_swap()scripts/install.shinstall_desktop()scripts/install.ps1Install-Desktopbuild.executableNamevia a newGet-DesktopExecutableNameTwo derivation mechanisms, chosen per language, both reading the same source of truth:
build.executableName/build.productNamestraight fromapps/desktop/package.json— the keys electron-builder itself consumes. (ConvertFrom-Jsonships in Windows PowerShell 5.1, so the installer stays 5.1-safe.)install.shandposix.share POSIX shell and cannot assume a JSON parser, so they read the same keys throughnode -p. node is guaranteed present on any path that could have produced a packed tree — that tree isnpm run pack's output. Both fall back to the stock names when node is unreachable.Two details worth a reviewer's eye:
release/mac*), and the product name is joined as a literal path segment, so a product name containing glob metacharacters cannot widen which trees are searched. Selection below the changed hunk (win32 arch preference, newest-mtime tiebreak) is untouched.executableName, its lowercase form, then the product name and its lowercase form — preserving the existinghermes/Hermesdouble-probe behavior for the stock pair rather than narrowing it.apps/desktop/electron/desktop-uninstall.tsneeds no change: it already resolves the bundle from the running executable's own path (resolveRemovableAppPath), which is name-independent.How to Test
scripts/run_tests.sh tests/hermes_cli/ -q— every existing desktop-update and uninstall test passes unedited.test_stock_names_still_resolve_without_a_build_configpins the no-package.jsonfallback that every existing install depends on.build.productName/build.executableNametoAurorainapps/desktop/package.json,cd apps/desktop && npm run pack, thenhermes desktop --build-only. Before: exits nonzero, "not built". After: exits 0 and reports the packed bundle.npm run pack, then run install.ps1's desktop stage — before it throws "no Hermes.exe was found", after it findsAurora.exeand writes the shortcuts against it.npm run pack, thenbash scripts/install.sh --include-desktop— before, no app is found; after, it resolvesrelease/linux-unpacked/aurora.npm run update:repro:freshfromapps/desktopwith the renamedpackage.json— the bundle swap inposix.shfinds the rebuilt app instead of leaving the old one in place.hermes uninstall --gui --dry-runlists the renamed bundle and userData directory.New tests follow the existing shape in this area — synthetic release trees under
tmp_path, platform faked withmonkeypatch.setattr(…sys, "platform", …)— so they run on every host with no new marks or fixtures.One filesystem detail, handled rather than papered over:
test_linux_binary_resolves_from_either_spellingpacksrelease/linux-unpacked/auroraand originally asserted that exact string back. On a case-insensitive filesystem (stock macOS APFS, NTFS)Path("Aurora").exists()is true after writingaurora, so the probe legitimately returns the capitalized spelling and a string comparison fails — a property of the host filesystem, not of the resolution under test. The test now compares by file identity (resolved.samefile(exe), plus the parent directory and a case-folded name check), so it is green on both filesystem kinds without touching the production probe order:--basetempon a case-sensitive APFS volume): 12 passed, 4 skippedIt still fails on unfixed code — with
hermes_cli/main.pyreverted tomain, all three renamed-build cases fail onassert resolved is not None, while the stock-name control keeps passing.Checklist
Code
fix(desktop): …)pytest tests/ -qand all tests pass — I ran the full suite viascripts/run_tests.shand it is not literally all-green on this machine, so here is the exact accounting instead of a checked box: 2873 files, 31542 passed, 77 failed, 310 skipped. 76 of those 77 are in 21 files this PR does not touch (tests/tools/,tests/plugins/,tests/gateway/, optional-dependency and local-environment failures); re-running exactly those 21 files on a pristine9166530942worktree on the same machine reproduces 76 failed, so they are pre-existing and unrelated. The 77th was the case-insensitive-filesystem comparison described above, fixed in the test since that run — this PR's own files are now green on both case-sensitive and case-insensitive filesystems.bash -n scripts/install.sh,bash -n scripts/desktop-update/posix.sh, and a[Parser]::ParseFileparse ofscripts/install.ps1under pwsh are all clean.python scripts/check-windows-footguns.py --all(what CI runs) → no footguns found, 962 files scanned.Documentation & Housekeeping
docs/, docstrings) — the derivation is documented in the docstrings/comments at each site; no user-facing docs surface changescli-config.yaml.exampleif I added/changed config keys — N/A, no config keys; the keys read are electron-builder's ownCONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/A, no architecture changeinstall.shandinstall.ps1changes were made in lockstepScreenshots / Logs
On this machine's stock (case-insensitive) APFS:
Same two files, same machine, with pytest's temp root on a case-sensitive APFS volume:
The three renamed-build cases against unfixed production code (
hermes_cli/main.pyreverted tomain, tests kept) — the stock-name control still passes, which is what makes the other three meaningful:Notes for reviewers
package.json(a packaged-only install with no checkout, a partially wiped tree) behaves exactly as it does today.gui_uninstallstill falls back to the stock name and will not find the bundle. Closing that means reading the name from the installed app's ownpackage.jsoninside the bundle — a different lookup with its own failure modes. Happy to add it here or in a follow-up, your call.*.app/*-unpacked/*by shape? It needs no config read, but it makes the checker accept whatever is in the tree — including a stale bundle from a previous product name. Deriving from the build config keeps the check exact: the app we were configured to build, or nothing.