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 23F247030C; Tue, 9 Feb 2021 14:39:42 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 23F247030C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1612870782; bh=BnM0QrdyLulN+nhuzYSjovFKZ4P++J3XUffkzDBQi8E=; 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=Oa0jADKRG4Xzbj9SwnxXDt3BQ8XRtSWGOUjhi/AS8q9V9VBO/hzgZLI5dRxF+kWKz miqywBDiMI64n5j0ZD8YhE1AHwS4jLHQx5JLfdkIwI9dXPAvSMf58vpUrQir212eiX 4OzMtBd8D5i8lYJVm9FW2joUQZ7Rz4F9JyI6trUw= Received: from smtp41.i.mail.ru (smtp41.i.mail.ru [94.100.177.101]) (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 E7DB17030C for ; Tue, 9 Feb 2021 14:39:38 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E7DB17030C Received: by smtp41.i.mail.ru with esmtpa (envelope-from ) id 1l9RMk-0000U9-8S; Tue, 09 Feb 2021 14:39:34 +0300 Date: Tue, 9 Feb 2021 14:38:52 +0300 To: Igor Munkin Message-ID: <20210209113852.GA23319@root> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD953AC099BC0052A9C4647521586BE7E637AB8B70F7375365A182A05F5380850403C1BFAF51407A0ADBAE77F995F4F09635E8F8D815B9B8EA1379EA8147C1BB14A X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7E044D6BF4785D596EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006379E389E29EFC3C6B78638F802B75D45FF5571747095F342E8C7A0BC55FA0FE5FCA38F318C1436F2E8CA066EADAB269E0471FD80EF1CE31841389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C0ECC8AC47CD0EDEFF8941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B6E232F00D8D26902CA471835C12D1D977C4224003CC836476EC64975D915A344093EC92FD9297F6718AA50765F790063786CC8ACA642ED665A7F4EDE966BC389F395957E7521B51C24C7702A67D5C33162DBA43225CD8A89F63B13348E4A204A042539A7722CA490CB5C8C57E37DE458B4C7702A67D5C3316FA3894348FB808DB48C21F01D89DB561574AF45C6390F7469DAA53EE0834AAEE X-C1DE0DAB: 0D63561A33F958A545B9DBBCF3905815CE185D85FC586CE5D84604636963F981D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75448CF9D3A7B2C848410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D344E332383F80D58BBB9B0ABBE98AFACDF86879696104593E3477FD0F855AB3F4EE1F0ED986678D2941D7E09C32AA3244C487DF6BA3886EA6472E08CC56541612AFE8DA44ABE2443F7FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojg67HQLCbniJsEvcmhAhY7g== X-Mailru-Sender: 3B9A0136629DC91206CBC582EFEF4CB4CC4581EDE4DEC6A1D83001EFA8F3BA5E70A61A9230AE28B3F2400F607609286E924004A7DEC283833C7120B22964430C52B393F8C72A41A89437F6177E88F7363CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 1/5] build: preserve the original build system 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: Sergey Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Igor! Thanks for the patch! LGTM, except 4 comments about memory profiler parser below. On 02.02.21, Igor Munkin wrote: > Since the build machinery is going to be ported to CMake there would be > Makefile names clashing. This change renames the original build system > (and a couple of auxiliary files requiring configuring) to keep all this > machinery working. > > As a result of these changes one need to explicitly specify the Makefile > in the build command: > | make -f Makefile.original > > Needed for tarantool/tarantool#4862 > > Signed-off-by: Igor Munkin > --- > Makefile => Makefile.original | 44 +++++++++++++-------- > etc/{luajit.pc => luajit.pc.in} | 4 +- > src/{Makefile.dep => Makefile.dep.original} | 0 > src/{Makefile => Makefile.original} | 4 +- > tools/luajit-parse-memprof | 9 ----- > tools/luajit-parse-memprof.in | 6 +++ > 6 files changed, 38 insertions(+), 29 deletions(-) > rename Makefile => Makefile.original (85%) > rename etc/{luajit.pc => luajit.pc.in} (91%) > rename src/{Makefile.dep => Makefile.dep.original} (100%) > rename src/{Makefile => Makefile.original} (99%) > delete mode 100755 tools/luajit-parse-memprof > create mode 100644 tools/luajit-parse-memprof.in > > diff --git a/Makefile b/Makefile.original > similarity index 85% > rename from Makefile > rename to Makefile.original > index 61967df..b85d4bf 100644 > --- a/Makefile > +++ b/Makefile.original > @@ -85,10 +85,10 @@ INSTALL_X= install -m 0755 > @@ -185,13 +185,25 @@ uninstall: > > ############################################################################## > > -amalg: > +amalg: tools > @echo "Building LuaJIT $(VERSION)" > - $(MAKE) -C src amalg > + $(MAKE) -C src -f Makefile.original amalg > > clean: > - $(MAKE) -C src clean > + $(RM) tools/$(FILE_TMEMPROF) > + $(MAKE) -C src -f Makefile.original clean > > -.PHONY: all install amalg clean > +tools: tools/$(FILE_TMEMPROF) > + > +# FIXME: This is an ugly hack to manually configure an auxiliary > +# tools/luajit-parse-memprof. I hope this file will have gone away 1. I suggest the following rewording ("I hope" is too personal :)): s/I hope this file will have gone away/This file should go away/ > +# in scope of https://github.com/tarantool/tarantool/issues/5688. > +tools/$(FILE_TMEMPROF): > + @sed -e "s|@LUAJIT_TOOLS_DIR@|$(realpath tools)|" \ > + -e "s|@LUAJIT_TOOLS_BIN@|$(realpath src/luajit)|" \ 2. @LUAJIT_BIN@ looks more convenient, doesn't it? > + $@.in > $@ > + @chmod +x $@ > + > +.PHONY: all install amalg clean tools > > ############################################################################## > diff --git a/etc/luajit.pc b/etc/luajit.pc.in > similarity index 91% > rename from etc/luajit.pc > rename to etc/luajit.pc.in > index a78f174..f32385d 100644 > --- a/etc/luajit.pc > +++ b/etc/luajit.pc.in > @@ -5,8 +5,8 @@ relver=0 > diff --git a/src/Makefile.dep b/src/Makefile.dep.original > similarity index 100% > rename from src/Makefile.dep > rename to src/Makefile.dep.original > diff --git a/src/Makefile b/src/Makefile.original > similarity index 99% > rename from src/Makefile > rename to src/Makefile.original > index 825b01c..502504c 100644 > --- a/src/Makefile > +++ b/src/Makefile.original > diff --git a/tools/luajit-parse-memprof b/tools/luajit-parse-memprof > deleted file mode 100755 > diff --git a/tools/luajit-parse-memprof.in b/tools/luajit-parse-memprof.in > new file mode 100644 > index 0000000..8867202 > --- /dev/null > +++ b/tools/luajit-parse-memprof.in > @@ -0,0 +1,6 @@ > +#!/bin/bash > +# > +# Launcher for memprof parser. > + > +LUA_PATH="@LUAJIT_TOOLS_DIR@/?.lua;;" \ > + @LUAJIT_TOOLS_BIN@ @LUAJIT_TOOLS_DIR@/memprof.lua $@ 3. Unfortunately, your solution doesn't work for me: | $ git log --oneline -n1 | 7badb7e (HEAD) build: preserve the original build system | $ make -f Makefile.original -j | ==== Building LuaJIT 2.1.0-beta3 ==== | ... | ==== Successfully built LuaJIT 2.1.0-beta3 ==== Side note: I suppose it is nothing bad to add corresponding lines about tools building. At least it simplifies build debugging. Feel free to ignore. | $ tools/luajit-parse-memprof /tmp/memprof_memleak.bin; echo -e "\n"; cat tools/luajit-parse-memprof | tools/luajit-parse-memprof: line 5: /home/burii/reviews/luajit/cmake/tools/memprof.lua: Permission denied | | #!/bin/bash | # | # Launcher for memprof parser. | | LUA_PATH="/home/burii/reviews/luajit/cmake/tools/?.lua;;" \ | /home/burii/reviews/luajit/cmake/tools/memprof.lua $@ This patch is working for me. But I am not bash|make guru :). | diff --git a/Makefile.original b/Makefile.original | index b85d4bf..877398a 100644 | --- a/Makefile.original | +++ b/Makefile.original | @@ -199,8 +199,8 @@ tools: tools/$(FILE_TMEMPROF) | # tools/luajit-parse-memprof. I hope this file will have gone away | # in scope of https://github.com/tarantool/tarantool/issues/5688. | tools/$(FILE_TMEMPROF): | - @sed -e "s|@LUAJIT_TOOLS_DIR@|$(realpath tools)|" \ | - -e "s|@LUAJIT_TOOLS_BIN@|$(realpath src/luajit)|" \ | + @sed -e "s|@LUAJIT_TOOLS_DIR@|`realpath tools`|" \ | + -e "s|@LUAJIT_TOOLS_BIN@|`realpath src/luajit`|" \ | $@.in > $@ | @chmod +x $@ | 4. Please, create separate commit to tools-related changes. Also, I suppose, it would be nice to add to <.gitignore> as far as it's generated. These lines may be confusing: | $ git status | HEAD detached at 7badb7e | Untracked files: | (use "git add ..." to include in what will be committed) | tools/luajit-parse-memprof > -- > 2.25.0 > -- Best regards, Sergey Kaplun