desktop_page(wallpaper): scan the pack registry, not just backgrounds paths - #24
desktop_page(wallpaper): scan the pack registry, not just backgrounds paths#24perlowja wants to merge 2 commits into
Conversation
… paths The picker enumerated a single level of a fixed backgrounds-path list and kept only entries whose content-type began with image/. Two bugs in that: - /usr/share/backgrounds contains no images directly, only ncz/ and singularity/ subdirectories -- a directory's content-type is inode/directory, so every shipped wallpaper was silently skipped and had never displayed in the picker. - Packs that declare their own directory outside any backgrounds path are unreachable by walking backgrounds at all. The Bing provider caches into /var/cache/ncz-wallpapers/bing, which nothing in the old scan roots would ever reach. Now reads every installed pack's .collection file for its Dir= entry (the current on-disk registry format; docs/WALLPAPER-PACKS.md's move to .pack.json should parse both when it lands, not replace this, or packs from the older deb disappear from the picker on upgrade) and walks each scan root recursively up to a bounded depth, skipping symlinks (both to avoid directory cycles and to avoid double-listing default.jpg, which the rotator repoints at whichever wallpaper is current) and avoiding re-walking overlapping roots.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09d5ecea6e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // wallpaper is current. Its target is enumerated in the same | ||
| // directory, so following it would list one image twice -- | ||
| // once under its own name and once as "default". | ||
| if (info.get_is_symlink()) continue; |
There was a problem hiding this comment.
When a wallpaper package exposes an image through a symlink whose target is elsewhere, this unconditional check removes its only gallery entry. The previous scanner followed the link and accepted the target's image content type, so skipping every symlink—not just the same-directory default.jpg alias described above—regresses these installed wallpapers; restrict the exclusion to known duplicate aliases or deduplicate resolved targets instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. Fixed in 0adbc99 — only elide a symlinked file when its resolved target sits in the same directory being scanned (the actual default.jpg double-listing case); a symlink pointing to a target elsewhere is now followed and listed, matching the previous scanner's behavior of resolving content-type through the link.
…same-dir pointers Codex review on PR singularityos-lab#24 caught a regression: the previous commit skipped every symlinked file to avoid double-listing default.jpg, but that also drops a pack's wallpaper if it ships one as a symlink to a shared asset outside the scanned directory -- the old scanner followed those fine (content-type resolves through the link). Only elide a symlink whose target sits in the same directory being scanned, which is the actual double-listing case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Hn3H7xGPDu6W2ntjotqao
…ntrols First of four plans implementing the wallpaper-pack-browser-ocs spec. Covers spec sections 1 and 5 only. Branches from PR singularityos-lab#24 (fix/wallpaper-picker-pack-registry) since it extends that PR's collection-registry scan. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Hn3H7xGPDu6W2ntjotqao
1 participant
Summary
The wallpaper picker in Desktop settings has never shown any wallpapers — not the shipped NCZ art, not artist packs, not the Bing provider cache. Two bugs:
/usr/share/backgroundsholds no images directly, only subdirectories (ncz/,singularity/). A directory's content-type isinode/directory, so the old one-level scan silently skipped everything./var/cache/ncz-wallpapers/bing), which the old fixed-path scan could never reach at all.What changed
populate_grid()now reads every installed pack's.collectionKeyFile for itsDir=entry (system dirs viaXDG_DATA_DIRS, plus the per-user collections dir) and recursively walks each resulting scan root, bounded to depth 3, skipping symlinks (cycle prevention, and avoids double-listingdefault.jpg, which the rotator repoints at whichever wallpaper is current), deduping overlapping roots.Scope note for review
Dir=values from the per-user collection dir ($XDG_DATA_HOME/ncz-wallpapers/collections) are trusted without a path-prefix check. This mirrors the existing shell-script registry (ncz-wallpaper-collections) that already treats the per-user dir as a legitimate pack-declaration tier — it's not a privilege boundary (same UID reading its own filesystem), and the design intentionally allows a pack'sDirto live anywhere (that's what makes the Bing cache reachable at all). But it does mean a.collectionfile dropped into that user-writable directory can point the picker at an arbitrary readable path and surface whatever images live there, recursively, up to depth 3. Flagging it explicitly rather than silently narrowing the feature — happy to add a path-prefix allowlist if you'd rather constrain per-userDirto known roots.Validation
Built clean in a
debian:forkycontainer (the documented isolation path —libgtk4-layer-shell-devon Debian conflicts with NCZ's patched runtime lib):meson compilesucceeded, all 5 targets linked includingsingularity-desktop. No new warnings introduced; existing deprecation warnings (Gtk.Widget.get_style_context, etc.) are pre-existing and untouched by this diff.🤖 Generated with Claude Code
https://claude.ai/code/session_018Hn3H7xGPDu6W2ntjotqao