velopack - #546
Conversation
|
Pushed 3 commits to this branch implementing the fixes below (bbd1012, 8bc7957, b3da5da): Built + installed the Windows Velopack package locally and exercised the install, file-association, and update flows. Three defects: 1. Distribution metadata not embedded → app reports version 2. 3. Updates never offered — every published release is un-updatable (main issue) Also: the "Check output" step validates only Verified the full path (detect → download → apply → restart) locally by packing a 0.0.3 release and pointing the updater at it; post-update |
|
The 0.0.1 release works on Mac with the same bugs it had on Windows. I imagine the last commits fix it. I will let a local build running and test tomorrow. |
|
@azfoo, could you fix CI too? |
FelipeDefensor
left a comment
There was a problem hiding this comment.
The Mac build worked, after applying the changes I commited. I wasn' t able to run it on Linux. Could you do that after you review my code?
|
Is this also ready for review? |
|
not quite. pending a final ci test. |
verify updates:
todo:
n.b.: failed test run because faked version number exceeds the programmed high version number in test. |
|
Works great in my Mac! Edit: I found the issues below later, but the other paths still work perfectly. |
There was a problem hiding this comment.
Reviewed the branch (base ad2a2a2e) and verified each of the findings below on macOS 15 (Darwin 24.6, arm64) against the installed 99.0.1 build. Two are hard reproductions (the threading crash and the clean-loop failure), one is a live behaviour check (open -a on a .tla), two are code reads.
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| def _on_velopack_install(version: str) -> None: |
There was a problem hiding this comment.
macOS: the .tla association registers, but opening a file from Finder gives a blank document.
The Info.plist patch in deploy.py adds CFBundleDocumentTypes / UTExportedTypeDeclarations, so TiLiA does show up under "Open with". But macOS does not pass the path in argv for a bundled app — LaunchServices sends an odoc Apple Event, which Qt turns into a QFileOpenEvent on the QApplication. Nothing in the codebase handles it: git grep -n "FileOpen" tilia/ returns nothing, boot.py creates a plain QApplication(sys.argv) with no event() override, and the only path into a file is args.file from setup_parser().
Verified live against the installed 99.0.1 build:
open -a /Applications/TiLiA.app /tmp/fileopen-probe.tla
A new session log appears (INITIALISED, blank SliderTimeline created) and the probe path never shows up anywhere in it — the app starts empty.
Needs a QFileOpenEvent handler on the application object that routes into the same "open file" path as the CLI argument (and it has to buffer the event if it arrives before the UI is built, which on a cold launch it does).
There was a problem hiding this comment.
@FelipeDefensor can you test this to confirm?
Replace the sdist-based build with a direct editable-install build. Previously the script created an sdist and extracted it to a temp lib directory; now deploy.py builds directly against the installed package, passing the Nuitka package-config YAML as an absolute path. Also fix macOS app bundle handling: read the binary name from CFBundleExecutable (avoids assumptions about casing) and glob for the .app bundle directory at zip time instead of hardcoding tilia.app. Removes the "build" package from the build dependency group and the now-unnecessary nuitka-package.config.yml entry from package-data (which was only present for the sdist extraction path).
- lifecycle.py: platform.init() handles Velopack install/uninstall hooks and OS file-association wiring (Windows registry, macOS Info.plist, Linux MIME/desktop); guarded by __compiled__ in boot.py - boot.py: call platform.init() before QApplication inside __compiled__ block
- updates.py: background thread checks Velopack (compiled builds) or git fetch (source runs); posts APP_UPDATE_AVAILABLE with manager+info - dialogs/update.py: show_update_dialog() dispatches to Velopack (download + restart) or git (pull + os.execv) path - qtui.py: listens on APP_UPDATE_AVAILABLE, marshals to main thread via QTimer.singleShot before showing dialog - menus.py: "Check for Updates..." item in Help menu - errors.py: VELOPACK_MANIFEST_NOT_FOUND, VELOPACK_UPDATE_FAILED, GIT_PULL_FAILED
- deploy.py: add _build_velopack() — runs vpk pack after Nuitka, downloads previous release nupkg for delta generation, writes vpk-outdir and pack-version to GITHUB_OUTPUT; --noPortable applied only for non-Linux (vpk on Linux does not support the flag); standalone mode per-platform (app on macOS, standalone on Windows/Linux); _semver_version() validates strict MAJOR.MINOR.PATCH (Velopack runtime requires exactly 3 parts; 4-part versions cause a semver parse error at install time); _patch_macos_plist() adds .tla file associations and returns CFBundleExecutable; Nuitka entry point changed from tilia/__main__.py to tilia/ (package mode) - build.yml: installs .NET 10 via brew with continue-on-error (setup-dotnet fails on Intel macOS runners); installs vpk CLI via dotnet tool; test steps run Setup.pkg/Setup.exe installers; release asset check verifies one full.nupkg per channel rather than a fixed total count; GH_TOKEN added to build env for gh release download - pyproject.toml: velopack in build group; remove onefile-tempdir-spec and mode="app" (now set per-platform in deploy.py)
Compiled builds resolved version to 0.0.0 because Nuitka dropped the bundled distribution metadata. Two causes, both fixed: (1) find_namespace_packages swept a stray top-level htmlcov/ into tilia.egg-info top_level.txt, so Nuitka treated htmlcov as the metadata owner and skipped embedding -- constrained discovery with include = [tilia*]; (2) added --include-distribution-metadata so the embed uses reason 'user requested', which bypasses Nuitka's hasDoneModule gate and fails loudly instead of silently degrading.
Velopack Setup/Update pass --veloapp-install/-updated/-obsolete/-uninstall/-firstrun, not --velopack-*. The misspelled prefix meant the file-association hook never fired and .tla files were never registered on install. Fixed the prefix in the dispatch table and the _hook_mode gate.
Published releases were un-updatable: clients found the GitHub release but check_for_updates() returned None because no releases.<channel>.json was present. vpk pack emits releases.<channel>.json / assets.<channel>.json / RELEASES-<channel>, but the upload-artifact glob matched only *.nupkg / *-Setup.* / *.AppImage, dropping the manifests before the deploy job's release upload (files: build/velopack/**). Without releases.<channel>.json Velopack's GitHub source enumerates zero releases, so no update is ever offered. Add the manifest globs. (The Check output step stayed green because it validates only full.nupkg + user-downloads, not the update manifest.)
On macOS the locator was Windows-shaped (Update.exe, <root>/packages, wrong root), so UpdateManager raised "not properly installed" and the Check for Updates dialog showed the misleading "TiLiA is running from source" message. Branch _make_locator by OS: use Contents/MacOS/UpdateMac and stage packages under ~/Library/Caches, since the .pkg-installed bundle is read-only. Also rework _run_check error handling. Previously a failed UpdateManager construction was swallowed into "running from source", and a failed check_for_updates masqueraded as "up to date". Split these into distinct, surfaced messages (build / install / network), tighten the bare `except Exception` to RuntimeError (everything Velopack raises), and add a thread-boundary backstop so an unexpected error can never kill the daemon thread silently.
Cover _make_locator OS-specific paths (UpdateMac vs Update.exe, the Library/Caches packages dir, non-fatal mkdir on read-only homes) and _run_check routing of every outcome: git fallback in dev, import/locator/ construct/network failures each producing their own surfaced message, up-to-date posting, update-available, silent-check gating, and the daemon-thread backstop. The velopack extension is a build-only dependency, so it is faked to keep these runnable in the source test environment.
"Check output" step never caught the missing-manifest bug (updates undiscoverable because releases.<channel>.json was never uploaded) because it only validated full.nupkg counts and download counts. Assert per-channel manifest presence too so this failure mode goes red instead of green.
validate_tla_data() only checked that top-level keys existed, so a
timelines dict with malformed values (e.g. {"id": 404}) sailed through
validation. migrate()'s _to_0_7_0_timeline_kind step then called
.get("kind", "") on that value unconditionally and crashed with
AttributeError instead of the file being rejected as invalid.
This migration targets "0.7.0" and only runs when app_version >=
0.7.0, so on dev's own real version (0.6.4) it's dormant and the bug
never surfaces. It ran here because this branch's version is
temporarily bumped past 0.7.0 for Velopack update-flow testing, which
tripped the version gate and exposed a pre-existing latent bug in
dev's migration system.
Extend validate_tla_data() to check that "timelines" is a dict and
that every value in it is itself a dict, so malformed data is caught
and reported as OPEN_FILE_INVALID_TLA before migrate() ever runs.
tilia/lifecycle.py had no tests, and two of the bugs that reached a published release lived in it: the hook flags matched --velopack-* while Velopack passes --veloapp-*, and uninstall leaves Software\Classes\.tla behind. Both are string-literal contracts that nothing asserted. All platform-specific imports in lifecycle.py are function-local, so the Windows path is testable from any OS by putting a fake in sys.modules["winreg"]. FakeRegistry stores keys flat and, like the real API, refuses to DeleteKey a key that still has subkeys - which is what makes the .tla leak observable. The Linux path needs nothing beyond XDG_DATA_HOME and a stubbed shutil.which. test_uninstall_removes_the_tla_key is xfail(strict=True): it asserts the correct behaviour, keeps CI green until the delete order is fixed, and turns into a failure the moment the fix lands and the xfail goes stale. These are boot-time hooks that no user action reaches, so they are tested by direct call rather than through commands.execute.
_report runs on check_for_updates' background thread. Post.DISPLAY_ERROR is dispatched synchronously, so an un-marshaled call there built/exec()'d a QMessageBox off the main thread and crashed the process on macOS (NSInternalInconsistencyException) whenever a non-silent update check failed (offline, missing velopack, bad install, etc.). Route it through QTimer.singleShot the same way on_update_available's success path already does.
DeleteKey refuses to remove a key that still has subkeys. .tla was deleted first, so that call silently failed (caught by except OSError: pass) and the .tla key survived uninstall, still pointing its default value at the now-deleted TiLiA.tla ProgID — a broken-handler error on the next double-click instead of falling back to no association. Only our own value is removed from OpenWithProgids, and the subkey itself only if that leaves it empty, so another app's entry there survives. Removes the xfail on test_uninstall_removes_the_tla_key and adds a coexistence test for other apps' OpenWithProgids entries.
Adds a Test uninstall cleanup step after the existing Windows install/smoke step: runs the real Update.exe uninstall --silent (which invokes our --veloapp-uninstall hook), then asserts the install dir and the .tla / Applications registry keys are gone. Tests the real, already-shipped Velopack mechanism plus the registry delete-order fix from an earlier commit on this branch.
macOS doesn't put the double-clicked/dropped file's path in argv the way Windows and Linux do — LaunchServices sends an odoc Apple Event, which Qt's Cocoa platform plugin turns into a QFileOpenEvent on the application object. Nothing handled it, so opening a .tla via Finder launched a blank TiLiA instead of the file. Adds TiliaApplication (a QApplication subclass overriding event()) and PendingFileOpen, which buffers the path if the event arrives before boot() has built App/UI to hand it to — which, on a cold launch, it does. PendingFileOpen is split out and unit-tested standalone since only one QApplication may exist per process.
Before Velopack, every platform used the versioned out_filename for the Nuitka output filename. When Windows/Linux moved to the plain, stable name (Velopack needs --mainExe to stay constant across updates), macOS was left on the old versioned branch - history shows no deliberate reason, just an oversight. Using the plain project name on mac too (as first tried here) breaks the build: macOS's default filesystem is case-insensitive, and an output filename that is a pure case-variant of the tilia package (e.g. TiLiA) collides with Nuitka's own Contents/MacOS/tilia/ data-files directory - NotADirectoryError partway through the build. Confirmed via an actual CI build (v89.9.8, build A) on all three platforms; Windows and Linux don't hit this (TiLiA.exe has an extension; Linux's filesystem is case-sensitive). Uses <name>-bin on mac instead - still stable across releases (unlike the old versioned name), just not a bare case-variant of the package name.
The Nuitka build crashes at GUI startup - AttributeError: module 'music21.repeat' has no attribute 'RepeatMark' - from tilia.ui.qtui's own import of tilia.parsers.csv.harmony pulling in music21. Confirmed via an actual CI build on both Windows and Linux, and via a local Nuitka 4.2 build. Not a music21 version issue (10.5.0 either way) and not import order (tried forcing an earlier import of music21.repeat first - had no effect). Bisected locally with fast isolated repro compiles instead of full-app CI round trips: - The old regex-based 'remove module tests' anti-bloat patch (global_replacements_re stripping class Test(unittest.TestCase) blocks across every music21 submodule) actively corrupts real, non-test code. Confirmed two different failure modes depending on what else runs first: it silently deletes music21.repeat.RepeatMark, or it cuts through a triple-quoted docstring in music21/test/testRunner.py and produces a flat SyntaxError. Removed - the plain-string 'remove tests' patch alongside it is untouched and safe (exact substring match, no regex). - Separately, nofollow-import-to = ["*.tests", "*.test"] in pyproject.toml (a real, intentional bloat exclusion, kept) collides with three places where music21's own __init__.py unconditionally imports its test submodule at package-init time for reasons unrelated to unit testing: music21.meter, music21.midi, music21.braille. Checked every music21 __init__.py for this exact pattern (module path ending in bare .test or .tests) - these three are the whole remaining list. Patches out just the import line in each; the (only) other reference is a stale __all__ entry, harmless to leave.
os.walk does not follow symlinked directories, but it does list them in dirs, and os.rmdir on a symlink raises NotADirectoryError. This sat before the try block, so deploy.py aborted outright.
_make_locator only ever branched on darwin vs else, and else was really Windows - its own docstring never mentioned Linux, and test_updates.py had no Linux coverage (only macos and windows). On Linux this handed UpdateManager a locator pointing at Update.exe, which does not exist in a Linux build, and RootAppDir/PackagesDir shaped for a current/+packages/ layout that Linux does not have either. Very likely meant in-app update checking silently never worked on Linux at all. Adds a real Linux branch matching Velopack's own LinuxVelopackLocator conventions: RootAppDir from the APPIMAGE env var the AppImage runtime itself sets (the FUSE mount point is a fresh temp dir every launch, useless as a stable identity), UpdateExePath is UpdateNix beside the usr/ tree, PackagesDir is the fixed packId-scoped spot under /var/tmp Velopack uses there, and IsPortable=True. Returns None (same as the missing-manifest case) when APPIMAGE is not set, since that means we are not actually running as a mounted AppImage.
--appimage-extract avoids needing FUSE, but only the real AppImage runtime sets the APPIMAGE env var _make_locator's Linux branch reads (tilia/updates.py) to find the update manifest - extracting and running the raw squashfs-root binary meant that path was never exercised by CI at all, only by unit tests with a mocked environment. Installs libfuse2 (Ubuntu 22.04+ ships fuse3, which the AppImage's own bundled runtime does not use) and runs the .AppImage file directly instead.
Two related additions to tilia/lifecycle.py: 1. _ensure_linux_file_association now calls the new _ensure_stable_appimage_copy first: copies the running AppImage (found via the APPIMAGE env var) to ~/.local/bin/TiLiA.AppImage if not already running from there, and points the desktop entry's Exec= at that stable path instead of sys.executable. Matches how AppImageLauncher-style self-integration normally works, so moving/deleting wherever the user originally downloaded the file to no longer silently breaks the launcher (or, previously, points Exec= at an ephemeral FUSE mount path that stops existing the moment the process exits). 2. A new uninstall() entry point, for the in-app command wired up in the next commit. Registration-only - never touches QSettings, autosaves, logs, or recent files. On macOS, unregisters the bundle from Launch Services (lsregister -u) and tells the user to drag it to Trash - does not attempt pkgutil --forget (needs root) or self-deletion of the running bundle. On Linux, removes the desktop/mime files plus the stable copy. Windows isn't handled here - Velopack's own Add/Remove Programs entry already covers it.
help.uninstall (QtUI.on_uninstall) confirms with the user, then calls the new tilia.lifecycle.uninstall() and shows its result. Not offered on Windows at all - Velopack already registers a proper Add/Remove Programs entry there, and a second, partial in-app path would just be a confusing duplicate (_help_menu_items in tilia/ui/menus.py). On a source run the menu item is still present (menu construction needs every listed command registered), but the handler checks __compiled__ first and explains there's nothing registered to remove instead of silently doing nothing. _help_menu_items is a plain function, not inlined into HelpMenu's class body, specifically so tests can monkeypatch sys.platform and call it directly - the class body only runs once, at import time, with the real platform, so it can't be exercised for both branches after the fact.
_on_velopack_uninstall did nothing on macOS and only unregistered the file association (not the stable AppImage copy) on Linux - it predates uninstall() and was never revisited when that was added. Now runs the same cleanup uninstall() does on all three platforms. Nothing in the real Linux/macOS distribution flow calls this hook today (Linux is an unpackaged portable AppImage, macOS's .pkg has no uninstaller to invoke it) - but if a future packaging change ever does, this does the right thing instead of nothing, and it means CI can exercise mac/Linux uninstall directly via this hook's argv flag, the same way the Windows check already does.
Mirrors the Windows uninstall-cleanup step, now that there's something real to test (tilia.lifecycle.uninstall(), unified with the --veloapp-uninstall hook in the previous commit): - mac: invokes --veloapp-uninstall on the installed binary, asserts the bundle is unregistered from Launch Services (lsregister -dump), then removes the bundle itself (uninstall() only unregisters - deleting it is left to the user, the mac norm - so this finishes that step to confirm the OS resources are actually reclaimed end to end). - Linux: invokes --veloapp-uninstall on the real .AppImage, asserts the desktop/mime files and the stable AppImage copy created by the earlier GUI-launch step are all gone.
GIT_BRANCH is what source (non-compiled) installs poll via git fetch for the in-app update check.
|
Several force pushes to clean up [temp] commits. Latest builds: |
Summary
.nupkgpackages for incremental updates.tilia/lifecycle.py) for install/uninstall and OS file-association registration (.tla files).Changes
scripts/deploy.py— removed sdist-based build path: previously the script created an sdist, extracted it to a temp lib directory, and built from there; now builds directly from the editable install, dropping thebuildpackage from the build dependency group. Added_build_velopack(): runsvpk packafter Nuitka, downloads the previous release nupkg fordelta generation, writes
vpk-outdirandpack-versionto GITHUB_OUTPUT;--noPortableon macOS/Windows only (not supported on Linux); strict semver validation;_patch_macos_plist()adds.tlaassociations and returns CFBundleExecutable; Nuitka entry point changed fromtilia/__main__.pytotilia/(package mode)..github/workflows/build.yml— .NET 10 via brew on macOS (setup-dotnetfails on Intel runners);vpkCLI viadotnet tool; CI tests run the actual Setup installers instead of extracting zip archives; upload path filters bypack-versionto exclude old nupkgs from delta generation; release check verifies onefull.nupkgper channel rather than a fixed count; GH_TOKEN added to build env forgh release download.tilia/lifecycle.py(new) — handles Velopack install/uninstall hooks and OS file-association wiring (Windows registry, macOSInfo.plist, Linux MIME/desktop). Called fromboot.pyinside__compiled__guard.tilia/updates.py(new) — background thread polls for updates via Velopack on compiled builds orgit fetchon source runs; postsAPP_UPDATE_AVAILABLEwith manager + info payload.tilia/ui/dialogs/update.py(new) — update dialog dispatches to Velopack (download + restart) or git (pull +os.execv) path.To verify