Skip to content
This repository has been archived by the owner on Mar 11, 2024. It is now read-only.

tmp fix: broken icon on linux (deb) #41

Merged
merged 1 commit into from
Nov 1, 2021
Merged

Conversation

0xjac
Copy link
Contributor

@0xjac 0xjac commented Nov 1, 2021

This was previously mentioned on discord.

Context

The icon is package incorrectly for linux (deb). This is due to a bug in app-builder which is a dependency (wrapped as a npm module in app-builder-bin) of electron-builder, the tool used for packaging.

There is a fix for it, but it hasn't been merged yet. And once it is the following must happened:

  1. A new version of app-builder must be released with the fix, together with a new version of the js wrapper (app-builder-bin).
  2. electron-builder must update its dependencies to include the new version of app-builder-bin.
  3. A new version of electron-builder with the updated dependencies must be released.
  4. The electron-builder dev dependency of alephium-wallet must be updated.
  5. A new release of alephium-wallet can be released with the icon.

While I'm confident that you can do the last two points quite quickly (providing no breaking changes from electron-builder), all the former points may take some times and are outside this project's control. It's likely that this won't happen before the mainnet release. Hence I'm submitting this quick fix to manually set the icon size to 512 and package the icon correctly for the big day.

If you would like to avoid hacks and temporary fixes like this one, feel free to close the PR 😉

Changes

This PR adds an afterPack hook which after gathering all the files for a deb archives, remaps all icons with a size of 0 to 512.

Things to note:

  • Any icon (I don't think there should be any other) with a correct (non-0) size will not be affected.
  • Any icon with a 0 size will be set to 512.
  • Adding icons with a detected size of 0 and an actual size other than 512 will be packaged incorrectly.
  • Once app-builder is fixed, the correct size of 512 should be detected and the hook should not have any effect. It can then be removed. (If it remains, it should have no effects, besides wasting some CPU cycles.)

Copy link
Member

@polarker polarker left a comment

Choose a reason for hiding this comment

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

LGTM, many thanks !

@0xjac
Copy link
Contributor Author

0xjac commented Nov 1, 2021

@polarker thanks, I missed the fact that my fork was just a bit behind so I updated my branch 🙄

@polarker polarker merged commit d0e431f into alephium:master Nov 1, 2021
0xjac added a commit to 0xjac/alephium-wallet that referenced this pull request Nov 5, 2021
Electron should use the executable icon as the default application
window. However this seems broken on Linux. This might be related to
alephium#41

This sets the alephium icon for the application to be displayed by the
wm on the window decoration, on windows switchers, etc.
polarker pushed a commit that referenced this pull request Nov 9, 2021
Electron should use the executable icon as the default application
window. However this seems broken on Linux. This might be related to
#41

This sets the alephium icon for the application to be displayed by the
wm on the window decoration, on windows switchers, etc.
LeeAlephium pushed a commit that referenced this pull request May 24, 2022
tmp fix: broken icon on linux (deb)
LeeAlephium pushed a commit that referenced this pull request May 24, 2022
Electron should use the executable icon as the default application
window. However this seems broken on Linux. This might be related to
#41

This sets the alephium icon for the application to be displayed by the
wm on the window decoration, on windows switchers, etc.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants