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

[Emil Velikov]

On 22/12/14 22:28, Chad Versace wrote:
> On 12/21/2014 07:53 AM, 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 ?
> 
> Emil, I don't understand. What do you mean that nacl_sdk defines NACL_SDK_ROOT?
> I see that some GNU Makefiles in the SDK define NACL_SDK_ROOT, but I failed
> to find any CMake files in the SDK.
> 
Admittedly I've not fetched/tried the SDK, but I would assume that this
commits [1] sets the env var. On the plus side it seems that it works
for some (again guesswork) [2]. And the bugreport about the missing way
to autodetect the SDK [3].

> I also think that the CMake configure line can be improved and the toolchain files
> simplified. I'll suggest those changes in a different subthread, though.
>  
Fully agreed. My last(i.e. this) round of nitpicking on the series is
mostly as jfyi/nice to have, rather than anything else.

>>>       -DCMAKE_TOOLCHAIN_FILE=cmake/toolchain-nacl-linux-glibc-x86_64.cmake \
>>>       -DCMAKE_BUILD_TYPE=Release \
>>>       .
> 
>>> 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 see a way for Waffle's CMake to autodetect the nacl_sdk_path, because the
> NaCl SDK is usually (I think) installed by the user into his home directory in a custom location.
> For example:
>   Tapani: ~/nacl/nacl_sdk
>   Chad:   ~/opt/nacl_sdk
>   Emil:   ????
> 
> I admit that I'm a complete NaCl newbie, though. Emil, do you have some plan
> on how to perform autodetection?
> 
If we can set/use NACL_SDK_ROOT, just check for a header/library that is
required and unlikely to be present on the host (to prevent false
positives). I.e. I'm thinking about find_path/find_library.

Cheers,
Emil


[1] https://code.google.com/p/nativeclient-sdk/source/detail?r=1385
[2]
https://github.com/seichter/CMake-Toolchain-Collection/blob/master/toolchain-nacl.cmake
[3] https://code.google.com/p/nativeclient-sdk/issues/detail?id=128