Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH luajit] test: fix luacheck invocation for non-real paths
@ 2021-03-05 23:20 Igor Munkin via Tarantool-patches
  2021-03-08  4:27 ` Alexander Turenko via Tarantool-patches
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2021-03-05 23:20 UTC (permalink / raw)
  To: Sergey Kaplun, Alexander Turenko; +Cc: tarantool-patches

Unfortunately, luacheck doesn't handle the working directory or one used
in options if it is not a real path. As a result of this patch both
PROJECT_SOURCE_DIR and PROJECT_BINARY_DIR are resolved prior to be used
within luacheck target custom command.

The issue has been already fixed in Tarantool repo after applying
af448464d15f60b87f1c9ef41a7816911c889459 ('tools: fix luacheck
invocation in different cases'), and this patch is necessary, since
<LuaJIT-luacheck> is the dependency for Tarantool <luacheck> target.

Relates to mpeterv/luacheck#208

Reported-by: Alexander Turenko <alexander.turenko@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
---

Branch: https://github.com/tarantool/luajit/tree/imun/luacheck-realpath
CI is green on this branch (see the tick near the HEAD commit):
https://github.com/tarantool/tarantool/tree/imun/fix-luacheck-invocation

 test/CMakeLists.txt | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index d166c9d8..c58a8ed5 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -5,25 +5,33 @@ cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
 
 find_program(LUACHECK luacheck)
 if(LUACHECK)
-  set(LUACHECK_RC ${PROJECT_SOURCE_DIR}/.luacheckrc)
-  file(GLOB_RECURSE LUACHECK_DEPS ${PROJECT_SOURCE_DIR}/*.lua)
+  # XXX: The tweak below relates to luacheck problem with paths.
+  # If the working directory or one used in luacheck options is
+  # not a real path, luacheck doesn't handle it the right way.
+  # Hence the paths used by luacheck in CMake ought to be resolved
+  # to the real ones. For more info, see the following issue.
+  # https://github.com/mpeterv/luacheck/issues/208
+  get_filename_component(LUACHECK_SRCDIR "${PROJECT_SOURCE_DIR}" REALPATH)
+  get_filename_component(LUACHECK_BINDIR "${PROJECT_BINARY_DIR}" REALPATH)
+  set(LUACHECK_RC ${LUACHECK_SRCDIR}/.luacheckrc)
+  file(GLOB_RECURSE LUACHECK_DEPS ${LUACHECK_SRCDIR}/*.lua)
   add_custom_target(${PROJECT_NAME}-luacheck
     DEPENDS ${LUACHECK_RC} ${LUACHECK_DEPS}
   )
   add_custom_command(TARGET ${PROJECT_NAME}-luacheck
     COMMENT "Running luacheck static analysis"
     COMMAND
-      ${LUACHECK} ${PROJECT_SOURCE_DIR}
+      ${LUACHECK} ${LUACHECK_SRCDIR}
         --codes
         --config ${LUACHECK_RC}
         # XXX: jit/vmdef.lua is an autogenerated Lua source, so
         # there is no need to run luacheck against it. Hence
         # explicitly exclude this file from the list for check.
-        --exclude-files ${LUAJIT_BINARY_DIR}/jit/vmdef.lua
+        --exclude-files ${LUACHECK_BINDIR}/src/jit/vmdef.lua
     # XXX: Filenames in .luacheckrc are considered relative to
     # the working directory, hence luacheck should be run in the
     # project root directory.
-    WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
+    WORKING_DIRECTORY ${LUACHECK_SRCDIR}
   )
 else()
   add_custom_target(${PROJECT_NAME}-luacheck)
-- 
2.25.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] test: fix luacheck invocation for non-real paths
  2021-03-05 23:20 [Tarantool-patches] [PATCH luajit] test: fix luacheck invocation for non-real paths Igor Munkin via Tarantool-patches
@ 2021-03-08  4:27 ` Alexander Turenko via Tarantool-patches
  2021-03-10  0:14   ` Igor Munkin via Tarantool-patches
  2021-03-08 19:26 ` Sergey Kaplun via Tarantool-patches
  2021-03-10 16:25 ` Igor Munkin via Tarantool-patches
  2 siblings, 1 reply; 6+ messages in thread
From: Alexander Turenko via Tarantool-patches @ 2021-03-08  4:27 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

LGTM.

Just curious, why you don't need exclusion of a build directory for the
case, when it is inside the source directory?

WBR, Alexander Turenko.

> Branch: https://github.com/tarantool/luajit/tree/imun/luacheck-realpath
> CI is green on this branch (see the tick near the HEAD commit):
> https://github.com/tarantool/tarantool/tree/imun/fix-luacheck-invocation

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] test: fix luacheck invocation for non-real paths
  2021-03-05 23:20 [Tarantool-patches] [PATCH luajit] test: fix luacheck invocation for non-real paths Igor Munkin via Tarantool-patches
  2021-03-08  4:27 ` Alexander Turenko via Tarantool-patches
@ 2021-03-08 19:26 ` Sergey Kaplun via Tarantool-patches
  2021-03-10  0:14   ` Igor Munkin via Tarantool-patches
  2021-03-10 16:25 ` Igor Munkin via Tarantool-patches
  2 siblings, 1 reply; 6+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2021-03-08 19:26 UTC (permalink / raw)
  To: Igor Munkin; +Cc: Alexander Turenko, tarantool-patches

Hi, Igor!

Thanks for the patch!

LGTM!

`${LUACHECK_SOURCE_DIR}` and `${LUACHECK_BINARY_DIR}` look more
consistent for me (according to other variables naming), but it's up to
you.
Feel free to ignore.

On 06.03.21, Igor Munkin wrote:
> Unfortunately, luacheck doesn't handle the working directory or one used
> in options if it is not a real path. As a result of this patch both
> PROJECT_SOURCE_DIR and PROJECT_BINARY_DIR are resolved prior to be used
> within luacheck target custom command.
> 
> The issue has been already fixed in Tarantool repo after applying

Nit: "same issue" or "similar issue"
Feel free to ignore.

> af448464d15f60b87f1c9ef41a7816911c889459 ('tools: fix luacheck
> invocation in different cases'), and this patch is necessary, since
> <LuaJIT-luacheck> is the dependency for Tarantool <luacheck> target.
> 
> Relates to mpeterv/luacheck#208
> 
> Reported-by: Alexander Turenko <alexander.turenko@tarantool.org>
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---

<snipped>

> 

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] test: fix luacheck invocation for non-real paths
  2021-03-08  4:27 ` Alexander Turenko via Tarantool-patches
@ 2021-03-10  0:14   ` Igor Munkin via Tarantool-patches
  0 siblings, 0 replies; 6+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2021-03-10  0:14 UTC (permalink / raw)
  To: Alexander Turenko; +Cc: tarantool-patches

Sasha,

Thanks for your review!

On 08.03.21, Alexander Turenko wrote:
> LGTM.

Added your tag:
| Reviewed-by: Alexander Turenko <alexander.turenko@tarantool.org>

> 
> Just curious, why you don't need exclusion of a build directory for the
> case, when it is inside the source directory?

As we discussed before, there is a single Lua file within build
directory: jit/vmdef.lua. Hence, it's enough to exclude only this file.

> 
> WBR, Alexander Turenko.
> 
> > Branch: https://github.com/tarantool/luajit/tree/imun/luacheck-realpath
> > CI is green on this branch (see the tick near the HEAD commit):
> > https://github.com/tarantool/tarantool/tree/imun/fix-luacheck-invocation

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] test: fix luacheck invocation for non-real paths
  2021-03-08 19:26 ` Sergey Kaplun via Tarantool-patches
@ 2021-03-10  0:14   ` Igor Munkin via Tarantool-patches
  0 siblings, 0 replies; 6+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2021-03-10  0:14 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: Alexander Turenko, tarantool-patches

Sergey,

Thanks for your review!

On 08.03.21, Sergey Kaplun wrote:
> Hi, Igor!
> 
> Thanks for the patch!
> 
> LGTM!

Added your tag:
| Reviewed-by: Sergey Kaplun <skaplun@tarantool.org>

> 
> `${LUACHECK_SOURCE_DIR}` and `${LUACHECK_BINARY_DIR}` look more
> consistent for me (according to other variables naming), but it's up to
> you.
> Feel free to ignore.

Fixed, squashed, force-pushed to the branch.

> 
> On 06.03.21, Igor Munkin wrote:
> > Unfortunately, luacheck doesn't handle the working directory or one used
> > in options if it is not a real path. As a result of this patch both
> > PROJECT_SOURCE_DIR and PROJECT_BINARY_DIR are resolved prior to be used
> > within luacheck target custom command.
> > 
> > The issue has been already fixed in Tarantool repo after applying
> 
> Nit: "same issue" or "similar issue"
> Feel free to ignore.

The definite article does its job here.

Ignoring.

> 
> > af448464d15f60b87f1c9ef41a7816911c889459 ('tools: fix luacheck
> > invocation in different cases'), and this patch is necessary, since
> > <LuaJIT-luacheck> is the dependency for Tarantool <luacheck> target.
> > 
> > Relates to mpeterv/luacheck#208
> > 
> > Reported-by: Alexander Turenko <alexander.turenko@tarantool.org>
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> 
> <snipped>
> 
> > 
> 
> -- 
> Best regards,
> Sergey Kaplun

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] test: fix luacheck invocation for non-real paths
  2021-03-05 23:20 [Tarantool-patches] [PATCH luajit] test: fix luacheck invocation for non-real paths Igor Munkin via Tarantool-patches
  2021-03-08  4:27 ` Alexander Turenko via Tarantool-patches
  2021-03-08 19:26 ` Sergey Kaplun via Tarantool-patches
@ 2021-03-10 16:25 ` Igor Munkin via Tarantool-patches
  2 siblings, 0 replies; 6+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2021-03-10 16:25 UTC (permalink / raw)
  To: Sergey Kaplun, Alexander Turenko; +Cc: tarantool-patches

I've checked the patchset into all long-term branches in
tarantool/luajit and bumped a new version in 1.10, 2.6, 2.7 and master.

On 06.03.21, Igor Munkin wrote:
> Unfortunately, luacheck doesn't handle the working directory or one used
> in options if it is not a real path. As a result of this patch both
> PROJECT_SOURCE_DIR and PROJECT_BINARY_DIR are resolved prior to be used
> within luacheck target custom command.
> 
> The issue has been already fixed in Tarantool repo after applying
> af448464d15f60b87f1c9ef41a7816911c889459 ('tools: fix luacheck
> invocation in different cases'), and this patch is necessary, since
> <LuaJIT-luacheck> is the dependency for Tarantool <luacheck> target.
> 
> Relates to mpeterv/luacheck#208
> 
> Reported-by: Alexander Turenko <alexander.turenko@tarantool.org>
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
> 
> Branch: https://github.com/tarantool/luajit/tree/imun/luacheck-realpath
> CI is green on this branch (see the tick near the HEAD commit):
> https://github.com/tarantool/tarantool/tree/imun/fix-luacheck-invocation
> 
>  test/CMakeLists.txt | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 

<snipped>

> -- 
> 2.25.0
> 

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-03-10 16:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-05 23:20 [Tarantool-patches] [PATCH luajit] test: fix luacheck invocation for non-real paths Igor Munkin via Tarantool-patches
2021-03-08  4:27 ` Alexander Turenko via Tarantool-patches
2021-03-10  0:14   ` Igor Munkin via Tarantool-patches
2021-03-08 19:26 ` Sergey Kaplun via Tarantool-patches
2021-03-10  0:14   ` Igor Munkin via Tarantool-patches
2021-03-10 16:25 ` Igor Munkin via Tarantool-patches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox