fix: extend search paths for app version on Linux systems#3399
Closed
marcnause wants to merge 2 commits into
Closed
fix: extend search paths for app version on Linux systems#3399marcnause wants to merge 2 commits into
marcnause wants to merge 2 commits into
Conversation
Contributor
Reviewer's GuideExtends the Linux app version resolution logic by adding a system-wide shared directory search path and adds a test to cover the new default package path behavior. Flow diagram for extended Linux app version search pathsflowchart TD
start[_loadLinuxBundleVersion]
start --> p1["Check ${executableParent.path}/data/flutter_assets/version.json"]
p1 -->|not found| p2["Check ${executableParent.parent.path}/share/pslab/flutter_assets/version.json"]
p2 -->|not found| p3["Check /usr/share/pslab/flutter_assets/version.json"]
p1 -->|found| success["Return version from candidatePaths"]
p2 -->|found| success
p3 -->|found| success
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The newly added absolute path (
/usr/share/pslab/flutter_assets/version.json) is hard-coded; consider moving it to a constant or configuration so it can be managed centrally and adjusted per distro/environment. - If multiple installation layouts are possible, you may want to derive the extra search path from the executable location or XDG conventions rather than assuming
/usr/share/pslab, to avoid failures on nonstandard installs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The newly added absolute path (`/usr/share/pslab/flutter_assets/version.json`) is hard-coded; consider moving it to a constant or configuration so it can be managed centrally and adjusted per distro/environment.
- If multiple installation layouts are possible, you may want to derive the extra search path from the executable location or XDG conventions rather than assuming `/usr/share/pslab`, to avoid failures on nonstandard installs.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
marcnause
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request fixes incorrect “fallback” app version display on Linux (notably Debian .deb installs) by extending the Linux-side lookup for the bundled version.json metadata when package_info_plus doesn’t provide a version.
Changes:
- Added an additional system-wide lookup path for
version.jsonon Linux (/usr/share/pslab/flutter_assets/version.json).
Contributor
Build StatusBuild successful. APKs to test: https://github.com/fossasia/pslab-app/actions/runs/28744501713/artifacts/8092928507. ScreenshotsAndroid Screenshots
iPhone Screenshots
iPad Screenshots
|
||||||||||||||||||||||||
-1_instruments_screen.png?raw=true)
-2_nav_drawer.png?raw=true)
-3_accelerometer.png?raw=true)
-4_power_source.png?raw=true)
-5_multimeter.png?raw=true)
-6_wave_generator.png?raw=true)
-7_oscilloscope.png?raw=true)
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The newly added
/usr/share/pslab/flutter_assets/version.jsonpath is hard-coded; consider building it usingPlatform.pathSeparatoror a shared constant to keep path handling consistent and avoid platform-specific literals. - In the new Linux version resolution test, the executable and version file paths are manually concatenated as string literals; refactor to reuse the same path construction logic as the production code to reduce duplication and keep the test aligned with real path handling.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The newly added `/usr/share/pslab/flutter_assets/version.json` path is hard-coded; consider building it using `Platform.pathSeparator` or a shared constant to keep path handling consistent and avoid platform-specific literals.
- In the new Linux version resolution test, the executable and version file paths are manually concatenated as string literals; refactor to reuse the same path construction logic as the production code to reduce duplication and keep the test aligned with real path handling.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+60
to
+73
| test('uses version in default path from package', () async { | ||
| final executablePath = | ||
| '${Platform.pathSeparator}usr${Platform.pathSeparator}bin${Platform.pathSeparator}' | ||
| 'pslab'; | ||
| final versionFile = File( | ||
| '${Platform.pathSeparator}usr${Platform.pathSeparator}bin${Platform.pathSeparator}' | ||
| 'data${Platform.pathSeparator}flutter_assets${Platform.pathSeparator}' | ||
| 'version.json', | ||
| ); | ||
|
|
||
| IOOverrides.runZoned(() async { | ||
| await versionFile.create(recursive: true); | ||
| await versionFile.writeAsString('{"version":"6.5.3"}'); | ||
|
|
Comment on lines
57
to
61
| final candidatePaths = <String>[ | ||
| '${executableParent.path}/data/flutter_assets/version.json', | ||
| '${executableParent.parent.path}/share/pslab/flutter_assets/version.json', | ||
| '/usr/share/pslab/flutter_assets/version.json', | ||
| ]; |
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.
Fixes #3398
Changes
Screenshots / Recordings
N/A
Checklist:
constants.dartor localization files instead of hard-coded values.dart formator the IDE formatter.flutter analyzeand tests run influtter test.Summary by Sourcery
Extend Linux app version resolution to search an additional shared installation path and cover it with tests.
Bug Fixes:
Tests: