Bug 354610 - ECMInstallIcons.cmake may call REMOVE_DUPLICATES(_themes) without checking if _themes is actually set.
Summary: ECMInstallIcons.cmake may call REMOVE_DUPLICATES(_themes) without checking if...
Status: RESOLVED FIXED
Alias: None
Product: extra-cmake-modules
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Alex Merry
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-10-30 18:05 UTC by Brice De Bruyne
Modified: 2015-11-04 09:41 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
ecm-fix-ECMInstallIcons_cmake-1.patch (618 bytes, patch)
2015-10-30 18:07 UTC, Brice De Bruyne
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Brice De Bruyne 2015-10-30 18:05:59 UTC
git master commit fb7b8eea7d91772f989d5b060c86df20f2ebdb66
breaks ECMInstallIcons.cmake when calling REMOVE_DUPLICATES(_theme)
when _theme is unset.

This renders ksirk unconfigurable, possibly other packages too.

A simple check whether _themes is set fixes this:

--- extra-cmake-modules-1/modules/ECMInstallIcons.cmake 2015-10-30 18:53:04.065271019 +0100
+++ extra-cmake-modules/modules/ECMInstallIcons.cmake   2015-10-30 18:53:51.103271014 +0100
@@ -169,10 +169,12 @@
       endif( _theme_GROUP)
    endforeach (_current_ICON)
 
-   list(REMOVE_DUPLICATES _themes)
-   foreach(_theme ${_themes})
-       _ecm_update_iconcache("${_defaultpath}" "${_theme}")
-   endforeach()
+   if(_themes)
+      list(REMOVE_DUPLICATES _themes)
+      foreach(_theme ${_themes})
+          _ecm_update_iconcache("${_defaultpath}" "${_theme}")
+      endforeach()
+   endif(_themes)
 
 endmacro()
 




Reproducible: Always

Steps to Reproduce:
1. install latest ecm
2. get latest ksirk git
3. try to get it configured.


Actual Results:  
cmake error

Expected Results:  
cmake succeeds

fix:

--- extra-cmake-modules-1/modules/ECMInstallIcons.cmake 2015-10-30 18:53:04.065271019 +0100
+++ extra-cmake-modules/modules/ECMInstallIcons.cmake   2015-10-30 18:53:51.103271014 +0100
@@ -169,10 +169,12 @@
       endif( _theme_GROUP)
    endforeach (_current_ICON)
 
-   list(REMOVE_DUPLICATES _themes)
-   foreach(_theme ${_themes})
-       _ecm_update_iconcache("${_defaultpath}" "${_theme}")
-   endforeach()
+   if(_themes)
+      list(REMOVE_DUPLICATES _themes)
+      foreach(_theme ${_themes})
+          _ecm_update_iconcache("${_defaultpath}" "${_theme}")
+      endforeach()
+   endif(_themes)
 
 endmacro()
Comment 1 Brice De Bruyne 2015-10-30 18:07:34 UTC
Created attachment 95229 [details]
ecm-fix-ECMInstallIcons_cmake-1.patch
Comment 2 Alex Merry 2015-11-03 12:05:34 UTC
Given that this only happens when ecm_install_icons is called in such a way that it does nothing, I figured it would be better to make it a warning:

https://git.reviewboard.kde.org/r/125931/

In the meantime, ksirk can just have that ecm_install_icons call removed, as it is useless.
Comment 3 Alex Merry 2015-11-04 09:41:01 UTC
Git commit 21629f651a6a5d9d977be03fd9f98417c4fa27ae by Alex Merry.
Committed on 04/11/2015 at 09:40.
Pushed by alexmerry into branch 'master'.

Warn instead of error if ecm_install_icons finds no icons.

The V1 syntax of ecm_install_icons searched for icons by globbing files
with a particular naming pattern. If there were no such icons, this used
to do nothing, but silently. Commit fb7b8eea7d accidentally made this an
error. More sensible would be to make it a warning.
REVIEW: 125931

M  +8    -4    modules/ECMInstallIcons.cmake
M  +1    -0    tests/ECMInstallIconsTest/CMakeLists.txt
A  +1    -0    tests/ECMInstallIconsTest/v1-syntax-no-icons/CMakeLists.txt

http://commits.kde.org/extra-cmake-modules/21629f651a6a5d9d977be03fd9f98417c4fa27ae