On 15/12/14 12:59, Tapani Pälli wrote: > Patch adds changes required to use NaCl compiler and libraries to > build Waffle. Build can be configured to use specific version of > the NaCl SDK but currently toolchain is fixed to glibc and target > architecture hardcoded as 64bit x86. > If I understand things correctly, you're hard-coding within the cmake what is essentially the toolchain file. I would kindly ask you to not do that.
> Example command line to configure a build: > > cmake -Dwaffle_has_nacl=ON \ > -Dnacl_sdk_path=/home/tpalli/nacl/nacl_sdk \ > -DCMAKE_BUILD_TYPE=Release \ > . > > Signed-off-by: Tapani Pälli <[email protected]> > --- > Options.cmake | 13 +++++++++++++ > cmake/Modules/WaffleDefineCompilerFlags.cmake | 23 +++++++++++++++++++++++ > cmake/Modules/WaffleDefineInternalOptions.cmake | 4 ++++ > cmake/Modules/WaffleValidateOptions.cmake | 11 +++++++++-- > examples/CMakeLists.txt | 18 ++++++++++-------- > src/CMakeLists.txt | 7 +++++-- > src/waffle/CMakeLists.txt | 10 ++++++++++ > 7 files changed, 74 insertions(+), 12 deletions(-) > > diff --git a/Options.cmake b/Options.cmake > index c316070..a7f4c26 100644 > --- a/Options.cmake > +++ b/Options.cmake > @@ -28,6 +28,19 @@ if(waffle_on_linux) > option(waffle_has_wayland "Build support for Wayland" ${wayland_default}) > option(waffle_has_x11_egl "Build support for X11/EGL" ${x11_egl_default}) > option(waffle_has_gbm "Build support for GBM" ${gbm_default}) > + option(waffle_has_nacl "Build support for NaCl" OFF) > + > + # NaCl specific settings. > + set(nacl_sdk_path "" CACHE STRING "Set nacl_sdk path here") > + set(nacl_version "pepper_39" CACHE STRING "Set NaCl bundle here") > +endif() > + > +# When building for NaCl, disable uncompatible backends. > +if(waffle_has_nacl) > + set(waffle_has_glx OFF) > + set(waffle_has_x11_egl OFF) > + set(waffle_has_gbm OFF) > + set(waffle_has_wayland OFF) > endif() s/uncompatible/incompatible/ Is that incompatibility a temporary or a permanent one ? I was under the impression by replacing the dynamic linking with dlopen/dlsym we essentially make waffle more versatile. Afaict you will need a check in WaffleValidateOptions.cmake as the user can override the values you set in here. > > option(waffle_build_tests "Build tests" ON) > diff --git a/cmake/Modules/WaffleDefineCompilerFlags.cmake > b/cmake/Modules/WaffleDefineCompilerFlags.cmake > index 710b7e0..79d214c 100644 > --- a/cmake/Modules/WaffleDefineCompilerFlags.cmake > +++ b/cmake/Modules/WaffleDefineCompilerFlags.cmake > @@ -122,6 +122,29 @@ if(waffle_on_linux) > add_definitions(-DWAFFLE_HAS_TLS_MODEL_INITIAL_EXEC) > endif() > > + if(waffle_has_nacl) > + add_definitions(-DWAFFLE_HAS_NACL) > + > + # setup architecture and toolchain to use, note that this is > + # currently hardcoded for 64bit x86 with glibc toolchain > + set(nacl_target_arch "x86_64") > + set(nacl_ports "glibc_x86_64") > + set(nacl_toolchain "linux_x86_glibc") > + > + # setup paths for nacl > + set(nacl_root "${nacl_sdk_path}/${nacl_version}") > + set(nacl_toolpath "${nacl_root}/toolchain/${nacl_toolchain}/bin") > + > + # setup compilers from toolchain > + set(CMAKE_C_COMPILER "${nacl_toolpath}/${nacl_target_arch}-nacl-gcc") > + set(CMAKE_CXX_COMPILER > "${nacl_toolpath}/${nacl_target_arch}-nacl-g++") > + > + # setup nacl include and library paths > + set(nacl_INCLUDE_DIRS "${nacl_INCLUDE_DIRS} > ${nacl_sdk_path}/${nacl_version}/include") > + set(nacl_LIBS > "${nacl_sdk_path}/${nacl_version}/lib/${nacl_ports}/${CMAKE_BUILD_TYPE}") > + set(nacl_LDFLAGS "-L${nacl_LIBS} -lppapi_cpp -lppapi -lpthread -ldl") > + endif() > + See the toolchain comment above. Imho adding a toolchain file to git, similar to the Archlinux/Debian build scripts, won't be a bad idea. > add_definitions(-D_XOPEN_SOURCE=600) > endif() > > diff --git a/cmake/Modules/WaffleDefineInternalOptions.cmake > b/cmake/Modules/WaffleDefineInternalOptions.cmake > index 3ef7a25..34dbb61 100644 > --- a/cmake/Modules/WaffleDefineInternalOptions.cmake > +++ b/cmake/Modules/WaffleDefineInternalOptions.cmake > @@ -9,3 +9,7 @@ if(waffle_has_glx OR waffle_has_x11_egl) > else() > set(waffle_has_x11 FALSE) > endif() > + > +if(waffle_has_nacl) > + set(waffle_build_tests FALSE) > +endif() Move this to Options.cmake. > diff --git a/cmake/Modules/WaffleValidateOptions.cmake > b/cmake/Modules/WaffleValidateOptions.cmake > index ea60b0e..69eb34a 100644 > --- a/cmake/Modules/WaffleValidateOptions.cmake > +++ b/cmake/Modules/WaffleValidateOptions.cmake > @@ -46,11 +46,13 @@ endif() > > if(waffle_on_linux) > if(NOT waffle_has_glx AND NOT waffle_has_wayland AND > - NOT waffle_has_x11_egl AND NOT waffle_has_gbm) > + NOT waffle_has_x11_egl AND NOT waffle_has_gbm AND > + NOT waffle_has_nacl) > message(FATAL_ERROR > "Must enable at least one of: " > "waffle_has_glx, waffle_has_wayland, " > - "waffle_has_x11_egl, waffle_has_gbm.") > + "waffle_has_x11_egl, waffle_has_gbm, " > + "waffle_has_nacl.") > endif() > if(waffle_has_gbm) > if(NOT gbm_FOUND) > @@ -122,6 +124,11 @@ if(waffle_on_linux) > message(FATAL_ERROR "x11_egl dependency is missing: > ${x11_egl_missing_deps}") > endif() > endif() > + if(waffle_has_nacl) > + if(NOT EXISTS ${nacl_sdk_path}) > + message(FATAL_ERROR "NaCl SDK path not found : ${nacl_sdk_path}") > + endif() > + endif() Perhaps you can check a header/library's presence and error out. Otherwise thing will blow up, if one makes a typo while setting the variable. Imho you can add an extra check here: if(waffle_build_tests) message(FATAL_ERROR "Building the tests with the NaCl backend is not supported") endif() > elseif(waffle_on_mac) > if(waffle_has_gbm) > message(FATAL_ERROR "Option is not supported on Darwin: > waffle_has_gbm.") > diff --git a/examples/CMakeLists.txt b/examples/CMakeLists.txt > index 281ef47..ede3797 100644 > --- a/examples/CMakeLists.txt > +++ b/examples/CMakeLists.txt > @@ -11,7 +11,7 @@ install( > # Target: simple-x11-egl (executable) > # > ---------------------------------------------------------------------------- > > -if(waffle_on_linux) > +if((waffle_on_linux) AND (waffle_has_x11_egl)) That's a nice independent fix. Maybe split it out ? > add_executable(simple-x11-egl simple-x11-egl.c) > target_link_libraries(simple-x11-egl ${waffle_libname}) > endif() > @@ -20,12 +20,14 @@ endif() > # Target: gl_basic (executable) > # > ---------------------------------------------------------------------------- > > -add_executable(gl_basic gl_basic.c) > -target_link_libraries(gl_basic ${waffle_libname} ${GETOPT_LIBRARIES}) > +if(!waffle_has_nacl) > + add_executable(gl_basic gl_basic.c) > + target_link_libraries(gl_basic ${waffle_libname} ${GETOPT_LIBRARIES}) Adding the following seems shorter/cleaner, imho. if(waffle_has_nacl) return() endif() > > -if(waffle_on_mac) > - set_target_properties(gl_basic > - PROPERTIES > - COMPILE_FLAGS "-ObjC" > - ) > + if(waffle_on_mac) > + set_target_properties(gl_basic > + PROPERTIES > + COMPILE_FLAGS "-ObjC" > + ) > + endif() > endif() > diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt > index aa6d3c3..a47ef10 100644 > --- a/src/CMakeLists.txt > +++ b/src/CMakeLists.txt > @@ -1,6 +1,9 @@ > -if(waffle_build_tests) > +if((waffle_build_tests) AND (!waffle_has_nacl)) > add_subdirectory(waffle_test) > endif() > With the WaffleValidateOptions.cmake change you can drop this. > -add_subdirectory(utils) > +if(!waffle_has_nacl) > + add_subdirectory(utils) > +endif() > + Replace ! with NOT. Optionally add the following in utils/CMakefile.txt if(waffle_has_nacl) return() endif() > add_subdirectory(waffle) > diff --git a/src/waffle/CMakeLists.txt b/src/waffle/CMakeLists.txt > index d76e029..b4dd63f 100644 > --- a/src/waffle/CMakeLists.txt > +++ b/src/waffle/CMakeLists.txt > @@ -25,6 +25,7 @@ include_directories( > ${gl_INCLUDE_DIRS} > ${GLEXT_INCLUDE_DIR} > ${libudev_INCLUDE_DIRS} > + ${nacl_INCLUDE_DIRS} > ${wayland-client_INCLUDE_DIRS} > ${wayland-egl_INCLUDE_DIRS} > ${x11-xcb_INCLUDE_DIRS} > @@ -55,6 +56,11 @@ if(waffle_on_linux) > ${libudev_LDFLAGS} > ) > endif() > + if(waffle_has_nacl) > + list(APPEND waffle_libdeps > + ${nacl_LDFLAGS} > + ) > + endif() > endif() > > set(waffle_sources > @@ -278,6 +284,10 @@ function(add_unittest unittest_name) > return() > endif() > > + if(waffle_has_nacl) > + return() > + endif() > + With the WaffleValidateOptions.cmake change you can drop this hunk. Nicely done Tapani. Must admit that you're work with cmake is way better than my first attempts :P -Emil _______________________________________________ waffle mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/waffle
