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 772956EC58; Sun, 20 Jun 2021 21:58:10 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 772956EC58 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1624215490; bh=BoA3U/I0nmbbj/NwTLk5YYj/bSAuXQxQJLWILMU56eE=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=FmjZ2av5AYU09nVn7Bnp1Es0RKpSwslQbyozIvz2icX5KLwJgOBmDTBgxF53yRzcD NF7OCo5eKcJ/x01/6HCsnBPFGCDCmqaAsJaRB8+oEaNzeFuS6FySQp57kNTMIpfvXE tneTvaDtuC/xYCnluF2yPBQzkKQegRC6q9cOR17E= 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 7285C6EC58 for ; Sun, 20 Jun 2021 21:58:09 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 7285C6EC58 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1lv2e0-0005y7-1A; Sun, 20 Jun 2021 21:58:08 +0300 Date: Sun, 20 Jun 2021 21:57:59 +0300 To: Timur Safin Cc: alexander.turenko@tarantool.org, tarantool-patches@dev.tarantool.org Message-ID: <20210620185759.GC10212@tarantool.org> References: 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: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD91C2C07775F13263A612E9F961DCF5576EBB1C736C2A4021700894C459B0CD1B9E0232BB0A31B34B01E015D32A1EC824A231B5704311BD6EE080FAAE239EDB74A X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7CC84CC3AD347B910EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063703E3935C5A8197E98638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D875ADBC679C94916EE3473BF9F8FB9007117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BF1175FABE1C0F9B6A471835C12D1D977C4224003CC8364762BB6847A3DEAEFB0F43C7A68FF6260569E8FC8737B5C2249EC8D19AE6D49635B68655334FD4449CB9ECD01F8117BC8BEAAAE862A0553A39223F8577A6DFFEA7CB1724D34C644744043847C11F186F3C59DAA53EE0834AAEE X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A2AD77751E876CB595E8F7B195E1C978314421050EE5BFC8A663EC326E2DBBB914 X-C1DE0DAB: 0D63561A33F958A5E4D8F6E64056594C0B742F933D92F59A51DCB29FA1A3490DD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75448CF9D3A7B2C848410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D349FF8F8245A2FAA7B08F716939EB2FA72F6C176B127C49D62242D3AF072B39A82422C51614782EE841D7E09C32AA3244C67390F97FAE4DA95949632A4F6449322A90944CA99CF22E3927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj8x+Gb+jwA+TY7O763HZCpQ== X-Mailru-Sender: 689FA8AB762F73936BC43F508A06382254127DFFD3E50689D6576E6E8D4FDA55A7C8D0F45F857DBFE9F1EFEE2F478337FB559BB5D741EB964C8C2C849690F8E70A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v2 1/3] test: corrected reported error lines 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 Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Timur, Thanks for the patch! Though the change is quite trivial, the patch is not OK right from the top. Please adjust the commit subject according to our contribution guidelines[1]. Regarding the changes itself: why did you send it within absolutely unrelated changeset? This is not required for the following patches. This is not related to SQL at all. This patch should be on the another branch, and the next patches should be rebased on that branch (if you need it so much). You can argue that this is a trivial oneliner and I am making lot of ado about nothing, *but* this is your point of view. And now let me share mine one with you: * This patch should be backported to all longterm branches, since I consider this as a bug. Since this patch is on the bottom of the series, I need to make more actions for this. For what? I see no reason for it. * You've chosen Sasha as a *first* reviewer for the first patch of the series. However, I'm sure every member of our "L" team can carefully review such a simple fix. Hence, you decided to stall the whole series until Sasha approves this tiny patch? * On the other hand if this patch has enough LGTMs, it can be applied "out of order". And when it is applied, you will need to rebase your working branch with SQL-related changes on master. As a result this *optimization* has no sense at all and only makes maintainer's life much worse. Finally, if there was not a nit regarding this patch, I would suggest you a deal: leave the patch as is and consider everything written above for the further patches. However, there are several nits to be resolved, so please move this patch out from this series in a separate one. On 11.06.21, Timur Safin via Tarantool-patches wrote: > It always was a problem that reported source line was not > pointing to the actual callee line number, but rather to Don't get why you are talking about callee here: regardless the line to be reported (the current one or the one where the function is defined), it is a *caller* for the function. So, the actual problem you're writing is that the line with function signature is used instead of the line where the function execution is stopped at the moment of the call. As for me, these are the different reasons. > the start of file, i.e. we have seen: > ``` > [001] sql-tap/tkt-9a8b09f8e6.test.lua memtx > [001] not ok 22 - 4.3 # > [001] Traceback: > [001] [Lua ] function 'do_catchsql_test' at > [001] [main] at > [001] > [001] not ok 23 - 4.4 # > [001] Traceback: > [001] [Lua ] function 'do_catchsql_test' at > [001] [main] at > ``` > (see the :0 part) Minor: Strictly saying :123 part is also broken. The only difference between them is that :0 line is used for so called "main" function. > Instead of correct line numbers: > ``` > [001] sql-tap/tkt-9a8b09f8e6.test.lua memtx > [001] not ok 22 - 4.3 # > [001] Traceback: > [001] [Lua ] function 'do_catchsql_test' at > [001] [main] at > [001] > [001] not ok 23 - 4.4 # > [001] Traceback: > [001] [Lua ] function 'do_catchsql_test' at > [001] [main] at > ``` > > The problem was due to `.linedefined` used, instead of source line in `.currentline`. Typo: The line exceeds 72 chars. > > Closes #6134 > --- > src/lua/tap.lua | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/lua/tap.lua b/src/lua/tap.lua > index 346724d84..77fd8d096 100644 > --- a/src/lua/tap.lua > +++ b/src/lua/tap.lua > @@ -23,7 +23,7 @@ local function traceback(level) > local frame = { > source = info.source; > src = info.short_src; > - line = info.linedefined or 0; > + line = info.currentline or info.linedefined or 0; Why did you leave such a complex code here? I believe you can use just info.currentline here. > what = info.what; > name = info.name; > namewhat = info.namewhat; > -- > 2.29.2 > [1]: https://www.tarantool.io/en/doc/latest/dev_guide/developer_guidelines/#how-to-write-a-commit-message -- Best regards, IM