Change CMakeLists.txt in LibreSSL to use target_include_directores

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Change CMakeLists.txt in LibreSSL to use target_include_directores

Cameron Palmer
It is beneficial for projects that depend on LibreSSL libraries and are built with CMake to use target_link_libraries and automatically receive the PUBLIC or INTERFACE headers without needing to specify include_directories. This patch changes the project to use target_include_directories and header scoping.





patch.diff (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Change CMakeLists.txt in LibreSSL to use target_include_directores

Brent Cook
On Thu, May 24, 2018 at 10:10:58AM +0000, Cameron Palmer wrote:
> It is beneficial for projects that depend on LibreSSL libraries and are built with CMake to use target_link_libraries and automatically receive the PUBLIC or INTERFACE headers without needing to specify include_directories. This patch changes the project to use target_include_directories and header scoping.
>

Makes sense. I made some minor fixes and committed to master.

Reply | Threaded
Open this post in threaded view
|

Re: Change CMakeLists.txt in LibreSSL to use target_include_directores

Cameron Palmer
Question about the PUBLIC status of the ../include/compat headers in CMakeLists.txt.

I wrote the target_include_directories calls to include ../include/compat in each of the targets and marked them PUBLIC, but I’m wondering if that will cause conflicts with system headers like time.h and if they should be marked PRIVATE.

With them marked PUBLIC and including ssl or crypto one must add a compiler define like -D HAVE_CLOCK_GETTIME in the linking project to avoid a conflict.

> On 29 May 2018, at 12:48, Brent Cook <[hidden email]> wrote:
>
> On Thu, May 24, 2018 at 10:10:58AM +0000, Cameron Palmer wrote:
>> It is beneficial for projects that depend on LibreSSL libraries and are built with CMake to use target_link_libraries and automatically receive the PUBLIC or INTERFACE headers without needing to specify include_directories. This patch changes the project to use target_include_directories and header scoping.
>>
>
> Makes sense. I made some minor fixes and committed to master.

Reply | Threaded
Open this post in threaded view
|

Re: Change CMakeLists.txt in LibreSSL to use target_include_directores

Brent Cook
You're correct, ​include/compat is intended to ​be private. We will need to
make some tweaks here.

On Mon, Jun 4, 2018 at 5:36 PM, Cameron Palmer <[hidden email]> wrote:

> Question about the PUBLIC status of the ../include/compat headers in
> CMakeLists.txt.
>
> I wrote the target_include_directories calls to include ../include/compat
> in each of the targets and marked them PUBLIC, but I’m wondering if that
> will cause conflicts with system headers like time.h and if they should be
> marked PRIVATE.
>
> With them marked PUBLIC and including ssl or crypto one must add a
> compiler define like -D HAVE_CLOCK_GETTIME in the linking project to avoid
> a conflict.
>
> > On 29 May 2018, at 12:48, Brent Cook <[hidden email]> wrote:
> >
> > On Thu, May 24, 2018 at 10:10:58AM +0000, Cameron Palmer wrote:
> >> It is beneficial for projects that depend on LibreSSL libraries and are
> built with CMake to use target_link_libraries and automatically receive the
> PUBLIC or INTERFACE headers without needing to specify include_directories.
> This patch changes the project to use target_include_directories and header
> scoping.
> >>
> >
> > Makes sense. I made some minor fixes and committed to master.
>
>
Reply | Threaded
Open this post in threaded view
|

Re: Change CMakeLists.txt in LibreSSL to use target_include_directores

Cameron Palmer
I propose some changes in this diff. I move more of cmake calls to target specific calls (target_sources, target_compile_definitions) to clean up the files and reduce their scope.


On 14 Jun 2018, at 04:24, Brent Cook <[hidden email]<mailto:[hidden email]>> wrote:

You're correct, ​include/compat is intended to ​be private. We will need to make some tweaks here.

On Mon, Jun 4, 2018 at 5:36 PM, Cameron Palmer <[hidden email]<mailto:[hidden email]>> wrote:
Question about the PUBLIC status of the ../include/compat headers in CMakeLists.txt.

I wrote the target_include_directories calls to include ../include/compat in each of the targets and marked them PUBLIC, but I’m wondering if that will cause conflicts with system headers like time.h and if they should be marked PRIVATE.

With them marked PUBLIC and including ssl or crypto one must add a compiler define like -D HAVE_CLOCK_GETTIME in the linking project to avoid a conflict.

> On 29 May 2018, at 12:48, Brent Cook <[hidden email]<mailto:[hidden email]>> wrote:
>
> On Thu, May 24, 2018 at 10:10:58AM +0000, Cameron Palmer wrote:
>> It is beneficial for projects that depend on LibreSSL libraries and are built with CMake to use target_link_libraries and automatically receive the PUBLIC or INTERFACE headers without needing to specify include_directories. This patch changes the project to use target_include_directories and header scoping.
>>
>
> Makes sense. I made some minor fixes and committed to master.




target_specific.diff (47K) Download Attachment