Skip to content

ci(nix): include assets/ in flake source filter#41

Merged
hewigovens merged 1 commit into
mainfrom
ci/nix-fix-assets
May 8, 2026
Merged

ci(nix): include assets/ in flake source filter#41
hewigovens merged 1 commit into
mainfrom
ci/nix-fix-assets

Conversation

@hewigovens
Copy link
Copy Markdown
Owner

Summary

First AppImage CI run (https://github.com/hewigovens/jayjay/actions/runs/25495870533) failed at the build phase:
```
error: couldn't read `shell/gpui/src/../assets/fonts/Phosphor.ttf`: No such file or directory
```

`craneLib.cleanCargoSource` keeps only Rust/Cargo files and drops `shell/gpui/assets/`, but the binary `include_bytes!`s fonts at compile time. Replace with a custom `cleanSourceWith` filter that keeps cargo sources and anything under an `assets/` directory.

Test plan

  • CI workflow run on this branch goes green: `gh workflow run appimage.yml --ref ci/nix-fix-assets`
  • AppImage artifact uploaded

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request modifies the Nix build configuration in flake.nix to include asset files in the build source, preventing compilation errors for binaries that rely on include_bytes!. The reviewer suggested an improvement to the source filtering by using pkgs.lib.cleanSource to avoid unnecessary rebuilds triggered by changes in local metadata like the .git directory.

Comment thread flake.nix Outdated
@hewigovens hewigovens force-pushed the ci/nix-fix-assets branch 2 times, most recently from cb69f22 to 2671d35 Compare May 7, 2026 13:44
@hewigovens
Copy link
Copy Markdown
Owner Author

Second iteration:

Run https://github.com/hewigovens/jayjay/actions/runs/25497574946 cleared the asset filter issue, then failed at the test phase — crane.buildPackage runs cargo test by default and opens_linear_fixture_with_working_copy_selected panicked because jj isn't on PATH inside the nix sandbox.

Fix: doCheck = false for the AppImage build. The AppImage workflow's job is packaging; gpui-ci.yml already runs cargo test on Linux with a provisioned jj.

Re-running on https://github.com/hewigovens/jayjay/actions/runs/25499683366

@hewigovens hewigovens force-pushed the ci/nix-fix-assets branch from 2671d35 to 2f4ef3b Compare May 7, 2026 13:47
hewigovens added a commit that referenced this pull request May 7, 2026
- nix-community/cache-nix-action caches the store between runs (cold ~25 min,
  cached ~3-5 min). Keyed by hashes of flake.nix, flake.lock, and Cargo.lock.
- Wrap flake src in lib.cleanSource so .git, result symlinks, and editor
  cruft don't churn the build hash. Addresses PR #41 review comment.
@hewigovens hewigovens force-pushed the ci/nix-fix-assets branch from 2f4ef3b to 216ae60 Compare May 7, 2026 22:39
hewigovens added a commit that referenced this pull request May 7, 2026
- Custom cleanSourceWith filter so include_bytes! finds shell/gpui/assets/.
  Wrap input in lib.cleanSource to ignore .git, result symlinks, and editor
  cruft (review feedback on PR #41).
- doCheck = false; the AppImage build is for packaging — gpui-ci.yml runs
  cargo test on Linux with a provisioned jj on PATH, so don't duplicate.
- nix-community/cache-nix-action persists /nix/store between runs (cold
  ~25 min → cached ~3-5 min). Keyed by hashes of flake.nix, flake.lock,
  and Cargo.lock.
@hewigovens hewigovens force-pushed the ci/nix-fix-assets branch from 216ae60 to fdbf08f Compare May 7, 2026 22:39
@hewigovens
Copy link
Copy Markdown
Owner Author

✅ Green: https://github.com/hewigovens/jayjay/actions/runs/25525954689

The next run on this branch (no flake.nix / flake.lock / Cargo.lock changes) should hit the warm cache. Will trigger one to verify before merge.

@hewigovens hewigovens force-pushed the ci/nix-fix-assets branch from fdbf08f to c63b9c5 Compare May 7, 2026 23:40
…mments; drop tag trigger

- Rebased onto post-#42 main (jj-lib 0.41 + gix 0.83). Cargo.lock churn means
  the cache key shifts; this run repopulates.
- Bump actions/upload-artifact v4 -> v5 and cache-nix-action v6 -> v7. v6 was
  failing tar exit 2 on cache restore; v7 reworked the tarball strategy. Both
  ride Node 24 (v4/v6 were deprecated Node 20).
- Drop the `tags: v*` trigger; we're not shipping the AppImage as a release
  asset yet (no Linux icons/menus). workflow_dispatch only.
- Tighten verbose comments in flake.nix to one-liners.
@hewigovens hewigovens force-pushed the ci/nix-fix-assets branch from c63b9c5 to 674f834 Compare May 8, 2026 00:02
@hewigovens hewigovens merged commit 1ac88aa into main May 8, 2026
1 check passed
@hewigovens hewigovens deleted the ci/nix-fix-assets branch May 8, 2026 00:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant