rikyoz / bit7z

A C++ static library offering a clean and simple interface to the 7-zip shared libraries.
https://rikyoz.github.io/bit7z
Mozilla Public License 2.0
623 stars 113 forks source link

CMake wrong target_include_headers scope #59

Closed evilenzo closed 2 years ago

evilenzo commented 2 years ago

bit7z version

3.1.x

7-zip version

v19.00

7-zip DLL used

7za.dll

MSVC version

2019

Architecture

x86_64

Which version of Windows are you using?

Windows 10

Bug description

When built with cmake your lib just doesn't provide the user needed headers. For example, this code just won't compile because it cannot find bitextractor.hpp:

# CMakeLists.txt
cmake_minimum_required(VERSION 3.1)
project(uses_bit7z LANGUAGES CXX)

add_subdirectory(bit7z)
add_executable(uses_bit7z main.cpp)
target_link_libraries(uses_bit7z bit7z64)
// main.cpp
#include "bitextractor.hpp"

int main() {
    return 0;
}

Because of this code in your CMakeLists.txt:

target_include_directories(${TARGET_NAME} PRIVATE 
    ${PROJECT_SOURCE_DIR}/include/
    ${PROJECT_SOURCE_DIR}/lib/7zSDK/CPP/
)

Includes that end user will use must be marked as PUBLIC

Steps to reproduce

Just copy my example and try to build it. Even examples on README.md won't work

Expected behavior

My CMake example must work. Because it is default behaviour for CMake projects

Relevant compilation output

No response

Code of Conduct

evilenzo commented 2 years ago

Done in Pull Request 60

rikyoz commented 2 years ago

Hi! First of all, thank you for the pull request!

When built with cmake your lib just doesn't provide the user needed headers.

Yeah, when the CMakeLists.txt was firstly introduced, it was meant for only separately building the library, not for using the project through the add_subdirectory.

My CMake example must work. Because it is default behaviour for CMake projects

You're right! Actually, I already provided a fix for this (https://github.com/rikyoz/bit7z/commit/49a74d9882fcff365ac6e2a4a07d14ff1c65e52f), which is very similar to yours; however, it will be available only in the next major version, so I'll merge your pull request, allowing users of the current version to use bit7z in CMake projects as intended.