| Summary: | ECMInstallIcons.cmake may call REMOVE_DUPLICATES(_themes) without checking if _themes is actually set. | ||
|---|---|---|---|
| Product: | [Frameworks and Libraries] extra-cmake-modules | Reporter: | Brice De Bruyne <bricedb> |
| Component: | general | Assignee: | Alex Merry <alex.merry> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | ecm-bugs-null |
| Priority: | NOR | ||
| Version First Reported In: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Compiled Sources | ||
| OS: | Linux | ||
| Latest Commit: | http://commits.kde.org/extra-cmake-modules/21629f651a6a5d9d977be03fd9f98417c4fa27ae | Version Fixed/Implemented In: | |
| Sentry Crash Report: | |||
| Attachments: | ecm-fix-ECMInstallIcons_cmake-1.patch | ||
Created attachment 95229 [details]
ecm-fix-ECMInstallIcons_cmake-1.patch
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. 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 |
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()