viaduck/openssl-cmake

Update target_include_directories

Closed this issue · 1 comments

Hi,

first of all, thank you for your project, that's really a good addition to the community.

I think you may improve the main cmakelists.txt using generator expressions for the last two target_include_directories in this way. You may replace the lines :

    # set include locations
    target_include_directories(ssl BEFORE INTERFACE ${OPENSSL_PREFIX}/usr/local/include)
    target_include_directories(crypto BEFORE INTERFACE ${OPENSSL_PREFIX}/usr/local/include)

with theses ones :

    # set include locations
    target_include_directories(ssl BEFORE INTERFACE $<BUILD_INTERFACE:${OPENSSL_PREFIX}/usr/local/include> )
    target_include_directories(crypto BEFORE INTERFACE $<BUILD_INTERFACE:${OPENSSL_PREFIX}/usr/local/include> )

This will allow projects that include your project to be able to perform install step without complaining about cmake errors like this :

... 
CMake Error in external/openssl-cmake/CMakeLists.txt:
  Target "crypto" INTERFACE_INCLUDE_DIRECTORIES property contains path:

    "/XXX/openssl-cmake/usr/local/include"

  which is prefixed in the build directory.Target "crypto"
  INTERFACE_INCLUDE_DIRECTORIES property contains path:

    "/XXX/openssl-cmake/usr/local/include"

  which is prefixed in the source directory.

Sounds reasonable. However, as soon as we add an install step for openssl-cmake, we should add an expression for the install interface to target_include_directories, too.

Are you going to submit a PR with your proposed changes?