From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 821706F87A; Wed, 28 Apr 2021 12:31:22 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 821706F87A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1619602282; bh=ShNkEQe1RPrrWeBLPqyO90PJW0mfOkKL0POQTYUkPa0=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=KsVEyvGYDcdB3J+BkosgFBp5XJjyYunkj4Iu9h4y5+KgdvNXJMfabfQHfwOrgTQUX lOtUbpFE7eGT5BmOvbMV83mQWExHLzchhppjbCUWS+++Q67D8HI9A1MuuO+XYFXscW YJiyti8CUNEJ4FdV+gNH/Y8jeHwRn9y31mlMpCE4= Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 4BF506F87A for ; Wed, 28 Apr 2021 12:31:21 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 4BF506F87A Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1lbgXO-0004qY-H1; Wed, 28 Apr 2021 12:31:19 +0300 Date: Wed, 28 Apr 2021 12:10:07 +0300 To: Sergey Kaplun Message-ID: <20210428091007.GA24014@tarantool.org> References: <5465d7bc81f04842dcdb65f222db381036c0659c.1619561872.git.imun@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.10.1 (2018-07-13) X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9ECFD8CE5F059401082903BCA05D22506BBA2EDD66B931DE4182A05F5380850407D05980A1AB46E038077E674D208BDC8FC1B2D813521D93A265DE1EC4FBE4FEC X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7F544D30F1A6FA191EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AA32F0A5ADCF96E68638F802B75D45FF914D58D5BE9E6BC1A93B80C6DEB9DEE97C6FB206A91F05B264058118AD5F910CF2CF09FB201FD1B6C26CFBAC0749D213D2E47CDBA5A96583C09775C1D3CA48CFCF36E64A7E3F8E58117882F4460429724CE54428C33FAD30A8DF7F3B2552694AC26CFBAC0749D213D2E47CDBA5A9658378DA827A17800CE74A95F4E53E8DCE969FA2833FD35BB23DF004C906525384302BEBFE083D3B9BA71A620F70A64A45A98AA50765F79006372E808ACE2090B5E1725E5C173C3A84C3C5EA940A35A165FF2DBA43225CD8A89F616AD31D0D18CD5C35872C767BF85DA2F004C90652538430E4A6367B16DE6309 X-C1DE0DAB: 0D63561A33F958A550E634272195D68EDBDB6DCCDA1545A2E9233BFB62529BB3D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7502E6951B79FF9A3F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D340D3A19269BBEBAAF00C770B7380150E5FDB3C2CD54F7291D7A554F21253429483A6283878DFBC3811D7E09C32AA3244CC8A018A3D16EED8505C02079912EEC8733C9DC155518937F8D5DD81C2BAB7D1D X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojoCaqxM2e5sri66bo5G37iw== X-Mailru-Sender: 689FA8AB762F73936BC43F508A063822393329EEDAA78B40F53FE77DADA2C987A7C8D0F45F857DBFE9F1EFEE2F478337FB559BB5D741EB964C8C2C849690F8E70A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH] build: fix linker flags for executable on MacOS X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Igor Munkin via Tarantool-patches Reply-To: Igor Munkin Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Sergey, Thanks for your review! On 28.04.21, Sergey Kaplun wrote: > > Hi, Igor! > > Thanks for the patch! > LGTM, except a few nits below. Added your tag: | Reviewed-by: Sergey Kaplun > > On 28.04.21, Igor Munkin wrote: > > This patch fixes inaccuracy in Tarantool build configuration introduced > > by commit 07c83aab5c066ca75c149112b331b4dbb81b3f38 ('build: adjust > > LuaJIT build system'). All those MacOS-related tweaks for __PAGEZERO > > size and preferred load address for the bundle are necessary only for > > builds with 32-bit GC area on 64-bit host. The only case fitting these > > conditions is x86_64 with no LUAJIT_ENABLE_GC64. All other 64-bit builds > > use 64-bit GC area unconditionally. > > > > Part of #5983 > > Needed for #5629 > > Follows up #4862 > > > > Signed-off-by: Igor Munkin > > --- > > > > This patch partially fixes the build on M1. I tested it on tntmac07 > > alongside with the changes Nikita made for libcoro[1]. As a result > > Tarantool has been successfully built, but fails to start. CI looks to > > be OK[2] except the known problems with ASAN[3]. > > > > Issue: https://github.com/tarantool/tarantool/issues/5983 > > Branch: https://github.com/tarantool/tarantool/tree/imun/gh-5983-fix-build-on-m1 > > > > [1]: https://github.com/tarantool/tarantool/commit/309ce38 > > [2]: https://github.com/tarantool/tarantool/commit/5465d7b > > [3]: https://github.com/tarantool/tarantool/issues/6031 > > > > > > cmake/luajit.cmake | 20 +++++++++++++++----- > > Should we add corresponding ChangeLog entry? Ugh... At first, I have no idea what do we need to write for this patch. This is a partial fix for the build on M1. There is more work to be done. The issue is not resolved by it, so it's too odd to me to add the ChangeLog entry for such changes. BTW, as we discussed in chat, the target branch for these patches is not the default one, but wip-m1/master[1]. I guess, we can add everything necessary, when Tarantool passes the tests on M1. > > > 1 file changed, 15 insertions(+), 5 deletions(-) > > > > diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake > > index 3d37164e8..9390d0dfd 100644 > > --- a/cmake/luajit.cmake > > +++ b/cmake/luajit.cmake > > @@ -73,11 +73,21 @@ if(ENABLE_ASAN) > > add_definitions(-DLUAJIT_USE_ASAN=1) > > endif() > > > > -if(TARGET_OS_DARWIN) > > - # Necessary to make LuaJIT (and Tarantool) work on Darwin, see > > - # http://luajit.org/install.html. > > - set(CMAKE_EXE_LINKER_FLAGS > > - "${CMAKE_EXE_LINKER_FLAGS} -pagezero_size 10000 -image_base 100000000") > > +if(TARGET_OS_DARWIN AND NOT LUAJIT_ENABLE_GC64) > > + # XXX: This is not the best idea to build LuaJIT on MacOS > > + # with GC64 disabled. But nobody will stop you from this. > > + # You are warned. For more info see the following issue. > > Typo: s/For more info/For more info,/ OK, fixed. > > > + # https://github.com/tarantool/tarantool/issues/2643 > > + message(WARNING "LUAJIT_ENABLE_GC64 is disabled for MacOS. " > > + "See #2643, why this is not a good idea.") > > Nit: may be the full link is better here (like we do for coredumps). > Feel free to ignore. If it makes someone happier. Diff with both fixes is below: ================================================================================ diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake index 9390d0dfd..6a94327a7 100644 --- a/cmake/luajit.cmake +++ b/cmake/luajit.cmake @@ -76,10 +76,11 @@ endif() if(TARGET_OS_DARWIN AND NOT LUAJIT_ENABLE_GC64) # XXX: This is not the best idea to build LuaJIT on MacOS # with GC64 disabled. But nobody will stop you from this. - # You are warned. For more info see the following issue. + # You are warned. For more info, see the following issue. # https://github.com/tarantool/tarantool/issues/2643 message(WARNING "LUAJIT_ENABLE_GC64 is disabled for MacOS. " - "See #2643, why this is not a good idea.") + "If one wants to know why this is not a good idea, " + "see https://github.com/tarantool/tarantool/issues/2643.") # XXX: Necessary to make LuaJIT (and hence Tarantool) work on # Darwin/x86_64, see the following links for more info: # * http://luajit.org/install.html#embed ================================================================================ > > > + # XXX: Necessary to make LuaJIT (and hence Tarantool) work on > > + # Darwin/x86_64, see the following links for more info: > > + # * http://luajit.org/install.html#embed > > + # * https://github.com/tarantool/luajit/blob/789820a/cmake/SetTargetFlags.cmake > > + if(CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64") > > + set(CMAKE_EXE_LINKER_FLAGS > > + "${CMAKE_EXE_LINKER_FLAGS} -pagezero_size 10000 -image_base 100000000") > > + endif() > > endif() > > > > # Define the locations for LuaJIT sources and artefacts. > > -- > > 2.25.0 > > > > -- > Best regards, > Sergey Kaplun [1]: https://github.com/tarantool/tarantool/tree/wip-m1/master -- Best regards, IM