Review Request 113406: Add a macro to automatically generate forward headers

[Aleix Pol Gonzalez]

> On Oct. 24, 2013, 1:54 p.m., Stephen Kelly wrote:
> > modules/ECMGenerateHeaders.cmake, line 32
> > <http://git.reviewboard.kde.org/r/113406/diff/2/?file=205462#file205462line32>
> >
> >     I recommend not putting this in the API of the function, and instead users should use 
> >     
> >      install(DIRECTORY) to install the generated files.
> 
> Aleix Pol Gonzalez wrote:
>     Why?
> 
> Stephen Kelly wrote:
>     I think it's better API. It's what all existing functions which generate headers do.
> 
> Aleix Pol Gonzalez wrote:
>     Then it's weird because we'll need to have a separate call for installing all the headers as well, so we will have them all listed twice.
> 
> Stephen Kelly wrote:
>     Are you sure? You wouldn't just have a single install(DIRECTORY), as I wrote before?
> 
> Aleix Pol Gonzalez wrote:
>     See the KParts patch I attached. We want to install the "actual" header, not the forward header, right?
> 
> Stephen Kelly wrote:
>     Your kparts patch contains this:
>     
>      +#still need to see what to do with these...
>      +# install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/kparts DESTINATION  ${INCLUDE_INSTALL_DIR} COMPONENT Devel )
>     
>     Why would you install the contents of ${CMAKE_CURRENT_BINARY_DIR}/kparts ? It contains hardcoded source dir paths. They don't need to though. If srcdir/part.h exists, then write include 'part.h' to that file, not 'srcdir/part.h'. The source dir is already part of the -I passed to the compiler (recall that I wrote before "I think that because the source dir is already in the INTERFACE_INCLUDE_DIRECTORIES of the target"), so no need to hardcode it at all.
>     
>     Actually reading your message again, I think that's what you are asking me. Yes, you want to install the actual headers.

Yes, so we can install the headers either from the macro or from the CMakeLists.txt. If we do it from the CMakeLists.txt files, we'll end up having to list the headers twice.

See updated diff.


- Aleix


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


On Oct. 29, 2013, 10:52 a.m., Aleix Pol Gonzalez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113406/
> -----------------------------------------------------------
> 
> (Updated Oct. 29, 2013, 10:52 a.m.)
> 
> 
> Review request for Build System, KDE Frameworks and Stephen Kelly.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> -------
> 
> Create a macro that will generate the forward headers like we used to, in cmake configure time.
> 
> Here's an example of how we'd port KParts to that macro: http://paste.opensuse.org/9880051
> After the change, these are the installed headers: http://paste.opensuse.org/90980400
> 
> 
> Diffs
> -----
> 
>   modules/ECMGenerateHeaders.cmake PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/113406/diff/
> 
> 
> Testing
> -------
> 
> Ported KParts and still everything works.
> 
> 
> File Attachments
> ----------------
> 
> kparts changes example
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/10/29/a3557579-801b-4ee6-8e3d-5d487428759a__kf5-kparts-headers-test.patch
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20131029/aa5572b6/attachment-0001.html>