D26752: ECMAddAppIcon: Do not warn about mac and window icons if isnt a OS specific build

[Christophe Giboudeaux]

cgiboudeaux added a comment.


  In D26752#597149 <https://phabricator.kde.org/D26752#597149>, @patrickelectric wrote:
  
  > Hi @cgiboudeaux and @bcooksley, there is a reason of why this patch is valid. Have you read the commit message ?
  >
  > In #kirogi <https://phabricator.kde.org/tag/kirogi/> we provide a valid icon (svg) with a valid prefix (sc), as you probably know *sc* stands for for scalable (SVG) files.
  
  
  As expected, the problem is in the kirogi code. Read the ECMAddAppIcon doc:
  
    # The given icons, whose names must match the pattern::
    #
    #   <size>-<other_text>.png
  
  and now look at the kirogi code:
  
    file(GLOB ICONS_SRCS "${CMAKE_CURRENT_SOURCE_DIR}/../data/icons/*apps-kirogi.svg")
    ecm_add_app_icon(kirogi_SRCS ICONS ${ICONS_SRCS})
  
  The warning about Windows is expected.
  
  Now about Apple: if a .svg or .svgz is passed to ecm_add_app_icon, ksvg2icns (if found) will create the icons bundle.
  
  The solution is not hiding the warning when the macro is called on different platform but fixing the condition.
  
  Suggestion: After landing D26751 <https://phabricator.kde.org/D26751>, add a `else()` block to `if (NOT (ext STREQUAL "svg" OR ext STREQUAL "svgz"))` where you `set(_ecm_add_app_icon_svg_icons TRUE)`
  and change:
  
    if (NOT (mac_icons OR mac_sidebar_icons))
  
  to:
  
    if (NOT (mac_icons OR mac_sidebar_icons) AND NOT _ecm_add_app_icon_svg_icons)
  
  Since ksvg2ico doesn't create the sidebar icons it can be further improved but at least you won't see a warning if you call ecm_add_app_icon with only svg files.
  
  You will only get the warning about the broken behaviour on Windows.
  
  Now another note: kirogi doesn't provide any png icon, this is also bad on Linux. Please create png icons and install them in the hicolor namespace.
  See https://specifications.freedesktop.org/icon-theme-spec/icon-theme-spec-latest.html#install_icons

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D26752

To: patrickelectric, apol, tcanabrava, cgiboudeaux, bcooksley
Cc: bcooksley, patrickelectric, apol, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-buildsystem/attachments/20200120/6058d41b/attachment.html>