From: Alexander Turenko <alexander.turenko@tarantool.org> To: Alexander Tikhonov <avtikhon@tarantool.org> Cc: tarantool-patches@freelists.org Subject: [tarantool-patches] Re: [PATCH 1/1] travis-ci: fixed Mojave mac build with CMAKE_OSX_DEPLOYMENT_TARGET Date: Wed, 20 Feb 2019 17:58:27 +0300 [thread overview] Message-ID: <20190220145826.4leayus7oo6b4iom@tkn_work_nb> (raw) In-Reply-To: <1550585135.878144114@f412.i.mail.ru> 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
next prev parent reply other threads:[~2019-02-20 14:58 UTC|newest] Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-02-19 14:05 [tarantool-patches] [tarantool-patches] " Alexander Tikhonov 2019-02-20 14:58 ` Alexander Turenko [this message] 2019-02-21 10:39 ` [tarantool-patches] Re[2]: " Alexander Tikhonov [not found] ` <1550951995.54875386@f494.i.mail.ru> 2019-03-05 20:27 ` [tarantool-patches] " Alexander Turenko 2019-03-12 7:27 ` [tarantool-patches] " Alexander Tikhonov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190220145826.4leayus7oo6b4iom@tkn_work_nb \ --to=alexander.turenko@tarantool.org \ --cc=avtikhon@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH 1/1] travis-ci: fixed Mojave mac build with CMAKE_OSX_DEPLOYMENT_TARGET' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox