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

[Tapani Pälli]

On 12/30/2014 12:53 AM, Emil Velikov wrote:
> 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].

This sure looks like a 'development shell' which would set it, I don't 
know if this is used though because I cannot find it in the SDK or 
referenced by the 'Getting started' tutorials, I'm using the SDK from here:

https://developer.chrome.com/native-client/sdk/download

Maybe they had it but it was removed or optional to have (?) Current 
instructions suggest that building is done in the users environment 
(regular shell).


>> 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
>