Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(Application Search): Improve Linux compatibility #1097

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ke1v
Copy link
Contributor

@ke1v ke1v commented May 16, 2024

Changes:

  • Refactored LinuxAppIconExtractor to use sharp instead of calling convert on the command line as some Linux distros (notably Linux Mint) does not come with the convert package installed by default.
  • Refactored LaunchDesktopFile to use gio launch as it appears to be installed on all Linux distros I tested (even KDE based ones)

@oliverschwendener I import sharp directly in LinuxAppIconExtractor. Let me know if you'd like me to create a module to handle image conversion. I'm also unfamiliar with electron-builder so I'm not sure if asar is the best way to handle packaging sharp (it wouldn't bundle without it)

@oliverschwendener
Copy link
Owner

Can you try packaging the app and check if it works with the newly added package?

rm -rf node_modules

pnpm config set shamefully-hoist true --location=project
pnpm config set auto-install-peers true --location=project
pnpm config set strict-peer-dependencies true --location=project

pnpm install
pnpm build
pnpm package

@ke1v
Copy link
Contributor Author

ke1v commented May 17, 2024

Tried it and it still doesn't package correctly without asarUnpack, which I think is because sharp uses C binaries. I could try switching to a pure Javascript image library if you don't want to use libraries with binary dependencies.

@oliverschwendener
Copy link
Owner

So, if this doesn't work when packaged I don't think we can merge it 🤷

@oliverschwendener
Copy link
Owner

If you want, you can open a separate PR for the refactoring in LaunchDesktopFile so we can merge that.

@ke1v
Copy link
Contributor Author

ke1v commented May 31, 2024

Sorry about the late reply, but I think I may have misworded my initial issue. sharp works and packages without issue with the addition of asarUnpack but I thought you had disabled asar for electron. However I believe it is enabled by default so this shouldn't be an issue.

@oliverschwendener oliverschwendener marked this pull request as ready for review June 1, 2024 09:29
@oliverschwendener
Copy link
Owner

Generally I am very conservative when it comes to adding more dependencies, especially when they're only used on one platform. But let's give this a try as it simplifies the code a lot regarding the icon generation on Linux. Nevertheless, I reserve the right to remove this package anytime if it proves to be a burden.

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.

None yet

2 participants