PLSysSec / rlbox

RLBox sandboxing framework
https://rlbox.dev
MIT License
287 stars 21 forks source link

Add install feature in CMake #29

Closed bansan85 closed 3 years ago

bansan85 commented 3 years ago

Add feature to install header via make install.

I also change:

Example of find_package:

cmake_minimum_required(VERSION 3.10)
project(zpipe CXX)
find_package(rlbox-sandboxing-api CONFIG REQUIRED)
add_executable(zpipe main.cpp mylib.c)
target_link_libraries(zpipe rlbox-sandboxing-api)

This is for this feature that I propose to add an option to not build tests and avoid catch2 dependency #28.

shravanrn commented 3 years ago

I'm reviewing this now

shravanrn commented 3 years ago

Thanks for submitting this! Looks good overall (couple of minor comments). I just want to test this in Windows & Linux & Mac before landing this. I'll try to do this over the next 24 hours.

shravanrn commented 3 years ago

@bansan85 Could you post a test of how to use the cmake installed files. Here is what I have so far, but this doesn't seem to work

cmake_minimum_required(VERSION 3.13)

project(rlbox_cmake_test
        VERSION 0.1
        DESCRIPTION "RLBox rlbox_cmake_test")

find_package(rlbox-sandboxing-api)
if(NOT rlbox-sandboxing-api_FOUND)
    message(FATAL_ERROR "NO rlbox-sandboxing-api_FOUND")
endif()

add_executable(testt test.cpp)
target_link_libraries(testt rlbox-sandboxing-api)
bansan85 commented 3 years ago

cmake example added :)

shravanrn commented 3 years ago

cmake example added :)

Ah excellent :+1:! This looks great! I'll run a quick test on Windows/Mac/Linux in the next 24 hours, and we should be able to merge it soon

bansan85 commented 3 years ago

Thanks. About the workaround, I will try some tests to understand the bug. Maybe add a warning to detect if include(GNUInstallDirs) was already called before your call.

bansan85 commented 3 years ago

I made a try for the bug about GNUInstallDirs with the following file:

cmake_minimum_required(VERSION 3.13)

project(rlbox_cmake_test
        VERSION 0.1
        DESCRIPTION "RLBox rlbox_cmake_test")

include(GNUInstallDirs)

add_subdirectory(rlbox_sandboxing_api)

add_library(lib mylib.c)

add_executable(testt main.cpp)
target_link_libraries(testt rlbox-sandboxing-api lib)

install(FILES ${CMAKE_CURRENT_BINARY_DIR}/testt
        DESTINATION
        ${CMAKE_INSTALL_DOCDIR}/${PROJECT_NAME}/TTT)

when I make a make install, I don't have problem with ${PROJECT_NAME} but it also install header from rlbox:

-- Install configuration: "Release"
-- Installing: /tmp/ttt2/usr/local/share/doc/rlbox_cmake_test/rlbox_cmake_test/TTT/testt
-- Up-to-date: /tmp/ttt2/usr/local/include/rlbox/rlbox.hpp
-- Up-to-date: /tmp/ttt2/usr/local/include/rlbox/rlbox_app_pointer.hpp
-- Up-to-date: /tmp/ttt2/usr/local/include/rlbox/rlbox_conversion.hpp
-- Up-to-date: /tmp/ttt2/usr/local/include/rlbox/rlbox_dylib_sandbox.hpp
-- Up-to-date: /tmp/ttt2/usr/local/include/rlbox/rlbox_helpers.hpp
-- Up-to-date: /tmp/ttt2/usr/local/include/rlbox/rlbox_noop_sandbox.hpp
-- Up-to-date: /tmp/ttt2/usr/local/include/rlbox/rlbox_policy_types.hpp
-- Up-to-date: /tmp/ttt2/usr/local/include/rlbox/rlbox_range.hpp
-- Up-to-date: /tmp/ttt2/usr/local/include/rlbox/rlbox_sandbox.hpp
-- Up-to-date: /tmp/ttt2/usr/local/include/rlbox/rlbox_stdlib.hpp
-- Up-to-date: /tmp/ttt2/usr/local/include/rlbox/rlbox_stdlib_polyfill.hpp
-- Up-to-date: /tmp/ttt2/usr/local/include/rlbox/rlbox_struct_support.hpp
-- Up-to-date: /tmp/ttt2/usr/local/include/rlbox/rlbox_type_traits.hpp
-- Up-to-date: /tmp/ttt2/usr/local/include/rlbox/rlbox_types.hpp
-- Up-to-date: /tmp/ttt2/usr/local/include/rlbox/rlbox_unwrap.hpp
-- Up-to-date: /tmp/ttt2/usr/local/include/rlbox/rlbox_wrapper_traits.hpp
-- Up-to-date: /tmp/ttt2/usr/local/share/cmake/rlbox-sandboxing-api/rlbox-sandboxing-apiTargets.cmake
-- Up-to-date: /tmp/ttt2/usr/local/share/cmake/rlbox-sandboxing-api/rlbox-sandboxing-apiConfig.cmake
-- Up-to-date: /tmp/ttt2/usr/local/share/cmake/rlbox-sandboxing-api/rlbox-sandboxing-apiConfigVersion.cmake

Any way, I don't think it's a good idea to add_subdirectory a 3rd-party library.

shravanrn commented 3 years ago

Sorry took me a while to get to this. Confirmed that this works fine on Linux and Mac. Confirmed that cmake install doesn't do anything on the MSVC generator on Windows. I haven't checked to see what the Ninja generator does on windows, but this is probable fine.