Breadway/breadpaper
1
0
Fork 0
Code Issues Pull requests 1 Projects Releases Packages Wiki Activity Actions

Stop shelling out to pywal; use bread-theme's native extractor #6

Merged
Breadway merged 7 commits from feature/drop-pywal into main 2026-09-29 13:12:52 +08:00
Conversation 9 Commits 7 Files changed 14 +1276 -92
Breadway commented 2026-09-26 12:57:13 +08:00
Owner
Copy link

Removes breadpaper's dependency on wal (python-pywal):

  • Deletes src/pywal.rs and the generate_palette wrapper.
  • set() extracts the palette once via bread_theme::palette_from_image, writes it to bread-theme's global cache with write_global_palette, then reuses that same palette for every per-output file (previously it ran wal -i, then re-read ~/.cache/wal back out via load_palette()).
  • sync_focused_global() now writes the global palette from the per-output palette its caller already extracted, instead of re-running extraction.
  • get() reads the last wallpaper from current.json (focused output, then the "*" entry, then any persisted output, in that order; documented on the function) instead of ~/.cache/wal/wal.
  • Drops python-pywal from bakery.toml system_deps, and rewords README.md, EVENTS.md, and the clap help text.

Dependency note: bread-theme is pointed at the feature/native-palette branch of bread-ecosystem (which carries the new palette_from_image / write_global_palette API) instead of a tag. This must be switched to the real release tag before merge, and Cargo.lock needs a refresh once that branch is pushed/released; it's left resolving to the previous v0.7.4 lock entry for now since the branch isn't fetchable from a clean clone yet.

Verified: cargo build --release, cargo test --release (28 passed), cargo clippy --all-targets -- -D warnings, cargo fmt --check, all clean. Since feature/native-palette isn't pushed yet, these were run with a temporary, uncommitted local path override to the in-progress bread-theme worktree, then reverted; the committed Cargo.toml/Cargo.lock state was not built against that override.

Closes #5

Removes breadpaper's dependency on `wal` (python-pywal): - Deletes `src/pywal.rs` and the `generate_palette` wrapper. - `set()` extracts the palette once via `bread_theme::palette_from_image`, writes it to bread-theme's global cache with `write_global_palette`, then reuses that same palette for every per-output file (previously it ran `wal -i`, then re-read `~/.cache/wal` back out via `load_palette()`). - `sync_focused_global()` now writes the global palette from the per-output palette its caller already extracted, instead of re-running extraction. - `get()` reads the last wallpaper from `current.json` (focused output, then the `"*"` entry, then any persisted output, in that order; documented on the function) instead of `~/.cache/wal/wal`. - Drops `python-pywal` from `bakery.toml` `system_deps`, and rewords `README.md`, `EVENTS.md`, and the clap help text. Dependency note: `bread-theme` is pointed at the `feature/native-palette` branch of bread-ecosystem (which carries the new `palette_from_image` / `write_global_palette` API) instead of a tag. This must be switched to the real release tag before merge, and `Cargo.lock` needs a refresh once that branch is pushed/released; it's left resolving to the previous `v0.7.4` lock entry for now since the branch isn't fetchable from a clean clone yet. Verified: `cargo build --release`, `cargo test --release` (28 passed), `cargo clippy --all-targets -- -D warnings`, `cargo fmt --check`, all clean. Since `feature/native-palette` isn't pushed yet, these were run with a temporary, uncommitted local path override to the in-progress bread-theme worktree, then reverted; the committed `Cargo.toml`/`Cargo.lock` state was not built against that override. Closes #5
fix: extract palette natively via bread-theme instead of shelling to pywal
Some checks failed
check / check (push) Failing after 2s
Details
872596bea9 
set() now extracts the palette once with bread_theme::palette_from_image
and writes it through write_global_palette, instead of running wal -i and
re-reading its cache. sync_focused_global() reuses the palette its caller
already extracted rather than re-running extraction. get() reads the last
wallpaper from current.json (focused output, then the "*" entry, then any
persisted output) instead of ~/.cache/wal/wal.

Deletes src/pywal.rs, drops python-pywal from bakery.toml system_deps, and
rewords README/EVENTS.md/clap help text accordingly.

Points the bread-theme dependency at the feature/native-palette branch,
which carries the new palette_from_image/write_global_palette API; this
must move to the real release tag before merge, and Cargo.lock needs a
refresh once that branch exists (left pointing at the prior v0.7.4 lock
entry for now since the branch isn't fetchable yet).
Breadway commented 2026-09-26 17:51:13 +08:00
Author
Owner
Copy link

Review: changes requested (not merging: depends on unreleased bread-theme)

CI is red: check / check (push) fails on 872596b. I reproduced it in a clean clone. cargo metadata --locked fails with "cannot update the lock file" because the committed Cargo.lock still pins bread-theme at the old tag, while Cargo.toml points at branch = "feature/native-palette". Without --locked, the clean clone builds, so the bread-theme bump itself compiles here (breadpaper has no stylesheet() call sites to update).

Change requests

  • Re-point and relock. Once bread-ecosystem#36 is reworked and released, change Cargo.toml:15 to the release tag = "vX.Y.Z" and commit the regenerated Cargo.lock from a build without any local [patch] override. CI must be green from a clean clone.
  • src/lib.rs:36-44 (set()) regenerates the shared stylesheet through the external bread-theme reload binary. That binary re-reads the palette from the cache with its own copy of load_palette. If the installed bread-theme CLI is older than the library breadpaper links, it reads ~/.cache/wal/colors.json and not the palette.json just written, and the shared sheet goes out of sync with the per-output files. bakery bread_deps has no version floor to prevent that. Since the palette is already in hand, write the shared sheet in-process with theme::write_shared_from(&palette), the same way sync_focused_global (src/lib.rs:163-167) does, and drop the reload_theme() call. The atomic rewrite of theme.css is what wakes the running apps. That also makes the global and focused paths consistent.
  • Format coverage, tracked upstream. IMAGE_EXTENSIONS (src/lib.rs:24) accepts webp, gif and bmp, but the current bread-theme branch only decodes png and jpeg. set() on a webp wallpaper applies it via awww, then fails at palette_from_image and leaves a stale palette. The fix belongs in bread-theme (#36). Add a test here once it lands, or reject formats bread-theme cannot decode in validate() before apply_wallpaper, so a failure does not leave half-applied state.
  • Stale comment at src/lib.rs:469: "never reaches awww/wal".

Acceptance criteria as I read them (PAPER-5)

  1. src/pywal.rs deleted, generate_palette gone. Met.
  2. set() extracts once, writes the global cache and per-output files. Met (src/lib.rs:41-43, :78-82), apart from the stylesheet issue above.
  3. sync_focused_global writes the global palette from the palette it already has, with no second extraction. Met (src/lib.rs:163-167).
  4. get() reads current.json. Met (src/lib.rs:325-343). The focused output wins, then "*", then the first entry in BTreeMap order. The semantics change is reasonable and documented in the README. There is no unit test for the precedence; a test on a Current fixture would be cheap (the Hyprland lookup can stay untested).
  5. bread-theme bumped to a tag. Not yet: it points at a branch, which is expected before the release.
  6. python-pywal removed from bakery.toml, and README, EVENTS.md and the bakery description updated. Met.

Rework happens on the existing branch feature/drop-pywal, and this PR stays open. Do not open a second PR for PAPER-5.

## Review: changes requested (not merging: depends on unreleased bread-theme) CI is red: `check / check (push)` fails on 872596b. I reproduced it in a clean clone. `cargo metadata --locked` fails with "cannot update the lock file" because the committed Cargo.lock still pins bread-theme at the old tag, while Cargo.toml points at `branch = "feature/native-palette"`. Without `--locked`, the clean clone builds, so the bread-theme bump itself compiles here (breadpaper has no `stylesheet()` call sites to update). ### Change requests - [ ] **Re-point and relock.** Once bread-ecosystem#36 is reworked and released, change `Cargo.toml:15` to the release `tag = "vX.Y.Z"` and commit the regenerated `Cargo.lock` from a build without any local `[patch]` override. CI must be green from a clean clone. - [ ] **`src/lib.rs:36-44` (`set()`) regenerates the shared stylesheet through the external `bread-theme reload` binary.** That binary re-reads the palette from the cache with its own copy of `load_palette`. If the installed bread-theme CLI is older than the library breadpaper links, it reads `~/.cache/wal/colors.json` and not the `palette.json` just written, and the shared sheet goes out of sync with the per-output files. bakery `bread_deps` has no version floor to prevent that. Since the palette is already in hand, write the shared sheet in-process with `theme::write_shared_from(&palette)`, the same way `sync_focused_global` (`src/lib.rs:163-167`) does, and drop the `reload_theme()` call. The atomic rewrite of theme.css is what wakes the running apps. That also makes the global and focused paths consistent. - [ ] **Format coverage, tracked upstream.** `IMAGE_EXTENSIONS` (`src/lib.rs:24`) accepts webp, gif and bmp, but the current bread-theme branch only decodes png and jpeg. `set()` on a webp wallpaper applies it via awww, then fails at `palette_from_image` and leaves a stale palette. The fix belongs in bread-theme (#36). Add a test here once it lands, or reject formats bread-theme cannot decode in `validate()` before `apply_wallpaper`, so a failure does not leave half-applied state. - [ ] Stale comment at `src/lib.rs:469`: "never reaches awww/wal". ### Acceptance criteria as I read them (PAPER-5) 1. `src/pywal.rs` deleted, `generate_palette` gone. Met. 2. `set()` extracts once, writes the global cache and per-output files. Met (`src/lib.rs:41-43`, `:78-82`), apart from the stylesheet issue above. 3. `sync_focused_global` writes the global palette from the palette it already has, with no second extraction. Met (`src/lib.rs:163-167`). 4. `get()` reads current.json. Met (`src/lib.rs:325-343`). The focused output wins, then `"*"`, then the first entry in BTreeMap order. The semantics change is reasonable and documented in the README. There is no unit test for the precedence; a test on a `Current` fixture would be cheap (the Hyprland lookup can stay untested). 5. bread-theme bumped to a tag. Not yet: it points at a branch, which is expected before the release. 6. `python-pywal` removed from bakery.toml, and README, EVENTS.md and the bakery description updated. Met. Rework happens on the existing branch `feature/drop-pywal`, and this PR stays open. Do not open a second PR for PAPER-5.
fix: address review on native palette extraction
All checks were successful
check / check (push) Successful in 2m52s
Details
523e8b60f9 
Write the shared stylesheet in-process (theme::write_shared_from) instead
of shelling out to bread-theme reload, so set() no longer depends on the
bread-theme CLI being installed and cannot pick up a stale cache from an
older installed copy.

Extract the palette before applying the wallpaper in both set() and
set_on(), so a decode failure leaves the old wallpaper and theme intact
instead of half-applying the change.

Split get()'s output precedence into resolve_current() and add tests for
it (focused output, then "*", then first persisted entry), stubbing the
focused-output lookup instead of depending on a live Hyprland socket.

Fix a stale comment referencing awww/wal, and reword remaining
bread-theme reload references in README.md and EVENTS.md.

Refreshes Cargo.lock now that bread-theme's feature/native-palette branch
is pushed; still points at that branch, not a release tag, until the
bread-theme release lands.
Breadway commented 2026-09-26 22:33:40 +08:00
Author
Owner
Copy link

Pushed a follow-up commit addressing review:

  1. set() now writes the shared stylesheet in-process via theme::write_shared_from(&palette), no more shelling out to bread-theme reload. Confirmed nothing else relies on the external reload: it only re-rendered the shared sheet from the global cache, which set() already keeps current via write_global_palette; per-output files are written separately in-process either way. Removed the now-dead theme::reload() / reload_theme() wrapper.
  2. Both set() and set_on() now extract the palette before applying the wallpaper, so a decode failure leaves the old wallpaper and theme intact.
  3. Added resolve_current() (get()'s precedence logic split out) with tests covering focused output, "*" fallback, and first-persisted-entry fallback, stubbing the focused-output name instead of depending on a live Hyprland socket.
  4. Fixed the stale awww/wal comment at the old lib.rs:469, and reworded the remaining "bread-theme reload" mentions in README.md/EVENTS.md to match the in-process behavior.

Also refreshed Cargo.lock now that feature/native-palette is pushed (cargo update -p bread-theme), verified with cargo build --locked in a fresh clone (not a local path override). Still points at the branch, not a release tag; will need another lock refresh once bread-theme cuts its release.

Verified: cargo build --release, cargo test --release (36 passed), cargo clippy --all-targets -- -D warnings, cargo fmt --check, all clean.

Pushed a follow-up commit addressing review: 1. `set()` now writes the shared stylesheet in-process via `theme::write_shared_from(&palette)`, no more shelling out to `bread-theme reload`. Confirmed nothing else relies on the external reload: it only re-rendered the shared sheet from the global cache, which `set()` already keeps current via `write_global_palette`; per-output files are written separately in-process either way. Removed the now-dead `theme::reload()` / `reload_theme()` wrapper. 2. Both `set()` and `set_on()` now extract the palette before applying the wallpaper, so a decode failure leaves the old wallpaper and theme intact. 3. Added `resolve_current()` (get()'s precedence logic split out) with tests covering focused output, "*" fallback, and first-persisted-entry fallback, stubbing the focused-output name instead of depending on a live Hyprland socket. 4. Fixed the stale awww/wal comment at the old lib.rs:469, and reworded the remaining "bread-theme reload" mentions in README.md/EVENTS.md to match the in-process behavior. Also refreshed Cargo.lock now that feature/native-palette is pushed (`cargo update -p bread-theme`), verified with `cargo build --locked` in a fresh clone (not a local path override). Still points at the branch, not a release tag; will need another lock refresh once bread-theme cuts its release. Verified: cargo build --release, cargo test --release (36 passed), cargo clippy --all-targets -- -D warnings, cargo fmt --check, all clean.
Breadway commented 2026-09-26 22:33:52 +08:00
Author
Owner
Copy link

Correction: the test count above is wrong, it's 32 passed (28 existing plus the 4 new resolve_current tests), not 36.

Correction: the test count above is wrong, it's 32 passed (28 existing plus the 4 new resolve_current tests), not 36.
chore: refresh Cargo.lock to bread-theme's latest commit
All checks were successful
check / check (push) Successful in 28s
Details
96526b9a39 
feature/native-palette received new commits since the last lock
refresh; re-pin to the current branch head.
Breadway commented 2026-09-27 08:23:25 +08:00
Author
Owner
Copy link

Refreshed Cargo.lock again to bread-theme's latest feature/native-palette commit (3758f0c), since the branch got new commits after my previous refresh. Re-verified in a fresh clone: cargo build --release, cargo test --release (32 passed), cargo clippy --all-targets -- -D warnings, cargo fmt --check, all clean.

Refreshed Cargo.lock again to bread-theme's latest feature/native-palette commit (3758f0c), since the branch got new commits after my previous refresh. Re-verified in a fresh clone: cargo build --release, cargo test --release (32 passed), cargo clippy --all-targets -- -D warnings, cargo fmt --check, all clean.
Breadway commented 2026-09-27 08:34:53 +08:00
Author
Owner
Copy link

Second-round review: two small changes, plus the relock (not merging)

CI is green on 96526b9 (run 140, fresh clone building bread-theme at 3758f0c), and the PR is mergeable.

The round-1 items are resolved:

  • set() extracts before apply_wallpaper (src/lib.rs:34-47), and a decode failure now returns before anything changes.
  • The shared sheet is written in-process via theme::write_shared_from (src/lib.rs:57). The bread-theme reload shell-out and theme::reload are gone.
  • webp, gif and bmp now decode upstream. I verified all three against real wal in the bread-theme review.
  • get() precedence is split into resolve_current with four behavioural tests: focused entry, "*" fallback, first-entry fallback, and empty is Err.
  • The stale awww/wal comment is fixed.
  • The skel current.json in bos#52 ({"outputs":{"*": ...}}) deserialises into current.rs's Current, and get() returns it on a fresh install.

Change requests

  • src/lib.rs:146-157 (set_on) still half-applies in the other direction. theme::generate_for_output writes palettes/<output>.json and themes/<output>.css before wallpaper::apply_on. If awww fails (daemon not up, bad output name), that monitor recolours to the new palette while it keeps the old wallpaper, and current.json is not updated. A later apply then restores the old wallpaper next to the new palette. Extract with bread_theme::palette_from_image(&path) first, then apply_on, then write the per-output files with the existing write_output_theme(output, &palette). That matches set()'s order: extract, apply, write.
  • README.md:19 still lists bread-theme as a required $PATH binary for set. set no longer execs anything but awww. Move bread-theme to a note: the bakery dependency is still useful for the CLI and other apps, but breadpaper does not need it on PATH.
  • Relock (blocked until the release): once bread-ecosystem#36 is released, change Cargo.toml:15 from branch = "feature/native-palette" to the release tag and commit the regenerated Cargo.lock. Merging #36 deletes that branch, so the current lock will not resolve in a fresh clone after that.

For the user (not blocking this PR's code)

breadpaper is the first consumer that statically links bread-theme's ImageMagick- and IJG-derived extractor, while bakery.toml still declares license_file = "LICENSE" (MIT). See the licensing section of Breadway/bread-ecosystem#36 (comment). The IJG licence wants an "based in part on the work of the Independent JPEG Group" statement in the documentation of executable-only distributions.

Acceptance criteria (PAPER-5), as I read them

  1. No wal subprocess, palette extracted natively. Met (src/pywal.rs deleted).
  2. Extract-before-apply with no half-applied state. Met for set(). set_on() still has the writes-before-apply ordering above.
  3. The global cache and shared sheet are written from the same palette in-process. Met (src/lib.rs:56-57, sync_focused_global).
  4. get() works without ~/.cache/wal/wal. Met, with tests.
  5. python-pywal is dropped from bakery system_deps. Met.

Rework happens on the existing branch feature/drop-pywal, and PR #6 stays open. Do not open a second PR for this key.

## Second-round review: two small changes, plus the relock (not merging) CI is green on 96526b9 (run 140, fresh clone building bread-theme at 3758f0c), and the PR is mergeable. The round-1 items are resolved: - `set()` extracts before `apply_wallpaper` (`src/lib.rs:34-47`), and a decode failure now returns before anything changes. - The shared sheet is written in-process via `theme::write_shared_from` (`src/lib.rs:57`). The `bread-theme reload` shell-out and `theme::reload` are gone. - webp, gif and bmp now decode upstream. I verified all three against real wal in the bread-theme review. - `get()` precedence is split into `resolve_current` with four behavioural tests: focused entry, `"*"` fallback, first-entry fallback, and empty is `Err`. - The stale awww/wal comment is fixed. - The skel `current.json` in bos#52 (`{"outputs":{"*": ...}}`) deserialises into `current.rs`'s `Current`, and `get()` returns it on a fresh install. ### Change requests - [ ] **`src/lib.rs:146-157` (`set_on`) still half-applies in the other direction.** `theme::generate_for_output` writes `palettes/<output>.json` and `themes/<output>.css` *before* `wallpaper::apply_on`. If awww fails (daemon not up, bad output name), that monitor recolours to the new palette while it keeps the old wallpaper, and current.json is not updated. A later `apply` then restores the old wallpaper next to the new palette. Extract with `bread_theme::palette_from_image(&path)` first, then `apply_on`, then write the per-output files with the existing `write_output_theme(output, &palette)`. That matches `set()`'s order: extract, apply, write. - [ ] **`README.md:19` still lists `bread-theme` as a required `$PATH` binary for `set`.** set no longer execs anything but awww. Move bread-theme to a note: the bakery dependency is still useful for the CLI and other apps, but breadpaper does not need it on PATH. - [ ] **Relock (blocked until the release):** once bread-ecosystem#36 is released, change `Cargo.toml:15` from `branch = "feature/native-palette"` to the release tag and commit the regenerated Cargo.lock. Merging #36 deletes that branch, so the current lock will not resolve in a fresh clone after that. ### For the user (not blocking this PR's code) breadpaper is the first consumer that statically links bread-theme's ImageMagick- and IJG-derived extractor, while `bakery.toml` still declares `license_file = "LICENSE"` (MIT). See the licensing section of https://git.breadway.dev/Breadway/bread-ecosystem/pulls/36#issuecomment-1148. The IJG licence wants an "based in part on the work of the Independent JPEG Group" statement in the documentation of executable-only distributions. ### Acceptance criteria (PAPER-5), as I read them 1. No `wal` subprocess, palette extracted natively. Met (`src/pywal.rs` deleted). 2. Extract-before-apply with no half-applied state. Met for `set()`. `set_on()` still has the writes-before-apply ordering above. 3. The global cache and shared sheet are written from the same palette in-process. Met (`src/lib.rs:56-57`, `sync_focused_global`). 4. `get()` works without `~/.cache/wal/wal`. Met, with tests. 5. python-pywal is dropped from bakery `system_deps`. Met. Rework happens on the existing branch `feature/drop-pywal`, and PR #6 stays open. Do not open a second PR for this key.
fix: fix set_on ordering, enable bread-theme extract feature, add third-party notices
All checks were successful
check / check (push) Successful in 20s
Details
f835da5364 
set_on now extracts the palette (bread_theme::palette_from_image, no side
effects) before applying the wallpaper, then writes the per-output theme
files only after apply_on succeeds, so an awww failure can no longer leave
the monitor recoloured without the wallpaper actually changing. Matches
set()'s existing order.

bread-theme's palette_from_image/generate_output moved behind its new
extract feature (bread-ecosystem's licensing split): enable it here, since
extraction is breadpaper's whole purpose. That means this binary now
statically links code ported from ImageMagick and libjpeg-turbo/IJG
(bread-palette-extract); added NOTICE, LICENSE-ImageMagick and
LICENSE-libjpeg-turbo, a `breadpaper notice` subcommand to print them, and
a README licence note. Cargo.toml's license field is now
"MIT AND ImageMagick AND IJG".

README.md: dropped the stale "bread-theme must be on $PATH" line; palette
extraction and the shared stylesheet write are in-process via the
bread-theme library now, not a shelled-out CLI.

Relocked Cargo.lock against bread-theme's latest feature/native-palette
commit (the crate split). Verified: cargo build --locked, cargo test
--locked (32 passed), cargo clippy --all-targets --locked -- -D warnings,
cargo fmt --all -- --check, all clean.
Breadway commented 2026-09-27 21:54:50 +08:00
Author
Owner
Copy link

Round-3 update, addressing the second-round review:

  • set_on now extracts via bread_theme::palette_from_image (no side effects) before wallpaper::apply_on, and writes the per-output theme files only after the wallpaper apply succeeds, matching set()'s order. An awww failure can no longer leave a monitor recoloured without the wallpaper changing.
  • README.md:19: dropped the stale "bread-theme must be on $PATH" line.
  • Enabled bread-theme's extract feature (its licensing split moved palette_from_image/generate_output behind it). Since this binary now statically links ImageMagick/libjpeg-turbo(IJG) code via bread-palette-extract, added NOTICE, LICENSE-ImageMagick, LICENSE-libjpeg-turbo, a breadpaper notice subcommand, and a README licence note. Cargo.toml license is now MIT AND ImageMagick AND IJG.
  • Relocked Cargo.lock against bread-theme's latest feature/native-palette commit (the crate split).

Verified: cargo build --locked, cargo test --locked (32 passed), cargo clippy --all-targets --locked -- -D warnings, cargo fmt --all -- --check, all clean.

Still needed before merge: repin Cargo.toml from the feature/native-palette branch to the release tag once bread-ecosystem's release goes out, and relock again.

Round-3 update, addressing the second-round review: - `set_on` now extracts via `bread_theme::palette_from_image` (no side effects) before `wallpaper::apply_on`, and writes the per-output theme files only after the wallpaper apply succeeds, matching `set()`'s order. An `awww` failure can no longer leave a monitor recoloured without the wallpaper changing. - `README.md:19`: dropped the stale "bread-theme must be on $PATH" line. - Enabled bread-theme's `extract` feature (its licensing split moved `palette_from_image`/`generate_output` behind it). Since this binary now statically links ImageMagick/libjpeg-turbo(IJG) code via `bread-palette-extract`, added `NOTICE`, `LICENSE-ImageMagick`, `LICENSE-libjpeg-turbo`, a `breadpaper notice` subcommand, and a README licence note. Cargo.toml license is now `MIT AND ImageMagick AND IJG`. - Relocked `Cargo.lock` against bread-theme's latest `feature/native-palette` commit (the crate split). Verified: `cargo build --locked`, `cargo test --locked` (32 passed), `cargo clippy --all-targets --locked -- -D warnings`, `cargo fmt --all -- --check`, all clean. Still needed before merge: repin `Cargo.toml` from the `feature/native-palette` branch to the release tag once bread-ecosystem's release goes out, and relock again.
Breadway commented 2026-09-29 08:12:02 +08:00
Author
Owner
Copy link

Third-round review (round-3 commit f835da5): licence packaging gap plus the repin (not merging)

CI is green on f835da5. Forgejo reports the PR as mergeable, but see the pin item below: it cannot land as it stands.

What checks out

  • set_on ordering (src/lib.rs set_on): extract with palette_from_image (no side effects), then wallpaper::apply_on, then write_output_theme, then current.json. An awww failure no longer leaves the monitor recoloured. This matches set(). Good.
  • README no longer claims bread-theme must be on $PATH.
  • features = ["gtk", "extract"] is the only consumer that needs extract, and it is the correct one.
  • Verified in a throwaway clone (since deleted): rebased onto main (clean, no conflicts), bread-theme pin swapped to a tag, and bread-theme patched to a rebased bread-ecosystem#36. lint-pins.sh passes, cargo clippy --all-targets -D warnings is clean, cargo test passes 32, the release build works, breadpaper notice prints the NOTICE and both licences, and cargo tree shows bread-palette-extract linked.
  • The vendored LICENSE-ImageMagick and LICENSE-libjpeg-turbo are byte-identical to bread-palette-extract's copies.

Change requests

  • The shipped package carries only the MIT text. bakery.toml has license_file = "LICENSE", and .forgejo/workflows/release.yml:44 (and the dev/rc equivalents) copy only src/LICENSE, which is MIT-only. NOTICE, LICENSE-ImageMagick and LICENSE-libjpeg-turbo never reach the user except through breadpaper notice. IJG clause 2 requires the accompanying documentation of an executable-only distribution to say "This software is based in part on the work of the Independent JPEG Group". ImageMagick License 4(a) requires giving recipients a copy of the licence, and 4(d) requires the NOTICE text in a NOTICE file, the docs, or a display "wherever such third-party notices normally appear". A hidden subcommand is at best arguable. Do what bread-theme did in #36: make the file that license_file points at a combined text (MIT, then NOTICE, then both licences), either committed or concatenated in dev-release.yml, rc-release.yml and release.yml before the copy. Then the bakery package carries the terms, and breadpaper notice becomes the belt-and-braces copy.
  • Repin to a tag (new convention from bread-ecosystem #39 / breadpaper #7). Main's check.yml now runs lint-pins.sh, which fails on branch = "feature/native-palette" (Cargo.toml:20). Once bread-ecosystem#36 is merged and tagged, set tag = "<that tag>" (keep features = ["gtk", "extract"]), run cargo update -p bread-theme, and commit Cargo.toml and Cargo.lock. This cannot be done earlier: no existing tag has the extract feature, so a committed lockfile cannot resolve against an older tag.

Nit

  • NOTICE and the two licence files are vendored copies of bread-palette-extract's. They are identical today, but nothing keeps them in sync. Optionally, have bread-theme re-export bread_palette_extract::{NOTICE, LICENSE_*} under extract, so breadpaper notice prints the dependency's own text.

Acceptance criteria (my reading)

  1. No pywal subprocess, and extraction goes through bread-theme: met.
  2. set/set_on extract once, never leave a half-applied state, and write the global palette in-process: met.
  3. get() reads current.json: met (resolve_current tests assert the precedence).
  4. python-pywal is out of bakery.toml, and the docs are updated: met.
  5. The bread-theme tag is bumped: not met yet (branch pin, see above).
  6. The distributed binary complies with the licences of the code it links: not met (first change request).

Rework happens on the existing branch feature/drop-pywal, and PR #6 stays open. Do not open a second PR for this key.

## Third-round review (round-3 commit f835da5): licence packaging gap plus the repin (not merging) CI is green on f835da5. Forgejo reports the PR as mergeable, but see the pin item below: it cannot land as it stands. ### What checks out - `set_on` ordering (`src/lib.rs` set_on): extract with `palette_from_image` (no side effects), then `wallpaper::apply_on`, then `write_output_theme`, then current.json. An awww failure no longer leaves the monitor recoloured. This matches `set()`. Good. - README no longer claims `bread-theme` must be on $PATH. - `features = ["gtk", "extract"]` is the only consumer that needs `extract`, and it is the correct one. - Verified in a throwaway clone (since deleted): rebased onto main (clean, no conflicts), bread-theme pin swapped to a tag, and bread-theme patched to a rebased bread-ecosystem#36. `lint-pins.sh` passes, `cargo clippy --all-targets -D warnings` is clean, `cargo test` passes 32, the release build works, `breadpaper notice` prints the NOTICE and both licences, and `cargo tree` shows bread-palette-extract linked. - The vendored `LICENSE-ImageMagick` and `LICENSE-libjpeg-turbo` are byte-identical to bread-palette-extract's copies. ### Change requests - [ ] **The shipped package carries only the MIT text.** `bakery.toml` has `license_file = "LICENSE"`, and `.forgejo/workflows/release.yml:44` (and the dev/rc equivalents) copy only `src/LICENSE`, which is MIT-only. NOTICE, LICENSE-ImageMagick and LICENSE-libjpeg-turbo never reach the user except through `breadpaper notice`. IJG clause 2 requires the accompanying documentation of an executable-only distribution to say "This software is based in part on the work of the Independent JPEG Group". ImageMagick License 4(a) requires giving recipients a copy of the licence, and 4(d) requires the NOTICE text in a NOTICE file, the docs, or a display "wherever such third-party notices normally appear". A hidden subcommand is at best arguable. Do what bread-theme did in #36: make the file that `license_file` points at a combined text (MIT, then NOTICE, then both licences), either committed or concatenated in dev-release.yml, rc-release.yml and release.yml before the copy. Then the bakery package carries the terms, and `breadpaper notice` becomes the belt-and-braces copy. - [ ] **Repin to a tag (new convention from bread-ecosystem #39 / breadpaper #7).** Main's check.yml now runs `lint-pins.sh`, which fails on `branch = "feature/native-palette"` (`Cargo.toml:20`). Once bread-ecosystem#36 is merged and tagged, set `tag = "<that tag>"` (keep `features = ["gtk", "extract"]`), run `cargo update -p bread-theme`, and commit Cargo.toml and Cargo.lock. This cannot be done earlier: no existing tag has the `extract` feature, so a committed lockfile cannot resolve against an older tag. ### Nit - NOTICE and the two licence files are vendored copies of bread-palette-extract's. They are identical today, but nothing keeps them in sync. Optionally, have bread-theme re-export `bread_palette_extract::{NOTICE, LICENSE_*}` under `extract`, so `breadpaper notice` prints the dependency's own text. ### Acceptance criteria (my reading) 1. No pywal subprocess, and extraction goes through bread-theme: met. 2. set/set_on extract once, never leave a half-applied state, and write the global palette in-process: met. 3. `get()` reads current.json: met (resolve_current tests assert the precedence). 4. python-pywal is out of bakery.toml, and the docs are updated: met. 5. The bread-theme tag is bumped: not met yet (branch pin, see above). 6. The distributed binary complies with the licences of the code it links: not met (first change request). Rework happens on the existing branch `feature/drop-pywal`, and PR #6 stays open. Do not open a second PR for this key.
Breadway force-pushed feature/drop-pywal from f835da5364
All checks were successful
check / check (push) Successful in 20s
Details
to 7872f0bc43
Some checks failed
check / check (push) Failing after 1s
Details
2026-09-29 08:32:20 +08:00 Compare
Breadway commented 2026-09-29 08:32:28 +08:00
Author
Owner
Copy link

Round-4 update, addressing the third-round review (#issuecomment-1382):

  • The shipped package now carries the full third-party licence text, not just MIT. bakery.toml's license_file already pointed at LICENSE and the release workflows already copied it verbatim, so the only real gap was the file's content. LICENSE is now combined: breadpaper's own MIT text, then NOTICE, then LICENSE-ImageMagick and LICENSE-libjpeg-turbo in full (including the IJG "based in part on the work of the Independent JPEG Group" statement), mirroring how bread-theme's own LICENSE is assembled. breadpaper notice stays as the belt-and-braces copy.
  • Added tests/license_matches_sources.rs, asserting the combined LICENSE still carries the ImageMagick and libjpeg-turbo texts verbatim plus the IJG sentence, so it can't silently drift from LICENSE-ImageMagick/LICENSE-libjpeg-turbo/NOTICE again.
  • Rebased onto origin/main (clean, no conflicts).

Not done, as instructed: no repin yet, since bread-ecosystem#36 hasn't tagged a release. Cargo.toml still points bread-theme at branch = "feature/native-palette"; once that PR lands and cuts a tag (targeted v0.8.0), this needs tag = "v0.8.0" (keeping features = ["gtk", "extract"]) and a cargo update -p bread-theme + committed Cargo.lock. Flagging again here so it isn't missed at merge time, per the round-3 note.

Verified: cargo build --locked, cargo test --locked (36 passed: the existing 32 plus the 4 new licence tests), cargo clippy --all-targets --locked -- -D warnings, cargo fmt --all -- --check, all clean.

Round-4 update, addressing the third-round review (#issuecomment-1382): - **The shipped package now carries the full third-party licence text, not just MIT.** `bakery.toml`'s `license_file` already pointed at `LICENSE` and the release workflows already copied it verbatim, so the only real gap was the file's content. `LICENSE` is now combined: breadpaper's own MIT text, then `NOTICE`, then `LICENSE-ImageMagick` and `LICENSE-libjpeg-turbo` in full (including the IJG "based in part on the work of the Independent JPEG Group" statement), mirroring how bread-theme's own `LICENSE` is assembled. `breadpaper notice` stays as the belt-and-braces copy. - Added `tests/license_matches_sources.rs`, asserting the combined `LICENSE` still carries the ImageMagick and libjpeg-turbo texts verbatim plus the IJG sentence, so it can't silently drift from `LICENSE-ImageMagick`/`LICENSE-libjpeg-turbo`/`NOTICE` again. - Rebased onto origin/main (clean, no conflicts). **Not done, as instructed:** no repin yet, since bread-ecosystem#36 hasn't tagged a release. `Cargo.toml` still points `bread-theme` at `branch = "feature/native-palette"`; once that PR lands and cuts a tag (targeted v0.8.0), this needs `tag = "v0.8.0"` (keeping `features = ["gtk", "extract"]`) and a `cargo update -p bread-theme` + committed `Cargo.lock`. Flagging again here so it isn't missed at merge time, per the round-3 note. Verified: `cargo build --locked`, `cargo test --locked` (36 passed: the existing 32 plus the 4 new licence tests), `cargo clippy --all-targets --locked -- -D warnings`, `cargo fmt --all -- --check`, all clean.
chore: relock bread-theme against the rebased feature/native-palette tip
Some checks failed
check / check (push) Failing after 1s
Details
6528e81bb2 
bread-ecosystem#36 was rebased onto its own main and force-pushed as part
of its round-4 review fixes, which rewrote the commit this crate's
Cargo.lock had pinned (c29a829) out of existence on the remote. A fresh
clone's `cargo build --locked` could no longer resolve it, which is what
broke this branch's own CI. Cargo.toml is untouched (still branch =
"feature/native-palette", not repinned to a tag, per the standing note),
this is only a `cargo update -p bread-theme` to the branch's current tip
(4c51998), same as every previous round's lock refresh.
Breadway commented 2026-09-29 12:53:19 +08:00
Author
Owner
Copy link

Follow-up: two more things surfaced after rebasing onto origin/main.

  1. Relocked Cargo.lock. Rebasing onto main picked up bread-ecosystem#36's own round-4 rebase, which force-pushed feature/native-palette and rewrote the commit (c29a829) this branch's Cargo.lock had pinned out of existence. A fresh clone's cargo build --locked could no longer resolve it. Ran cargo update -p bread-theme to relock against the branch's current tip (4c51998); Cargo.toml is untouched, still branch = "feature/native-palette". Verified from an actual fresh clone (not this worktree): cargo build --release --locked and cargo test --release --locked (36 passed), both clean.

  2. CI is red, and it's expected, not a regression. The rebase also pulled in bread-ecosystem's ECO-38 dev-track check.yml rewrite that landed on origin/main since the round-3 review: it now runs lint-pins.sh --root src before anything else, which hard-fails on any branch = "..." git dependency pin, bread-theme's included. That's exactly the repin blocker the round-3 review already called out ("this cannot be done earlier: no existing tag has the extract feature"). Reproduced locally: bash lint-pins.sh --root . fails on Cargo.toml's bread-theme pin, in well under a second, matching the CI run's actual failure time. Everything downstream of that gate (clippy, build, test) is unaffected and passes, as shown above. This resolves itself the moment bread-ecosystem#36 tags a release and this branch repins to it, per the outstanding note.

Follow-up: two more things surfaced after rebasing onto origin/main. 1. **Relocked `Cargo.lock`.** Rebasing onto main picked up bread-ecosystem#36's own round-4 rebase, which force-pushed `feature/native-palette` and rewrote the commit (`c29a829`) this branch's `Cargo.lock` had pinned out of existence. A fresh clone's `cargo build --locked` could no longer resolve it. Ran `cargo update -p bread-theme` to relock against the branch's current tip (`4c51998`); `Cargo.toml` is untouched, still `branch = "feature/native-palette"`. Verified from an actual fresh clone (not this worktree): `cargo build --release --locked` and `cargo test --release --locked` (36 passed), both clean. 2. **CI is red, and it's expected, not a regression.** The rebase also pulled in bread-ecosystem's ECO-38 dev-track `check.yml` rewrite that landed on `origin/main` since the round-3 review: it now runs `lint-pins.sh --root src` before anything else, which hard-fails on any `branch = "..."` git dependency pin, `bread-theme`'s included. That's exactly the repin blocker the round-3 review already called out ("this cannot be done earlier: no existing tag has the `extract` feature"). Reproduced locally: `bash lint-pins.sh --root .` fails on `Cargo.toml`'s `bread-theme` pin, in well under a second, matching the CI run's actual failure time. Everything downstream of that gate (clippy, build, test) is unaffected and passes, as shown above. This resolves itself the moment bread-ecosystem#36 tags a release and this branch repins to it, per the outstanding note.
chore: pin bread-theme to v0.8.0
All checks were successful
check / check (push) Successful in 50s
Details
eff7dd5279 
bread-ecosystem v0.8.0 is released with native palette extraction behind
the extract feature, so the temporary feature/native-palette branch pin
becomes a tag pin (lint-pins requires tags), and Cargo.lock is relocked.
Breadway deleted branch feature/drop-pywal 2026-09-29 13:12:52 +08:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Breadway/breadpaper!6
No description provided.