[waffle] [PATCH v2 2/4] waffle: add support for building Waffle using NaCl toolchain

Tapani Pälli tapani.palli at intel.com
Sun Dec 28 23:47:45 PST 2014



On 12/21/2014 03:53 PM, Emil Velikov wrote:
> On 17 December 2014 at 10:17, Tapani Pälli <tapani.palli at intel.com> 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, toolchain for the build needs to be selected with
>> cmake variable CMAKE_TOOLCHAIN_FILE.
>>
>> Example command line to configure a build:
>>
>> cmake -Dwaffle_has_nacl=ON \
>>        -Dnacl_sdk_path=/home/tpalli/nacl/nacl_sdk \
>>        -Dnacl_version=pepper_39 \
> Tbh I'm not a huge fan of having those piped via the top cmake into
> the toolchain file.
>
> Quick look indicates that nacl_sdk as of Dec 2011 has/is defining NACL_SDL_ROOT.
> Perhaps we can reuse that one and error out if it's missing.
> Considering we can use (have tested only) a single version we could
> just hardcode it into the toolchain file ?

Problem is that this path is not in environment and user can choose to 
install to whatever location. I've tested 37, 38 and 39 which is the 
latest stable version. It seems there are new versions coming quite 
often (?) and there are unstable versions available for the cool guys so 
I would not hardcode it.


>>        -DCMAKE_TOOLCHAIN_FILE=cmake/toolchain-nacl-linux-glibc-x86_64.cmake \
>>        -DCMAKE_BUILD_TYPE=Release \
>>        .
>>
>> v2: cmake fixes, create toolchain files for nacl (Emil Velikov)
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>   Options.cmake                                 |  5 +++++
>>   cmake/Modules/WaffleDefineCompilerFlags.cmake |  4 ++++
>>   cmake/Modules/WaffleValidateOptions.cmake     | 31 +++++++++++++++++++++++++--
>>   cmake/toolchain-nacl-linux-glibc-x86_32.cmake | 29 +++++++++++++++++++++++++
>>   cmake/toolchain-nacl-linux-glibc-x86_64.cmake | 29 +++++++++++++++++++++++++
>>   examples/CMakeLists.txt                       |  4 ++++
>>   src/utils/CMakeLists.txt                      |  4 ++++
>>   src/waffle/CMakeLists.txt                     |  6 ++++++
>>   8 files changed, 110 insertions(+), 2 deletions(-)
>>   create mode 100644 cmake/toolchain-nacl-linux-glibc-x86_32.cmake
>>   create mode 100644 cmake/toolchain-nacl-linux-glibc-x86_64.cmake
>>
>> diff --git a/Options.cmake b/Options.cmake
>> index c316070..4f097a0 100644
>> --- a/Options.cmake
>> +++ b/Options.cmake
>> @@ -28,6 +28,11 @@ 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()
>>
>>   option(waffle_build_tests "Build tests" ON)
>> diff --git a/cmake/Modules/WaffleDefineCompilerFlags.cmake b/cmake/Modules/WaffleDefineCompilerFlags.cmake
>> index 710b7e0..4bdc0df 100644
>> --- a/cmake/Modules/WaffleDefineCompilerFlags.cmake
>> +++ b/cmake/Modules/WaffleDefineCompilerFlags.cmake
>> @@ -122,6 +122,10 @@ if(waffle_on_linux)
>>           add_definitions(-DWAFFLE_HAS_TLS_MODEL_INITIAL_EXEC)
>>       endif()
>>
>> +    if(waffle_has_nacl)
>> +        add_definitions(-DWAFFLE_HAS_NACL)
>> +    endif()
>> +
>>       add_definitions(-D_XOPEN_SOURCE=600)
>>   endif()
>>
>> diff --git a/cmake/Modules/WaffleValidateOptions.cmake b/cmake/Modules/WaffleValidateOptions.cmake
>> index ea60b0e..1275463 100644
>> --- a/cmake/Modules/WaffleValidateOptions.cmake
>> +++ b/cmake/Modules/WaffleValidateOptions.cmake
>> @@ -46,11 +46,38 @@ 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_nacl)
>> +        if(NOT EXISTS ${nacl_sdk_path})
>> +            message(FATAL_ERROR "NaCl SDK path not found : ${nacl_sdk_path}")
>> +        endif()
>> +
> In the future as we add a proper build check, we can add it into the
> auto-detection. Can you add a single line of comment ?

I don't believe auto-detection would be good idea in this case.

>> +        if(NOT EXISTS ${CMAKE_TOOLCHAIN_FILE})
>> +            message(FATAL_ERROR "Toolchain for Nacl not found. This must be "
>> +                                "configured using CMAKE_TOOLCHAIN_FILE.")
>> +        endif()
>> +
> Don't think we need this check, but I'll leave it up-to Chad to be the
> final judge.

This is fine for me, I left it there to have a bit more verbose output 
for the user as without it things blow up and it may not be clear for 
user that the file should be specified for compilation to work.

>> +        # Warn the user that building tests is disabled.
>> +        if(waffle_build_tests)
>> +            message(WARNING "Building the tests with the NaCl backend "
>> +                            "is not supported, skipping tests.")
>> +            set(waffle_build_tests OFF)
>> +        endif()
>> +
>> +        # When building for NaCl, disable incompatible backends.
>> +        set(waffle_has_gbm OFF)
>> +        set(waffle_has_egl OFF)
>> +        set(waffle_has_glx OFF)
>> +        set(waffle_has_x11 OFF)
>> +        set(waffle_has_x11_egl OFF)
>> +        set(waffle_has_wayland OFF)
> The platform supports dlopen/dlsym, so I'm quite confident that we can
> get it working.
> That will obviously can be looked at a later stage. Can you add a
> small TODO line ?

Only the glibc version supports dlopen, pnacl does not. Also I don't see 
the use case of calling waffle from javascript to open a Wayland window? 
I don't think Chrome security wise would allow such to happen. The 
realistic use case is to show content on the browser window. Also the 
resulted waffle binary can not be expected to work in any other system 
than within chrome.

>>       endif()
>>       if(waffle_has_gbm)
>>           if(NOT gbm_FOUND)
>> diff --git a/cmake/toolchain-nacl-linux-glibc-x86_32.cmake b/cmake/toolchain-nacl-linux-glibc-x86_32.cmake
>> new file mode 100644
>> index 0000000..3c13bb6
>> --- /dev/null
>> +++ b/cmake/toolchain-nacl-linux-glibc-x86_32.cmake
>> @@ -0,0 +1,29 @@
>> +#
>> +# NaCl toolchain file for 32bit x86 using glibc C library
>> +#
>> +
>> +set(CMAKE_SYSTEM_NAME Linux)
>> +
>> +set(nacl_target_arch "i686")
>> +set(nacl_ports "glibc_x86_32")
>> +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++)
>> +
>> +set(CMAKE_FIND_ROOT_PATH ${nacl_root})
>> +
>> +# for FIND_PROGRAM|LIBRARY|INCLUDE use ${nacl_root} only
>> +set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM ONLY)
>> +set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY)
>> +set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY)
>> +
>> +# setup nacl includes and required libraries
>> +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)
> Are those four the bare minimum required for any NaCl project ?
> I'm a huge fan of "include it (link against it) only when needed".
> I.e. add them if the linked complains :-P
>
> Either way, can you please use the following style. Imho it makes
> things easer to read
> set(nacl_LDFLAGS
>      -L${nacl_LIBS}
>      -lppapi_cpp
>      -lppapi
>      -lpthread
>      -ldl
>      )

This *should* be bare minimum, I will double check it and use multiple 
lines. Thanks!


> Similar comments for the second toolchain file.
>
> Thanks
> Emil
> _______________________________________________
> waffle mailing list
> waffle at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/waffle
>


More information about the waffle mailing list