search

psi-plus / main

Main repository with patches and required resources
https://psi-plus.com/
GNU Lesser General Public License v2.1
69 stars 20 forks source link

icons are no longer installed #776

Closed jirislaby closed 4 years ago

jirislaby commented 4 years ago

After update from 1.4.962 to 1.4.1004, iconsets are not installed during cmake's install. In particular files like these:

/usr/share/psi-plus/iconsets/activities/default/doing_chores.png
/usr/share/psi-plus/iconsets/activities/default/doing_chores_buying_groceries.png
/usr/share/psi-plus/iconsets/activities/default/doing_chores_cleaning.png
/usr/share/psi-plus/iconsets/activities/default/doing_chores_cooking.png

and so on. I bisected it to commit https://github.com/psi-plus/psi-plus-snapshots/commit/8db31f52c61b8058aef4a8174c49f8406f02a224 where prepare_iconsets was introduced. As far as I can see, the function is called only for prepare-bin. Is a call for another target missing?

tehnick commented 4 years ago

@jirislaby These files were never used in runtime, because they are embedded into psi-plus executable at build time and that embedded version of files is used in program runtime.

jirislaby commented 4 years ago

@tehnick thanks for confiriming as I am updating the openSUSE package and I hit this. BTW for completeness, it's all these dirs + README:

error: File not found: /usr/share/psi-plus/iconsets/activities/default
error: File not found: /usr/share/psi-plus/iconsets/affiliations/default
error: File not found: /usr/share/psi-plus/iconsets/clients/default
error: File not found: /usr/share/psi-plus/iconsets/moods/default
error: File not found: /usr/share/psi-plus/iconsets/roster/default
error: File not found: /usr/share/psi-plus/iconsets/system/default
error: File not found: /usr/share/psi-plus/iconsets/system/README
error: File not found: /usr/share/psi-plus/iconsets/emoticons/default
error: File not found: /usr/share/psi-plus/themes

All those default are embedded, I assume.

Vitozz commented 4 years ago

There is a regular expression to exclude default iconsets https://github.com/psi-im/psi/blob/d2199bcd5fa653e1240b30cc1131566790ab1b18/src/CMakeLists.txt#L475 which is used in https://github.com/psi-im/psi/blob/d2199bcd5fa653e1240b30cc1131566790ab1b18/src/CMakeLists.txt#L512

jirislaby commented 4 years ago

@Vitozz but (per default):

$ grep INSTALL_EXTRA_FILES CMakeCache.txt 
INSTALL_EXTRA_FILES:BOOL=ON

BTW is missing themes dir OK too? In the old version, it contained:

/usr/share/psi-plus/themes
/usr/share/psi-plus/themes/chatview
/usr/share/psi-plus/themes/chatview/adium
/usr/share/psi-plus/themes/chatview/adium/Template.html
/usr/share/psi-plus/themes/chatview/adium/adapter.js
/usr/share/psi-plus/themes/chatview/moment-with-locales.js
/usr/share/psi-plus/themes/chatview/psi
/usr/share/psi-plus/themes/chatview/psi/LunnaCat_Classic
/usr/share/psi-plus/themes/chatview/psi/LunnaCat_Classic/images
/usr/share/psi-plus/themes/chatview/psi/LunnaCat_Classic/images/ChatLog.png
/usr/share/psi-plus/themes/chatview/psi/LunnaCat_Classic/images/HR.png
/usr/share/psi-plus/themes/chatview/psi/LunnaCat_Classic/images/ScrollBarArrowDown.png
/usr/share/psi-plus/themes/chatview/psi/LunnaCat_Classic/images/ScrollBarArrowUp.png
/usr/share/psi-plus/themes/chatview/psi/LunnaCat_Classic/index.html
/usr/share/psi-plus/themes/chatview/psi/LunnaCat_Classic/load.js
/usr/share/psi-plus/themes/chatview/psi/LunnaCat_Classic/screenshot.png
/usr/share/psi-plus/themes/chatview/psi/adapter.js
/usr/share/psi-plus/themes/chatview/psi/classic
/usr/share/psi-plus/themes/chatview/psi/classic/index.html
/usr/share/psi-plus/themes/chatview/psi/classic/load.js
/usr/share/psi-plus/themes/chatview/psi/new_classic
/usr/share/psi-plus/themes/chatview/psi/new_classic/index.html
/usr/share/psi-plus/themes/chatview/psi/new_classic/load.js
/usr/share/psi-plus/themes/chatview/psi/new_classic/screenshot.png
/usr/share/psi-plus/themes/chatview/psi/psi.css
/usr/share/psi-plus/themes/chatview/util.js
Vitozz commented 4 years ago

Themes available only in webkit/webengine. May be you forgot to set CHAT_TYPE variable

jirislaby commented 4 years ago

@Vitozz from psi-im/psi@53cc2fe9a0aef8cdacbbd2c6c13581bd717329cd, I cab see the default has changed. What was the reason? Or, what is the preferred option for the distributions now?

Vitozz commented 4 years ago

After discussions at the developer conference, it was decided to make the base version the default version. Therefore, for other versions you need to set a flag. If you need webkit version you need to set -DCHAT_TYPE=webkit If you need webengine version you need to set -DCHAT_TYPE=webengine

jirislaby commented 4 years ago

@Vitozz I understand the flag, but it's not about me and what I need. Note above that I am a maintainer of openSUSE's psi+. So what should be the default for the distribution? I incline to still use webengine as it was the default until now.

tehnick commented 4 years ago

What was the reason?

See description here: https://github.com/psi-im/psi#description Different users prefers different variants.

From maintainer point of view...

In Debian and Ubuntu I build separate psi-plus and psi-plus-webkit packages: https://packages.debian.org/source/sid/psi-plus (webengine version is not built because QtWebengine available on small set of architectures)

For macOS I build separate app bundles for basic and webengine versions for Psi+: https://sourceforge.net/projects/psiplus/files/macOS/tehnick/ (webkit version is not built because QtWebkit is not available in Homebrew)

For MS Windows I place both basic and webkit versions in one archive: https://sourceforge.net/projects/psiplus/files/Windows/Personal-Builds/tehnick/ (webengine version is not built because QtWebengine is not available for MinGW builds)

In Haiku only package with webkit version of Psi+ exists. (webengine version is not built because nor Chromium, nor other programs based on its engine do not work in Haiku now, basic version is not built because most of Haiku users prefer as feature full software as possible)

tehnick commented 4 years ago

So what should be the default for the distribution? I incline to still use webengine as it was the default until now.

If you support packages only for amd64 architecture that is fine.

But on i686 QtWebengine has a number of problems, and on other architectures it may be unavailable at all...

tehnick commented 4 years ago

@jirislaby I have updated comment https://github.com/psi-plus/main/issues/776#issuecomment-579304080 Hope this helps.

jirislaby commented 4 years ago

Yes, thanks.