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: Tue, 5 Mar 2019 23:27:20 +0300 [thread overview] Message-ID: <20190305202719.nibah6nchctrns47@tkn_work_nb> (raw) In-Reply-To: <1550951995.54875386@f494.i.mail.ru> 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 ()
next prev parent reply other threads:[~2019-03-05 20:27 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 ` [tarantool-patches] " Alexander Turenko 2019-02-21 10:39 ` [tarantool-patches] Re[2]: [tarantool-patches] [tarantool-patches] " Alexander Tikhonov [not found] ` <1550951995.54875386@f494.i.mail.ru> 2019-03-05 20:27 ` Alexander Turenko [this message] 2019-03-12 7:27 ` [tarantool-patches] Re: [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=20190305202719.nibah6nchctrns47@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