sitter requested changes to this revision. sitter added inline comments. This revision now requires changes to proceed.
INLINE COMMENTS > KDEInstallDirs.cmake:699 > +if(INSTALL_PREFIX_SCRIPT) > + file(WRITE ${CMAKE_BINARY_DIR}/prefix.sh " > +export XDG_DATA_DIRS=${KDE_INSTALL_FULL_DATADIR}:$XDG_DATA_DIRS >From a style perspective, I'd suggest having the prefix.sh live somewhere in >the installed ECM tree and get copied, rather than maintained as a glorified >heredoc in the cmake code. That's just a suggestion though. > KDEInstallDirs.cmake:700 > + file(WRITE ${CMAKE_BINARY_DIR}/prefix.sh " > +export XDG_DATA_DIRS=${KDE_INSTALL_FULL_DATADIR}:$XDG_DATA_DIRS > +export XDG_CONFIG_DIRS=${KDE_INSTALL_FULL_CONFDIR}:$XDG_CONFIG_DIRS This is not correct, the XDG_ vars are not necessarily set, so all code that sets them ought to ensure their default values are appended if necessary. > If $XDG_DATA_DIRS is either not set or empty, a value equal to > /usr/local/share/:/usr/share/ should be used. i.e. export XDG_DATA_DIRS=${KDE_INSTALL_FULL_DATADIR}:${XDG_DATA_DIRS:-/usr/local/share:/usr/share} > KDEInstallDirs.cmake:701 > +export XDG_DATA_DIRS=${KDE_INSTALL_FULL_DATADIR}:$XDG_DATA_DIRS > +export XDG_CONFIG_DIRS=${KDE_INSTALL_FULL_CONFDIR}:$XDG_CONFIG_DIRS > +export PATH=${KDE_INSTALL_FULL_BINDIR}:$PATH Same as for XDG_DATA_DIRS > If $XDG_CONFIG_DIRS is either not set or empty, a value equal to /etc/xdg > should be used." REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D9299 To: apol, #frameworks, sitter Cc: sitter, cgiboudeaux, #build_system