[tarantool-patches] Re: [tarantool-patches] Re: [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: 13124 bytes --]


Fixed Mojave Mac build with MACOSX_DEPLOYMENT_TARGET
environment setup. Specified the minimum version of OS X
on which the target binaries are to be deployed. CMake
uses this value for the make environment and the
-mmacosx-version-min flag to help choose the default SDK

Fixed #3757
---
 .travis.mk | 5 +++++
 .travis.yml | 11 ++++++++---
 cmake/luajit.cmake | 15 ++++++++++++---
 3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/.travis.mk b/.travis.mk
index edd94cd7d..fd93ef1b9 100644
--- a/.travis.mk
+++ b/.travis.mk
@@ -47,6 +47,11 @@ 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 ) && \
+ pip install virtualenv )
 
 test_osx: deps_osx
  cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfoWError ${CMAKE_EXTRA_PARAMS}
diff --git a/.travis.yml b/.travis.yml
index ffe2e8247..8a118a7f2 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -8,7 +8,7 @@ language: cpp
 os: linux
 compiler: gcc
 
-osx_image: xcode9
+osx_image: xcode10.2
 
 cache:
     directories:
@@ -24,11 +24,16 @@ jobs:
       - name: RelWithDebInfoWError build + test (Linux, clang)
         env: TARGET=test
         compiler: clang
- - name: RelWithDebInfoWError build + test (OS X)
+ - name: RelWithDebInfoWError build + test (OS X Mojave 10.14)
         env: TARGET=test
         os: osx
       - name: Debug build + test + coverage (Linux, gcc)
         env: TARGET=coverage
+ - name: RelWithDebInfoWError build + test (OS X High Sierra 10.13)
+ env: TARGET=test
+ os: osx
+ osx_image: xcode9.4
+ if: branch = "2.1"
       - name: LTO build + test (Linux, gcc)
         env: TARGET=test CMAKE_EXTRA_PARAMS=-DENABLE_LTO=ON
         if: branch = "2.1"
@@ -36,7 +41,7 @@ jobs:
         env: TARGET=test CMAKE_EXTRA_PARAMS=-DENABLE_LTO=ON
         if: branch = "2.1"
         compiler: clang
- - name: LTO build + test (OS X)
+ - name: LTO build + test (OS X Mojave 10.14)
         os: osx
         env: TARGET=test CMAKE_EXTRA_PARAMS=-DENABLE_LTO=ON
         if: branch = "2.1"
diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake
index ea4878a93..e84fb21aa 100644
--- a/cmake/luajit.cmake
+++ b/cmake/luajit.cmake
@@ -196,10 +196,19 @@ macro(luajit_build)
         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)
+ execute_process(COMMAND sw_vers -productVersion
+ OUTPUT_VARIABLE PRODUCT_VERSION)
+ message(STATUS "PRODUCT_VERSION=${PRODUCT_VERSION}")
+ if (${PRODUCT_VERSION} VERSION_LESS 10.14)
+ set (luajit_osx_deployment_target 10.6)
+ else ()
+ set (luajit_osx_deployment_target 10.14)
+ endif ()
         else()
             set (luajit_osx_deployment_target ${CMAKE_OSX_DEPLOYMENT_TARGET})
         endif()
+ set(macosx_deployment_target_env MACOSX_DEPLOYMENT_TARGET=${luajit_osx_deployment_target})
+ message(STATUS "${macosx_deployment_target_env}")
         set(luajit_ldflags
             ${luajit_ldflags} -Wl,-macosx_version_min,${luajit_osx_deployment_target})
     endif ()
@@ -238,7 +247,7 @@ macro(luajit_build)
         add_custom_command(OUTPUT ${PROJECT_BINARY_DIR}/third_party/luajit/src/libluajit.a
             WORKING_DIRECTORY ${PROJECT_BINARY_DIR}/third_party/luajit
             COMMAND $(MAKE) ${luajit_buildoptions} clean
- COMMAND $(MAKE) -C src ${luajit_buildoptions} jit/vmdef.lua libluajit.a
+ COMMAND ${macosx_deployment_target_env} $(MAKE) -C src ${luajit_buildoptions} jit/vmdef.lua libluajit.a
             DEPENDS ${CMAKE_SOURCE_DIR}/CMakeCache.txt
         )
     else()
@@ -249,7 +258,7 @@ macro(luajit_build)
             WORKING_DIRECTORY ${PROJECT_BINARY_DIR}/third_party/luajit
             COMMAND ${CMAKE_COMMAND} -E copy_directory ${PROJECT_SOURCE_DIR}/third_party/luajit ${PROJECT_BINARY_DIR}/third_party/luajit
             COMMAND $(MAKE) ${luajit_buildoptions} clean
- COMMAND $(MAKE) -C src ${luajit_buildoptions} jit/vmdef.lua libluajit.a
+ COMMAND ${macosx_deployment_target_env} $(MAKE) -C src ${luajit_buildoptions} jit/vmdef.lua libluajit.a
             DEPENDS ${PROJECT_BINARY_DIR}/CMakeCache.txt ${PROJECT_BINARY_DIR}/third_party/luajit
         )
     endif()
-- 
2.17.1

>Вторник,  5 марта 2019, 23:27 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:
>
>On Sat, Feb 23, 2019 at 10:59:55PM +0300, Alexander Tikhonov wrote:
>> 
>> Hi!
>> 
>> Once more time sorry, that I didn't see your comments from the first time ))
>> 
>> 1) 
>> > It seems that we install pip twice: here and in test_osx.
>> 
>> Right, pip is needed twice:
>>  - for virtualenv to be installed while easy_install is depricated
>>  - inside the virtualenv
>> > I wonder whether all problems are resolved if we just add
>> > pyenv-virtualenv into 'brew install' command in deps_osx?
>> 
>> pyenv-virtualenv into 'brew install' command in deps_osx  can be installed - I'll do it
>> 
>> but the virtualenv on the new OSX versions is not installed by default and it
>> needs additional pip routine installation for 'pip install virtualenv' final install,
>> otherwise check the following bugus build without additional pip install command:
>>  https://travis-ci.org/tarantool/tarantool/jobs/497450467  
>> 
>
>ld: library not found for -lgcc_s.10.4
>clang: error: linker command failed with exit code 1 (use -v to see invocation)
>
>I don't understood how it is related to pip.
>
>I don't understood your description too to be honest. Let's discuss it
>tommorow voicely.
>
>> 2)
>> > 9.4 is not OS X version, it is xcode version. Proposed name:
>> 
>> > RelWithDebInfoWError build + test (OS X 10.13 High Sierra)
>> 
>> > 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).
>> 
>> Right, I'll do your suggestion
>> 
>> > Also consider: Travis-CI added Mojave builds:
>> >   https://blog.travis-ci.com/2019-02-12-xcode-10-2-beta-2-is-now-available
>> 
>> I missed it, thanks!
>> 
>> > Also I still think fix cmakes for Mojave and tweak travis targets should
>> > be in the separate commits.
>> 
>> Right, I'll do it in this way
>> 
>> 3)
>> > Since we set 10.6 for some targets, the comment seems to be still
>> > actual. I don't think we should remove it.
>> Restored back.
>> 
>> 4)
>> > Can we use VERSION_LESS operation to simplify the code?
>> 
>> Right, changed as suggested
>> 
>> > I think only the first 'message' directive is useful, others are
>> > redundant.
>> 
>> Ok, sure
>> 
>> > Why the edge is between 10.12-10.13, while a problem we trying to solve
>> > appears only on 10.14 (Mojave)?
>> 
>> Right, corrected
>> 
>> Patches below to check the changes made on suggestions:
>>  https://github.com/tarantool/tarantool/compare/2.1...e4beb12d7d3e   
>>  https://travis-ci.org/tarantool/tarantool/builds/497561631  
>>  https://travis-ci.org/tarantool/tarantool/builds/497554600  
>> 
>
>I'll paste your commits below as plaintext to comment them per line.
>
>> commit e265d6686761471301a1de2bf0ca7eab157527cc
>> Author: Alexander V. Tikhonov < avtikhon@tarantool.org >
>> Date:   Sun Feb 24 05:18:58 2019 -0500
>> 
>>     travis-ci: OSX version image updated
>> 
>>     Changed the OSX image from 9.4 to 10.2,
>>     added 9.4 version image to 2.1 release criteria
>
>The commit message assumes this a reading person has xcode version ->
>mac os version mapping in a head. 9.4 is not 'OSX image', it is xcode
>version. I think nobody is interested in xcode versions, so it is better
>to use Mac OS version in messages.
>
>I would also say 'CI for 2.1 branch' instead of vague '2.1 release
>criteria'.
>
>Note: 'OS X' should be written with a whitespace as I see.
>
>> 
>> diff --git a/.travis.mk b/.travis.mk
>> index edd94cd7d..35ed2612f 100644
>> --- a/.travis.mk
>> +++ b/.travis.mk
>> @@ -46,7 +46,12 @@ test_ubuntu: deps_ubuntu
>> 
>>  deps_osx:
>>  	brew update
>> -	brew install openssl readline curl icu4c --force
>> +	brew install openssl readline curl icu4c pyenv-virtualenv --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 ) && \
>> +		pip install virtualenv )
>> 
>
>Answered above.
>
>>  test_osx: deps_osx
>>  	cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfoWError ${CMAKE_EXTRA_PARAMS}
>> diff --git a/.travis.yml b/.travis.yml
>> index ffe2e8247..90af044ff 100644
>> --- a/.travis.yml
>> +++ b/.travis.yml
>> @@ -8,7 +8,7 @@ language: cpp
>>  os: linux
>>  compiler: gcc
>> 
>> -osx_image: xcode9
>> +osx_image: xcode10.2
>
>Please, add comment like 'Max OS 10.14'
>
>> 
>>  cache:
>>      directories:
>> @@ -24,11 +24,16 @@ jobs:
>>        - name: RelWithDebInfoWError build + test (Linux, clang)
>>          env: TARGET=test
>>          compiler: clang
>> -      - name: RelWithDebInfoWError build + test (OS X)
>> -        env: TARGET=test
>> +      - name: RelWithDebInfoWError build + test (OS X 10.14 Mojave)
>> +        env: TARGET=test MACOSX_DEPLOYMENT_TARGET=10.14
>
>Other targets are named like 'Ubuntu Trusty (14.04) build + deploy DEB':
>name of a release, then release number. So I think it should be 'OS X
>Mojave 10.14'.
>
>Why do you add MACOSX_DEPLOYMENT_TARGET=10.14 here? As I understand your
>work is about allowing to don't set it.
>
>>          os: osx
>>        - name: Debug build + test + coverage (Linux, gcc)
>>          env: TARGET=coverage
>> +      - name: RelWithDebInfoWError build + test (OS X 10.13 High Sierra)
>> +        env: TARGET=test
>> +        os: osx
>> +        osx_image: xcode9.4
>> +        if: branch = "2.1"
>
>OS X High Sierra 10.13.
>
>>        - name: LTO build + test (Linux, gcc)
>>          env: TARGET=test CMAKE_EXTRA_PARAMS=-DENABLE_LTO=ON
>>          if: branch = "2.1"
>> @@ -36,9 +41,9 @@ jobs:
>>          env: TARGET=test CMAKE_EXTRA_PARAMS=-DENABLE_LTO=ON
>>          if: branch = "2.1"
>>          compiler: clang
>> -      - name: LTO build + test (OS X)
>> +      - name: LTO build + test (OS X 10.14 Mojave)
>>          os: osx
>> -        env: TARGET=test CMAKE_EXTRA_PARAMS=-DENABLE_LTO=ON
>> +        env: TARGET=test CMAKE_EXTRA_PARAMS=-DENABLE_LTO=ON MACOSX_DEPLOYMENT_TARGET=10.14
>
>The same: 'OS X Mojave 10.14' and why MACOSX_DEPLOYMENT_TARGET=10.14 is
>added?
>
>>          if: branch = "2.1"
>>        - name: Create and deploy tarball
>>          env: TARGET=source
>> 
>> commit 0e703e04704de59a29f2f38f2bcb2ec0f028f046
>> Author: Alexander V. Tikhonov < avtikhon@tarantool.org >
>> Date:   Sun Feb 24 02:38:07 2019 -0500
>> 
>>     travis-ci: fixed Mojave mac build
>> 
>>     Fixed Mojave Mac build with CMAKE_OSX_DEPLOYMENT_TARGET.
>>     Specified the minimum version of OS X 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
>> 
>> diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake
>> index ea4878a93..40f2472ef 100644
>> --- a/cmake/luajit.cmake
>> +++ b/cmake/luajit.cmake
>> @@ -196,10 +196,20 @@ macro(luajit_build)
>>          if ("${CMAKE_OSX_DEPLOYMENT_TARGET}" STREQUAL "")
>>              # Default to 10.6 since @rpath support is NOT available in
>>              # earlier versions, needed by AddressSanitizer.
>
>Hm. Now the comment is only partially valid. We need to expand it.
>
>> -            set (luajit_osx_deployment_target 10.6)
>> +            execute_process(COMMAND sw_vers -productVersion
>> +                OUTPUT_VARIABLE PRODUCT_VERSION)
>> +            message(STATUS "PRODUCT_VERSION=${PRODUCT_VERSION}")
>> +            if (${PRODUCT_VERSION} VERSION_LESS 10.14)
>> +                set (luajit_osx_deployment_target 10.6)
>> +            else ()
>> +                set (luajit_osx_deployment_target 10.14)
>> +            endif ()
>> +            set(CMAKE_OSX_DEPLOYMENT_TARGET "${luajit_osx_deployment_target}"
>> +		CACHE STRING "Minimum OS X deployment version")
>
>Please, don't mix whitespaces and tabs.
>
>BTW, why we're set CMAKE_OSX_DEPLOYMENT_TARGET if it is not used
>anywhere else?
>
>>          else()
>>              set (luajit_osx_deployment_target ${CMAKE_OSX_DEPLOYMENT_TARGET})
>>          endif()
>> +        message(STATUS "LUAJIT_OSX_DEPLOYMENT_TARGET=${luajit_osx_deployment_target}")
>>          set(luajit_ldflags
>>              ${luajit_ldflags} -Wl,-macosx_version_min,${luajit_osx_deployment_target})
>>      endif ()
>


-- 
Alexander Tikhonov

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