eidheim / tiny-process-library

A small platform independent library making it simple to create and stop new processes in C++, as well as writing to stdin and reading from stdout and stderr of a new process
MIT License
338 stars 73 forks source link

Fixes build problem on Windows and makes building examples an option … #26

Closed robiwano closed 6 years ago

robiwano commented 6 years ago

…in CMake, also adds target_include_directories so include dirs get propagated properly to dependent targets.

eidheim commented 6 years ago

Just merged this PR, but made some changes in https://github.com/eidheim/tiny-process-library/commit/83854cd703ba647c9b7a0151e36ef1d40b3e4a33. Let me know if this looks good and work out in your use case. I went with checking CMAKE_SOURCE_DIR against CMAKE_CURRENT_SOURCE_DIR since this was easy to read. But thank you for the stackoverflow link.

eidheim commented 6 years ago

I also removed the install targets if TPL is a sub-project (https://github.com/eidheim/tiny-process-library/commit/91bd0e90debea4fd747aba708d494df34a8ce0f2)

foonathan commented 6 years ago

I don't think that's a good idea: If it's used as a subproject, you might want to install the parent project which also needs to install it.

robiwano commented 6 years ago

I don't agree, I think it's good. Since TPL is a static library, in the case of it being used in a parent project there isn't really anything to install ? If it were a shared library I would agree.

foonathan commented 6 years ago

Yeah, nevermind: as it doesn't export the targets installation isn't useful anyway.

eidheim commented 6 years ago

I also modernised the CMakeLists.txt further here: https://github.com/eidheim/tiny-process-library/commit/5e6d16b3b6080d6cdee2da39b2b6e35042de02be. Let me know if I messed up:)

eidheim commented 6 years ago

I just committed yet another cleanup of CMakeLists.txt in https://github.com/eidheim/tiny-process-library/commit/4c4f1c68e90423e0b43091deeebcdb6c0fc02fb2.