Reword pywal references to bread-theme in docs #6

Merged

Breadway merged 4 commits from fix/pywal-wording into main 2026-09-28 13:10:19 +08:00

Conversation 5 Commits 4 Files changed 20 +494 -194

Breadway commented 2026-09-26 12:52:40 +08:00

Owner

Copy link

Part of ECO-34.

Documentation.md and Examples.md described widget theming as following
"pywal's palette" / a "pywal-derived palette". bread-theme now extracts
palettes natively and pywal is being removed across the ecosystem, so the
wording is updated to match. Comment/doc only, no behaviour change.

Verified: cargo build succeeds. cargo fmt --check reports pre-existing
formatting diffs in xtask/src/main.rs unrelated to this change (no Rust
source touched here), left alone to keep this PR scoped to wording.

Part of ECO-34. Documentation.md and Examples.md described widget theming as following "pywal's palette" / a "pywal-derived palette". bread-theme now extracts palettes natively and pywal is being removed across the ecosystem, so the wording is updated to match. Comment/doc only, no behaviour change. Verified: `cargo build` succeeds. `cargo fmt --check` reports pre-existing formatting diffs in xtask/src/main.rs unrelated to this change (no Rust source touched here), left alone to keep this PR scoped to wording.

Breadway added 1 commit 2026-09-26 12:52:40 +08:00

docs: reword pywal references to bread-theme ... 299b7fc4b4

Documentation.md and Examples.md described widget theming as following
"pywal's palette"; bread-theme now extracts palettes natively, pywal
is being removed from the ecosystem. Comment/doc wording only, no
behaviour change.

Part of ECO-34.

Breadway referenced this pull request from Breadway/bread-ecosystem 2026-09-26 13:08:13 +08:00

Reword pywal mentions across remaining bread apps and docs #34

Breadway commented 2026-09-26 17:52:34 +08:00

Author

Owner

Copy link

Review: blocked on CI (wording itself is fine)

The diff is two Markdown lines (Documentation.md, Examples.md), both accurate. It cannot affect the build. But the check / check (push) run for 299b7fc failed, and I do not merge on red CI.

The failure looks pre-existing, not caused by this PR:

• The previous unrelated branch run (feature/register-breadcloud-app, run 95) also failed.
• main's "dev release / build" is red too.
• cargo fmt --check on this branch reports pre-existing diffs in bread-cli/src/modules_mgmt.rs and bread-cli/src/init.rs. Those files are not touched here. The workflow runs clippy and test through ci/build.sh.

Fix main's check on its own branch, then rebase or re-run this one. It can merge as soon as CI is green.

## Review: blocked on CI (wording itself is fine) The diff is two Markdown lines (Documentation.md, Examples.md), both accurate. It cannot affect the build. But the `check / check (push)` run for 299b7fc failed, and I do not merge on red CI. The failure looks pre-existing, not caused by this PR: - The previous unrelated branch run (`feature/register-breadcloud-app`, run 95) also failed. - `main`'s "dev release / build" is red too. - `cargo fmt --check` on this branch reports pre-existing diffs in `bread-cli/src/modules_mgmt.rs` and `bread-cli/src/init.rs`. Those files are not touched here. The workflow runs clippy and test through `ci/build.sh`. Fix main's check on its own branch, then rebase or re-run this one. It can merge as soon as CI is green.

Breadway added 1 commit 2026-09-26 23:01:53 +08:00

chore: fix pre-existing cargo fmt drift ... e729c118f3

Whitespace only, via cargo fmt --all. This drift already existed on
main; fixing it here so this branch's check workflow can go green.
Verified with cargo build --workspace, cargo clippy --workspace
--all-targets --locked -- -D warnings, and cargo test --workspace
--locked (all pass except two pre-existing module-host spawn test
failures that also fail identically on a clean main checkout in this
sandbox, unrelated to formatting).

Breadway commented 2026-09-27 08:11:42 +08:00

Author

Owner

Copy link

Pushed a cargo fmt --all commit fixing the pre-existing formatting drift (bread-cli, bread-module-host, bread-shared, breadd, xtask) that also exists on origin/main; this PR's own wording change was never the cause. Whitespace only.

Verified: cargo build --workspace --locked, cargo clippy --workspace --all-targets --locked -- -D warnings, cargo test --workspace --locked, all clean, except two pre-existing failures (scoped_module_sees_only_granted_state_read_permission, explicit_empty_permissions_is_scoped_but_not_flagged_ungated) that spawn bread-module-host as a subprocess. Confirmed these fail identically on a fresh clone of origin/main with no changes at all (Permission denied / spawn timeout), so they're a sandbox restriction in this environment, not something introduced or fixable here.

Pushed a `cargo fmt --all` commit fixing the pre-existing formatting drift (bread-cli, bread-module-host, bread-shared, breadd, xtask) that also exists on origin/main; this PR's own wording change was never the cause. Whitespace only. Verified: cargo build --workspace --locked, cargo clippy --workspace --all-targets --locked -- -D warnings, cargo test --workspace --locked, all clean, except two pre-existing failures (scoped_module_sees_only_granted_state_read_permission, explicit_empty_permissions_is_scoped_but_not_flagged_ungated) that spawn bread-module-host as a subprocess. Confirmed these fail identically on a fresh clone of origin/main with no changes at all (Permission denied / spawn timeout), so they're a sandbox restriction in this environment, not something introduced or fixable here.

Breadway referenced this pull request from Breadway/bread-ecosystem 2026-09-27 08:12:27 +08:00

Reword pywal mentions across remaining bread apps and docs #34

Breadway commented 2026-09-27 08:35:39 +08:00

Author

Owner

Copy link

Second-round review: wording is clean, blocked on CI (not merging; bread is also release-sensitive)

The diff is fine:

• The Documentation.md and Examples.md wording is accurate.
• The fmt commit e729c11 is formatting only. I compared every touched file token by token: the only non-whitespace differences are rustfmt reordering mod lines and adding match-arm braces.

The CI failure is not the runner. Run 97 (check / check) runs clippy and then cargo test --workspace --locked to completion. It then fails two tests in breadd/tests/ipc_integration.rs, explicit_empty_permissions_is_scoped_but_not_flagged_ungated and scoped_module_sees_only_granted_state_read_permission, with failed to spawn bread-module-host at bread-module-host: No such file or directory. Cause: breadd/src/module_host.rs:377-387 (resolve_module_host_binary) looks for bread-module-host next to current_exe(). For an integration test that is target/debug/deps/, but the binary lives in target/debug/, so the lookup falls back to a bare PATH lookup, and the CI container has no installed bread-module-host. It fails identically on the unrelated feature/register-breadcloud-app run 95 (2026-09-18), so it is pre-existing and not caused by this PR. Locally it passes only when bread-module-host is on PATH. The unlinkat ... permission denied line at the end of the log is the known hestia runner cleanup issue, and it is cosmetic: the job had already failed at the test step.

Fix, for a separate issue: have the tests point breadd at env!("CARGO_BIN_EXE_bread-module-host") (via an env override or config the tests set), or make the resolver also check the parent of deps/. This PR stays open until the check is green.

## Second-round review: wording is clean, blocked on CI (not merging; bread is also release-sensitive) The diff is fine: - The Documentation.md and Examples.md wording is accurate. - The fmt commit e729c11 is formatting only. I compared every touched file token by token: the only non-whitespace differences are rustfmt reordering `mod` lines and adding match-arm braces. **The CI failure is not the runner.** Run 97 (`check / check`) runs clippy and then `cargo test --workspace --locked` to completion. It then fails two tests in `breadd/tests/ipc_integration.rs`, `explicit_empty_permissions_is_scoped_but_not_flagged_ungated` and `scoped_module_sees_only_granted_state_read_permission`, with `failed to spawn bread-module-host at bread-module-host: No such file or directory`. Cause: `breadd/src/module_host.rs:377-387` (`resolve_module_host_binary`) looks for `bread-module-host` next to `current_exe()`. For an integration test that is `target/debug/deps/`, but the binary lives in `target/debug/`, so the lookup falls back to a bare `PATH` lookup, and the CI container has no installed bread-module-host. It fails identically on the unrelated `feature/register-breadcloud-app` run 95 (2026-09-18), so it is pre-existing and not caused by this PR. Locally it passes only when bread-module-host is on PATH. The `unlinkat ... permission denied` line at the end of the log is the known hestia runner cleanup issue, and it is cosmetic: the job had already failed at the test step. Fix, for a separate issue: have the tests point breadd at `env!("CARGO_BIN_EXE_bread-module-host")` (via an env override or config the tests set), or make the resolver also check the parent of `deps/`. This PR stays open until the check is green.

Breadway added 1 commit 2026-09-28 01:25:33 +08:00

fix: resolve bread-module-host correctly when running under a test binary ... 505977179a

resolve_module_host_binary looked only next to current_exe()'s own
directory. That works for a real breadd binary (target/debug/breadd,
sibling of target/debug/bread-module-host), but test and bench binaries
build one level deeper, into target/debug/deps/, so current_exe() there
never finds the sibling and the two breadd/tests/ipc_integration.rs tests
that spawn a real breadd child fail to find bread-module-host on some CI
layouts.

Now also checks the parent of a deps/ directory, which is where the real
workspace binaries actually live, without touching the primary
sibling-of-current_exe() lookup a production breadd install uses (it never
runs from a deps/ directory). Extracted the testable half into
resolve_module_host_binary_from(Option<&Path>) so this is covered by unit
tests instead of only ever being exercised implicitly through the
integration suite.

This is a pre-existing CI break also present on main (see ECO-34 second
review round), unrelated to this branch's wording changes.

Verified: cargo fmt --all -- --check, cargo clippy --workspace --all-targets
--locked -- -D warnings, cargo test --workspace --locked (all green,
including a re-run to rule out a one-off unrelated parallel-test flake),
matching this repo's actual CI gate (check.yml).

Breadway added 1 commit 2026-09-28 01:43:40 +08:00

fix: build bread-module-host before spawn_with_module's tests need it ... af38653c26

Reproduced the actual CI failure in this repo's own pinned container
(ci/build.sh's docker image), not just locally: the two failing
ipc_integration.rs tests were not a path-resolution bug at all.
`cargo test --workspace --locked` compiles bread-module-host's own
unit-test harness (proving the crate builds) but never links its actual
target/debug/bread-module-host executable, since nothing references
CARGO_BIN_EXE_bread-module-host (the trick breadd's own binary uses,
successfully, a few lines below) or otherwise forces cargo to build it.
The previous commit's deps/-parent fallback in resolve_module_host_binary
is real and worth keeping, but it was resolving a directory that never had
the binary in it either.

CARGO_BIN_EXE_bread-module-host cannot be forced through a
[dev-dependencies] path entry the normal way, because bread-module-host
deliberately has no [lib] target (see its own Cargo.toml), and Cargo's
artifact-dependency feature that would otherwise cover a bin-only crate is
still nightly-only (-Z bindeps, confirmed against this machine's stable
cargo).

spawn_with_module (the only TestHarness constructor that actually spawns a
module-host child) now runs `cargo build -p bread-module-host --locked`
once per test binary process before doing anything else, so the binary is
guaranteed to exist regardless of build/test ordering, matching what
build-order luck previously only sometimes provided.

Verified against a byte-for-byte reproduction of this repo's CI (its own
pinned Arch container via ci/build.sh, fresh target dir, no local
shortcuts): cargo clippy --workspace --all-targets --locked -- -D
warnings, and cargo test --workspace --locked, both fully green, including
the two previously-failing tests and module_host_sandbox.rs's tests (which
share the same underlying binary and happen to run after
ipc_integration.rs in cargo's target ordering).

Breadway commented 2026-09-28 13:03:33 +08:00

Author

Owner

Copy link

Round-3: fixed the pre-existing CI breakage blocking this PR (also present on main, unrelated to the wording changes here).

Reproduced the actual failure in this repo's own pinned CI container (not just locally): the two failing breadd/tests/ipc_integration.rs tests were not purely a path-resolution bug. cargo test --workspace --locked compiles bread-module-host's own unit-test harness but never links its actual target/debug/bread-module-host executable, since nothing forces cargo to build it (unlike breadd's own binary, forced via CARGO_BIN_EXE_breadd). CARGO_BIN_EXE_bread-module-host can't be used the same way because that crate deliberately has no [lib] target (can't be a [dev-dependencies] path entry), and Cargo's bin-only artifact-dependency feature is still nightly-only.

Two commits:

1. resolve_module_host_binary also checks the parent of a deps/ directory (real robustness improvement, though not the actual root cause here).
2. spawn_with_module now runs cargo build -p bread-module-host --locked once per test binary before spawning anything, guaranteeing the binary exists regardless of build/test ordering.

Verified against a byte-for-byte reproduction of this repo's CI (its own pinned Arch container via ci/build.sh, fresh target dir): cargo clippy --workspace --all-targets --locked -- -D warnings and cargo test --workspace --locked, both fully green, including module_host_sandbox.rs's tests.

CI on this branch is now green: https://git.breadway.dev/Breadway/bread/actions/runs/99

Round-3: fixed the pre-existing CI breakage blocking this PR (also present on main, unrelated to the wording changes here). Reproduced the actual failure in this repo's own pinned CI container (not just locally): the two failing `breadd/tests/ipc_integration.rs` tests were not purely a path-resolution bug. `cargo test --workspace --locked` compiles `bread-module-host`'s own unit-test harness but never links its actual `target/debug/bread-module-host` executable, since nothing forces cargo to build it (unlike `breadd`'s own binary, forced via `CARGO_BIN_EXE_breadd`). `CARGO_BIN_EXE_bread-module-host` can't be used the same way because that crate deliberately has no `[lib]` target (can't be a `[dev-dependencies]` path entry), and Cargo's bin-only artifact-dependency feature is still nightly-only. Two commits: 1. `resolve_module_host_binary` also checks the parent of a `deps/` directory (real robustness improvement, though not the actual root cause here). 2. `spawn_with_module` now runs `cargo build -p bread-module-host --locked` once per test binary before spawning anything, guaranteeing the binary exists regardless of build/test ordering. Verified against a byte-for-byte reproduction of this repo's CI (its own pinned Arch container via `ci/build.sh`, fresh target dir): `cargo clippy --workspace --all-targets --locked -- -D warnings` and `cargo test --workspace --locked`, both fully green, including `module_host_sandbox.rs`'s tests. CI on this branch is now green: https://git.breadway.dev/Breadway/bread/actions/runs/99

Breadway merged commit d215a3bd6b into main 2026-09-28 13:10:19 +08:00

Breadway deleted branch fix/pywal-wording 2026-09-28 13:10:19 +08:00

Breadway referenced this pull request from a commit 2026-09-28 13:10:20 +08:00

Merge pull request 'Reword pywal references to bread-theme in docs' (#6) from fix/pywal-wording into main

Breadway commented 2026-09-29 08:12:35 +08:00

Author

Owner

Copy link

Post-merge review of the round-3 commits (5059771, af38653): acceptable stopgap, one real gap, fix PR recommended (no revert)

Main (d215a3b) is green, and the wording change is fine. Comments on the two test-infra commits:

5059771, resolve_module_host_binary_from deps/-parent fallback (breadd/src/module_host.rs): fine. It is a harmless extra candidate, and production never runs from deps/. Its three unit tests assert the resolution behaviour directly. Keep it.

af38653, nested cargo build -p bread-module-host --locked in ensure_module_host_built (breadd/tests/ipc_integration.rs:1570-1584): it works in CI, and no deadlock occurs (cargo test does not hold the build lock while running test binaries). Offline is fine too, because the parent run already fetched every dependency. It is fragile in three ways:

1. module_host_sandbox.rs never calls it. Its header note (lines 35-45) still claims cargo test --workspace builds the binary, which your own commit message shows is false. It passes today only because ipc_integration runs first in target order. Reproduced on a fresh target dir: cargo test -p breadd --locked --test module_host_sandbox fails both tests. With no sibling binary, resolution fell back to the bare PATH name and picked up the installed ~/.local/bin/bread-module-host ("failed to spawn bread-module-host at bread-module-host: Permission denied"). So the sandbox tests can silently exercise a stale installed binary rather than the one under test.
2. Profile and target-dir mismatch. The nested build always uses the dev profile and default layout. Under cargo test --release, --target-dir or --target <triple>, the test binary looks next to target/release/ (or the custom dir), finds nothing, and falls back to PATH the same way.
3. The silent PATH fallback in tests is the root hazard. A test should fail loudly, not run against whatever is installed.

Recommended fix PR (small, test-only):

• Move ensure_module_host_built into a shared breadd/tests/common/mod.rs, and call it from both spawn_with_module and module_host_sandbox.rs's harness. Fix the stale note.
• Have it return the built path instead of relying on resolution: run the nested build with --message-format=json, and read the executable field of the bread-module-host compiler-artifact message. Pass that path to the spawned breadd through an explicit override env var that resolve_module_host_binary checks first. That makes profile, target-dir and triple irrelevant.
• Cheaper alternative, or in addition: add a cargo build --workspace --locked step before cargo test in .forgejo/workflows/check.yml, so CI never depends on the in-test build.

No revert needed. Nothing here affects production breadd.

## Post-merge review of the round-3 commits (5059771, af38653): acceptable stopgap, one real gap, fix PR recommended (no revert) Main (d215a3b) is green, and the wording change is fine. Comments on the two test-infra commits: **5059771, `resolve_module_host_binary_from` deps/-parent fallback** (`breadd/src/module_host.rs`): fine. It is a harmless extra candidate, and production never runs from `deps/`. Its three unit tests assert the resolution behaviour directly. Keep it. **af38653, nested `cargo build -p bread-module-host --locked` in `ensure_module_host_built`** (`breadd/tests/ipc_integration.rs:1570-1584`): it works in CI, and no deadlock occurs (cargo test does not hold the build lock while running test binaries). Offline is fine too, because the parent run already fetched every dependency. It is fragile in three ways: 1. **`module_host_sandbox.rs` never calls it.** Its header note (lines 35-45) still claims `cargo test --workspace` builds the binary, which your own commit message shows is false. It passes today only because `ipc_integration` runs first in target order. Reproduced on a fresh target dir: `cargo test -p breadd --locked --test module_host_sandbox` fails both tests. With no sibling binary, resolution fell back to the bare `PATH` name and picked up the **installed** `~/.local/bin/bread-module-host` ("failed to spawn bread-module-host at bread-module-host: Permission denied"). So the sandbox tests can silently exercise a stale installed binary rather than the one under test. 2. **Profile and target-dir mismatch.** The nested build always uses the dev profile and default layout. Under `cargo test --release`, `--target-dir` or `--target <triple>`, the test binary looks next to `target/release/` (or the custom dir), finds nothing, and falls back to `PATH` the same way. 3. **The silent PATH fallback in tests is the root hazard.** A test should fail loudly, not run against whatever is installed. **Recommended fix PR** (small, test-only): - Move `ensure_module_host_built` into a shared `breadd/tests/common/mod.rs`, and call it from both `spawn_with_module` and `module_host_sandbox.rs`'s harness. Fix the stale note. - Have it return the built path instead of relying on resolution: run the nested build with `--message-format=json`, and read the `executable` field of the `bread-module-host` compiler-artifact message. Pass that path to the spawned breadd through an explicit override env var that `resolve_module_host_binary` checks first. That makes profile, target-dir and triple irrelevant. - Cheaper alternative, or in addition: add a `cargo build --workspace --locked` step before `cargo test` in `.forgejo/workflows/check.yml`, so CI never depends on the in-test build. No revert needed. Nothing here affects production breadd.

Breadway referenced this pull request from Breadway/bread-ecosystem 2026-09-29 08:13:00 +08:00

Reword pywal mentions across remaining bread apps and docs #34

Breadway referenced this pull request from a commit 2026-09-29 08:44:56 +08:00

fix: build and pin the real bread-module-host binary for integration tests

Breadway referenced this pull request 2026-09-29 08:45:08 +08:00

Build and pin the real bread-module-host binary for integration tests #7

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

breadbug

  

bug

  

phase-1

  

phase-2

  

privacy

No labels

breadbug

bug

phase-1

phase-2

privacy

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

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/bread!6

No description provided.