From kde-core-devel Tue Oct 08 02:54:12 2013 From: "Ben Cooksley" Date: Tue, 08 Oct 2013 02:54:12 +0000 To: kde-core-devel Subject: Re: Review Request 113139: Try to export include targets for Plasma as well Message-Id: <20131008025412.20799.16755 () vidsolbach ! de> X-MARC-Message: https://marc.info/?l=kde-core-devel&m=138120089917248 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============6452491142818415390==" --===============6452491142818415390== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit > On Oct. 7, 2013, 9:35 a.m., Stephen Kelly wrote: > > src/plasma/CMakeLists.txt, line 173 > > > > > > The if-else shouldn't be needed. INSTALL_INTERFACE should already check if ${INCLUDE_INSTALL_DIR} is absolute. > > Ben Cooksley wrote: > I copied this code from KCoreAddons in kdelibs[frameworks]. Shall I correct it there as well? > > Stephen Kelly wrote: > It appears in several other frameworks too. Actually your snippet is needed until CMake 2.8.12. I'll remove it from them all when kde requires that version. Okay. I removed the IS_ABSOLUTE part when I committed though, so that might need to be adjusted back in then. - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113139/#review41324 ----------------------------------------------------------- On Oct. 7, 2013, 10:48 a.m., Ben Cooksley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/113139/ > ----------------------------------------------------------- > > (Updated Oct. 7, 2013, 10:48 a.m.) > > > Review request for kdelibs, Plasma and Stephen Kelly. > > > Repository: plasma-framework > > > Description > ------- > > Add include targets for KF5::plasma, which will hopefully contribute towards fixing the kde-workspace[master] build on build.kde.org. > Unfortunately it isn't entirely successful as it seems camelcase headers are installed by KF5::plasma into include/KDE/Plasma/ but it should be a start... > > > Diffs > ----- > > src/plasma/CMakeLists.txt b21fd7b > > Diff: http://git.reviewboard.kde.org/r/113139/diff/ > > > Testing > ------- > > In place on CI build system. Proper include path now given to compiler. > > > Thanks, > > Ben Cooksley > > --===============6452491142818415390== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit

This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113139/

On October 7th, 2013, 9:35 a.m. UTC, Stephen Kelly wrote:
src/plasma/CMakeLists.txt (Diff revision 1)
173
if(IS_ABSOLUTE "${INCLUDE_INSTALL_DIR}")
The if-else shouldn't be needed. INSTALL_INTERFACE should already check if ${INCLUDE_INSTALL_DIR} is absolute.
On October 7th, 2013, 9:36 a.m. UTC, Ben Cooksley wrote:
I copied this code from KCoreAddons in kdelibs[frameworks]. Shall I correct it there as well?
On October 7th, 2013, 11:50 a.m. UTC, Stephen Kelly wrote:
It appears in several other frameworks too. Actually your snippet is needed until CMake 2.8.12. I'll remove it from them all when kde requires that version.

Okay. I removed the IS_ABSOLUTE part when I committed though, so that might need to be adjusted back in then.

- Ben

On October 7th, 2013, 10:48 a.m. UTC, Ben Cooksley wrote:

Review request for kdelibs, Plasma and Stephen Kelly.

By Ben Cooksley.

Updated Oct. 7, 2013, 10:48 a.m.

Repository: plasma-framework

Description

Add include targets for KF5::plasma, which will hopefully contribute towards fixing the kde-workspace[master] build on build.kde.org.
Unfortunately it isn't entirely successful as it seems camelcase headers are installed by KF5::plasma into include/KDE/Plasma/ but it should be a start...

Testing

In place on CI build system. Proper include path now given to compiler.

Diffs

src/plasma/CMakeLists.txt (b21fd7b)

--===============6452491142818415390==--