refactor: replace sharp with imgkit#686
refactor: replace sharp with imgkit#686ThatOneCalculator wants to merge 1 commit intoletta-ai:mainfrom
Conversation
|
Requesting manual test/test guidance from a maintainer, tests that fail in CI also fail on my local machine on the main branch (83cdccd) -- I can't determine if my changes actually cause test regression, or if tests are having unrelated issues. |
|
Thanks for the PR and for maintaining the AUR package! I looked into the sharp loading issue from the AUR comment, and I think this can be solved at the packaging layer without swapping dependencies. The IssueSharp's bundled platform-specific binaries aren't being found during the AUR build/install process. Recommended Fix (in the PKGBUILD)Sharp's installation docs explicitly support using system libvips instead of bundled binaries - this is the standard approach for distro packages: depends=('nodejs' 'org.freedesktop.secrets' 'libvips') # Add libvips
package() {
cd "${_pkgname}-${pkgver}"
export SHARP_FORCE_GLOBAL_LIBVIPS=1 # Use system libvips, not bundled
bun install
bun run build
install -Dm755 ./letta.js "$pkgdir/usr/bin/letta"
}This way:
Concerns About imgkitWe'd prefer not to replace sharp with imgkit because:
If the PKGBUILD fix doesn't work, happy to discuss alternatives, but let's try the standard distro-packaging approach first. 👾 Generated with Letta Code |
|
^ @ThatOneCalculator asked my letta code to drop a comment here, the tldr is trying to keep deps as tight as possible in this repo, and feel a little uneasy swapping a mainline dep with a low profile package, would much rather try to solve the issue w/ sharp if possible |
|
@cpacker unfortunately, already had |
|
There's also always jimp, which is battle-tested (1,868,037 weekly downloads), but it's slower than sharp -- although for this use case I don't think that's a big issue. |
Yeah I think the speed doesn't really matter much here, given we're talking about a TUI connected to an LLM ;) Maybe one thing we could do is expose a build var that changes the dep for image resizing, and pass that for the AUR build? That way we keep the bundle size down / transitive deps as low as possible, but also unbrick the AUR build case (eg the AUR build could wire to jimp instead?). I think the callsites for the image operations is pretty narrow so intuitively this feels pretty reasonable to me. |
If you're fine with that solution, then I'll just implement it using |
|
Closing this PR, as I'll work on that solution in a new branch :) |
Yeah just to clarify we're on the page, something like:
|
Replaces sharp with imgkit for the following reasons: