Repository navigation
Conversation
velopack 1.2.161 gates VelopackLocator::get_root_dir to non-Linux targets, so native/updater never compiled for Linux. Read the root through a cfg-gated helper: get_appimage_path on Linux (the same RootAppDir value), get_root_dir elsewhere. The locator-to-config mapping moves into locator_config so a unit test covers it with a constructed locator. The C ABI and non-Linux behaviour are unchanged.
Create the custom packages directory before building the locator, so Windows Velopack keeps it instead of falling back to %LOCALAPPDATA%. Test-only; fixes review finding F1.
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
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.
Summary
native/updaternever compiled for Linux: velopack 1.2.161 gatesVelopackLocator::get_root_dirto non-Linux targets (src/locator.rs:216), sosrc/lib.rs:36failed with E0599. CI-1's new Linux job found this (PR #5).root_dirhelper reads the root throughget_appimage_pathon Linux andget_root_direverywhere else. On Linux, velopack fills both fromRootAppDir(the AppImage path), so the value is unchanged.config()intolocator_config(&VelopackLocator)so a unit test can cover it with a constructed locator.config()is unchanged for callers.shifter_updaterC ABI, toCargo.tomlor toCargo.lock. Behaviour on non-Linux targets is unchanged.Area: update-desktop (
native/updater/src/lib.rsonly). Size: 38 added and 3 removed lines (about 16 production, the rest test). Boundary: n/a (single-repository project).Regression
New test
config_uses_locator_rootchecks three things: the config root equals the locator root, the packages directory is<AppImage dir>/.shifter-updateson Linux and<packages>/shifter-verified-v1elsewhere, andconfig()fails closed for an unpackaged binary.cargo fmt --check0;cargo clippy --locked --all-targets -- -D warnings101 andcargo test --locked101, both witherror[E0599]: no method named get_root_dir found for struct VelopackLocatoratsrc/lib.rs:36.Validation (head 86f6fe2)
cargo fmt --check0,cargo clippy --locked --all-targets -- -D warnings0,cargo test --locked2 passed.root_diris compiled out on Linux. It runs in the macOS test but its coverage was not measured.-D warningsand tests (2 passed) all green.get_root_dirthere, as before.git diff --checkclean.Review fixes (head 013e816)
%LOCALAPPDATA%, soconfig_uses_locator_rootwould fail and write into the user profile. The test now creates<tmp>/packagesbefore building the locator. Linux on 013e816 (disposable Ubuntu 24.04 c-4, Rust 1.99.0):cargo fmt --check0,cargo clippy --locked --all-targets -- -D warnings0,cargo test --locked2 passed; cargo-llvm-cov counts the new line. Windows is still not built (no Windows host).Documentation
No user-facing or API documentation change: the C ABI and documented behaviour are unchanged. The Linux root semantics are documented on the new helper.