[tarantool-patches] Re[2]: [tarantool-patches] [tarantool-patches] [PATCH 1/1] travis-ci: fixed Mojave mac build with CMAKE_OSX_DEPLOYMENT_TARGET

["Alexander Tikhonov" <avtikhon@tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 9791 bytes --]

travis-ci: added fix for mojave mac build
    
Corrected the minimum version of OSX image on which the target
binaries are to be deployed. CMake uses this value for the
-mmacosx-version-min flag and to help choose the default SDK.
The CMAKE_OSX_DEPLOYMENT_TARGET depends on DARWIN_VERSION:
DARWIN_VERSION <= 10.12 -> CMAKE_OSX_DEPLOYMENT_TARGET = 10.6
DARWIN_VERSION >= 10.13 -> CMAKE_OSX_DEPLOYMENT_TARGET = 10.14

Fixes #3797
---
Travis-CI:  https://travis-ci.org/tarantool/tarantool/builds/496394051  
Branch: remotes/origin/avtikhon/gh-3797-fix-target-mojave-mac-build

 cmake/luajit.cmake | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake
index ea4878a93..d2253a0f9 100644
--- a/cmake/luajit.cmake
+++ b/cmake/luajit.cmake
@@ -194,9 +194,28 @@ macro(luajit_build)
         endif()
         # Pass deployment target
         if ("${CMAKE_OSX_DEPLOYMENT_TARGET}" STREQUAL "")
-            # Default to 10.6 since @rpath support is NOT available in
-            # earlier versions, needed by AddressSanitizer.
-            set (luajit_osx_deployment_target 10.6)
+            # DARWIN_VERSION <= 10.12 -> CMAKE_OSX_DEPLOYMENT_TARGET = 10.6
+            # DARWIN_VERSION >= 10.13 -> CMAKE_OSX_DEPLOYMENT_TARGET = 10.14
+            execute_process(COMMAND sw_vers -productVersion
+                OUTPUT_VARIABLE PRODUCT_VERSION)
+            message(STATUS "PRODUCT_VERSION=${PRODUCT_VERSION}")
+            string(REGEX MATCH "[0-9]+.[0-9]+" DARWIN_VERSION ${PRODUCT_VERSION})
+            message(STATUS "DARWIN_VERSION=${DARWIN_VERSION}")
+            string(REGEX MATCH "^[0-9]+" MAJOR_VERSION ${DARWIN_VERSION})
+            message(STATUS "MAJOR_VERSION=${MAJOR_VERSION}")
+            if (MAJOR_VERSION GREATER 10)
+                set (luajit_osx_deployment_target 10.14)
+            elseif (MAJOR_VERSION LESS 10)
+                set (luajit_osx_deployment_target 10.6)
+            elseif (MAJOR_VERSION EQUAL 10)
+                string(REGEX MATCH "[0-9]+$" MINOR_VERSION ${DARWIN_VERSION})
+                message(STATUS "MINOR_VERSION=${MINOR_VERSION}")
+                if (MINOR_VERSION GREATER 12)
+                    set (luajit_osx_deployment_target 10.14)
+                else ()
+                    set (luajit_osx_deployment_target 10.6)
+                endif ()
+            endif ()
         else()
             set (luajit_osx_deployment_target ${CMAKE_OSX_DEPLOYMENT_TARGET})
         endif()
-- 
2.17.1


>Среда, 20 февраля 2019, 17:58 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:
>
>Hi!
>
>Common notes:
>
>* Please, squash your patches before send (into 1, 2, several meaningful
>  patches, w/o temporary commits).
>* Your branch is named
>  remotes/origin/avtikhon/gh-3797-fix-mojave-mac-build (with extra
>  'remotes/origin/' prefix).
>* Please, add me to 'To' or 'CC' header when ask a review from me, so
>  I'll not miss it.
>* Work hard to fit a commit message header within 50 symbols and a
>  commit message within 72 symbols (see our guidelines).
>
>Other comments are below.
>
>Feel free to engage me into discussions / investigations if needed.
>
>WBR, Alexander Turenko.
>
>On Tue, Feb 19, 2019 at 05:05:35PM +0300, Alexander Tikhonov wrote:
>> 
>> 1. Changed the Travis-CI testing default OSX image from 'xcode9' to 'xcode10.1',
>> also added Travis-CI's default 'xcode9.4' image to the '2.1' branch testing.
>> 2. Corrected virtualenv setup for OSX images where it is not installed by default.
>> 3. Corrected the minimum version of OSX image on which the target binaries are to be
>> deployed. CMake uses this value for the -mmacosx-version-min flag and to help choose the
>> default SDK. The CMAKE_OSX_DEPLOYMENT_TARGET depends on DARWIN_VERSION:
>> DARWIN_VERSION <= 10.12 -> CMAKE_OSX_DEPLOYMENT_TARGET = 10.6
>> DARWIN_VERSION >= 10.13 -> CMAKE_OSX_DEPLOYMENT_TARGET = 10.14
>> 
>> Closes #3797
>> ---
>> Travis-CI:  https://travis-ci.org/tarantool/tarantool/builds/495448673  
>> Travis-CI(checked all OSX):  https://travis-ci.org/tarantool/tarantool/builds/495415195  
>> Branch:  https://github.com/tarantool/tarantool/tree/remotes/origin/avtikhon/gh-3797-fix-mojave-mac-build
>> 
>> diff --git a/.travis.mk b/.travis.mk
>> index edd94cd7d..181bab438 100644
>> --- a/.travis.mk
>> +++ b/.travis.mk
>> @@ -47,6 +47,10 @@ test_ubuntu: deps_ubuntu
>>  deps_osx:
>>      brew update
>>      brew install openssl readline curl icu4c --force
>> +    virtualenv -h >/dev/null 2>&1 || \
>> +        ( pip -h >/dev/null 2>&1 || ( curl --silent --show-error --retry 5  https://bootstrap.pypa.io/get-pip.py | python && pip --version ) && \
>> +        brew install pyenv-virtualenv --force && \
>> +        pip install virtualenv )
>
>It seems that we install pip twice: here and in test_osx.
>
>I wonder whether all problems are resolved if we just add
>pyenv-virtualenv into 'brew install' command in deps_osx?
>
>>  
>>  test_osx: deps_osx
>>      cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfoWError ${CMAKE_EXTRA_PARAMS}
>> diff --git a/.travis.yml b/.travis.yml
>> index ffe2e8247..e03bd3185 100644
>> --- a/.travis.yml
>> +++ b/.travis.yml
>> @@ -8,7 +8,7 @@ language: cpp
>>  os: linux
>>  compiler: gcc
>>  
>> -osx_image: xcode9
>> +osx_image: xcode10.1
>>  
>>  cache:
>>      directories:
>> @@ -27,6 +27,11 @@ jobs:
>>        - name: RelWithDebInfoWError build + test (OS X)
>>          env: TARGET=test
>>          os: osx
>> +      - name: RelWithDebInfoWError build + test (OS X 9.4)
>> +        env: TARGET=test
>> +        os: osx
>> +        osx_image: xcode9.4
>> +        if: branch = "2.1"
>
>9.4 is not OS X version, it is xcode version. Proposed name:
>
>RelWithDebInfoWError build + test (OS X 10.13 High Sierra)
>
>See the following links to match os_image values into OS X versions and
>names:
>
>https://docs.travis-ci.com/user/reference/osx/#macos-version
>https://en.wikipedia.org/wiki/MacOS_version_history#toc
>
>Now I see both versions are High Sierra. I don't see a reason to have
>two target of the same Mac OS versions (even despite different xcode
>versions).
>
>Also consider: Travis-CI added Mojave builds:
>https://blog.travis-ci.com/2019-02-12-xcode-10-2-beta-2-is-now-available
>
>Also I still think fix cmakes for Mojave and tweak travis targets should
>be in the separate commits.
>
>>        - name: Debug build + test + coverage (Linux, gcc)
>>          env: TARGET=coverage
>>        - name: LTO build + test (Linux, gcc)
>> diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake
>> index ea4878a93..d2253a0f9 100644
>> --- a/cmake/luajit.cmake
>> +++ b/cmake/luajit.cmake
>> @@ -194,9 +194,28 @@ macro(luajit_build)
>>          endif()
>>          # Pass deployment target
>>          if ("${CMAKE_OSX_DEPLOYMENT_TARGET}" STREQUAL "")
>> -            # Default to 10.6 since @rpath support is NOT available in
>> -            # earlier versions, needed by AddressSanitizer.
>> -            set (luajit_osx_deployment_target 10.6)
>
>Since we set 10.6 for some targets, the comment seems to be still
>actual. I don't think we should remove it.
>
>> +            # DARWIN_VERSION <= 10.12 -> CMAKE_OSX_DEPLOYMENT_TARGET = 10.6
>> +            # DARWIN_VERSION >= 10.13 -> CMAKE_OSX_DEPLOYMENT_TARGET = 10.14
>> +            execute_process(COMMAND sw_vers -productVersion
>> +                OUTPUT_VARIABLE PRODUCT_VERSION)
>> +            message(STATUS "PRODUCT_VERSION=${PRODUCT_VERSION}")
>> +            string(REGEX MATCH "[0-9]+.[0-9]+" DARWIN_VERSION ${PRODUCT_VERSION})
>> +            message(STATUS "DARWIN_VERSION=${DARWIN_VERSION}")
>> +            string(REGEX MATCH "^[0-9]+" MAJOR_VERSION ${DARWIN_VERSION})
>> +            message(STATUS "MAJOR_VERSION=${MAJOR_VERSION}")
>> +            if (MAJOR_VERSION GREATER 10)
>> +                set (luajit_osx_deployment_target 10.14)
>> +            elseif (MAJOR_VERSION LESS 10)
>> +                set (luajit_osx_deployment_target 10.6)
>> +            elseif (MAJOR_VERSION EQUAL 10)
>> +                string(REGEX MATCH "[0-9]+$" MINOR_VERSION ${DARWIN_VERSION})
>> +                message(STATUS "MINOR_VERSION=${MINOR_VERSION}")
>> +                if (MINOR_VERSION GREATER 12)
>> +                    set (luajit_osx_deployment_target 10.14)
>> +                else ()
>> +                    set (luajit_osx_deployment_target 10.6)
>> +                endif ()
>> +            endif ()
>
>Can we use VERSION_LESS operation to simplify the code?
>
>I think only the first 'message' directive is useful, others are
>redundant.
>
>Why the edge is between 10.12-10.13, while a problem we trying to solve
>appears only on 10.14 (Mojave)?
>
>>          else()
>>              set (luajit_osx_deployment_target ${CMAKE_OSX_DEPLOYMENT_TARGET})
>>          endif()
>> 
>> 
>> -- 
>> Alexander Tikhonov
>


-- 
Alexander Tikhonov

[-- Attachment #2: Type: text/html, Size: 15758 bytes --]