Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alexander Turenko <alexander.turenko@tarantool.org>
To: "Alexander V. Tikhonov" <avtikhon@tarantool.org>
Cc: tarantool-patches@freelists.org
Subject: [tarantool-patches] Re: [PATCH v2] travis-ci: fixed Mojave mac build
Date: Thu, 14 Mar 2019 02:44:33 +0300	[thread overview]
Message-ID: <20190313234433.5ttv2dx2yd33t62g@tkn_work_nb> (raw)
In-Reply-To: <a4d0948f8d2d3f7c0f1b6f72705fe75dc5199702.1552401527.git.avtikhon@tarantool.org>

> travis-ci: fixed Mojave mac build

Nit: Use imperative mood in a commit header. See
https://tarantool.io/en/doc/1.10/dev_guide/developer_guidelines/#how-to-write-a-commit-message

The Travis-CI changes looks okay for me.

I have comments re CMake changes, see below. I added proposed changes on
top of your commit. Please, if you are agree, test them and squash.

Then, please, send the next version of the patch to one of maintainers.

WBR, Alexander Turenko.

> Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-3797-mojave-mac-build
> Issue: https://github.com/tarantool/tarantool/issues/3797
 
> --- 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 ()

According to [1] `make MACOSX_DEPLOYMENT_TARGET=10.6` is enough, so we
can get rid of this `if` and just set it always to 10.6.

[1]: https://github.com/LuaJIT/LuaJIT/issues/484

>          else()
>              set (luajit_osx_deployment_target ${CMAKE_OSX_DEPLOYMENT_TARGET})
>          endif()
> +        set(macosx_deployment_target_env MACOSX_DEPLOYMENT_TARGET=${luajit_osx_deployment_target})

We can add MACOSX_DEPLOYMENT_TARGET="${luajit_osx_deployment_target}" into
set(luajit_buildoptions ...) below instead.

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

No need to set provide env if we'll add it MACOSX_DEPLOYMENT_TARGET to
${luajit_buildoptions}. It looks simpler.

>              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

The same.

>              DEPENDS ${PROJECT_BINARY_DIR}/CMakeCache.txt ${PROJECT_BINARY_DIR}/third_party/luajit
>          )
>      endif()
> -- 
> 2.17.1
> 

      reply	other threads:[~2019-03-13 23:44 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-12 14:40 [tarantool-patches] " Alexander V. Tikhonov
2019-03-13 23:44 ` Alexander Turenko [this message]

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=20190313234433.5ttv2dx2yd33t62g@tkn_work_nb \
    --to=alexander.turenko@tarantool.org \
    --cc=avtikhon@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH v2] travis-ci: fixed Mojave mac build' \
    /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