[Tarantool-patches] [PATCH luajit] test: fix luacheck invocation for non-real paths

[Tarantool-patches] [PATCH luajit] test: fix luacheck invocation for non-real paths

[Igor Munkin via 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

Re: [Tarantool-patches] [PATCH luajit] test: fix luacheck invocation for non-real paths

[Alexander Turenko via 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

Re: [Tarantool-patches] [PATCH luajit] test: fix luacheck invocation for non-real paths

[Sergey Kaplun via 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

Re: [Tarantool-patches] [PATCH luajit] test: fix luacheck invocation for non-real paths

[Igor Munkin via 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

Re: [Tarantool-patches] [PATCH luajit] test: fix luacheck invocation for non-real paths

[Igor Munkin via 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

Re: [Tarantool-patches] [PATCH luajit] test: fix luacheck invocation for non-real paths

[Igor Munkin via 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