Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Alexander Turenko <alexander.turenko@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v2] tools: fix luacheck invocation in different cases
Date: Thu, 25 Feb 2021 14:59:38 +0300	[thread overview]
Message-ID: <47b923aa-6d73-451b-169f-ea0c0e48f628@tarantool.org> (raw)
In-Reply-To: <524c0ce8acc18111ab4c8b36e383ff192779c780.1613661908.git.alexander.turenko@tarantool.org>

Hello,

thanks for the patch!

LGTM and see my comment below

On 18.02.2021 19:09, Alexander Turenko wrote:
> Now `make luacheck` gracefully handles different cases: in-source and
> out-of-source build (within the source tree or outside), current working
> directory as a real path or with symlink components.
>
> As result of looking into those problems I filed the issue [1] against
> luacheck. It seems, there are problems around absolute paths with
> symlinks components.
>
> [1]: https://github.com/mpeterv/luacheck/issues/208
> ---
>
> no issue
> Totktonada/fix-luacheck-invocation
> https://github.com/tarantool/tarantool/tree/Totktonada/fix-luacheck-invocation
>
> Changes since v1:
>
> * Moved the logic to CMake, dropped the shell wrapper.
> * Shrink comments.
> * Handled the case, when a build directory is in the source directory,
>    and cmake is called not like `cmake ..`, but `cmake /path/to/source`,
>    where the path is not a real path.
>
>   CMakeLists.txt    | 28 ++++++++++++++++++++++++++--
>   cmake/utils.cmake | 22 ++++++++++++++++++++++
>   2 files changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 4fbd19558..97cfff7ae 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -158,12 +158,36 @@ add_custom_target(ctags DEPENDS tags)
>   #
>   # Enable 'make luacheck' target.
>   #
> -
> +# The code looks tricky, because of luacheck side problems
> +# (see [1]).
> +#
> +# The following circumstances may lead to missing of source files
> +# or exclusions:
> +#
> +# * Calling `luacheck "${dir}/.luacheckrc" "${dir}"` from
> +#   outside of ${dir} or when ${dir} is not a real path.
> +# * Using of a path with symlink components in --exclude-files.
> +#
> +# [1]: https://github.com/mpeterv/luacheck/issues/208
> +#
> +set(EXCLUDE_FILES)
> +get_filename_component(BINARY_REALPATH "${PROJECT_BINARY_DIR}" REALPATH)
> +get_filename_component(SOURCE_REALPATH "${PROJECT_SOURCE_DIR}" REALPATH)
> +file_is_in_directory(BINARY_DIR_INSIDE_SOURCE_DIR "${BINARY_REALPATH}"
> +                     "${SOURCE_REALPATH}")
> +if (BINARY_DIR_INSIDE_SOURCE_DIR)
extra whitespace after "if"
> +    set(EXCLUDE_FILES --exclude-files "${BINARY_REALPATH}/**/*.lua")
> +endif()
>   add_custom_target(luacheck)
>   add_custom_command(TARGET luacheck
> -    COMMAND ${LUACHECK} --codes --config "${PROJECT_SOURCE_DIR}/.luacheckrc" "${PROJECT_SOURCE_DIR}"
> +    COMMAND ${LUACHECK} --codes --config .luacheckrc . ${EXCLUDE_FILES}
> +    WORKING_DIRECTORY ${SOURCE_REALPATH}
>       COMMENT "Perform static analysis of Lua code"
>   )
> +unset(BINARY_REALPATH)
> +unset(SOURCE_REALPATH)
> +unset(BINARY_DIR_INSIDE_SOURCE_DIR)
> +unset(EXCLUDE_FILES)
>   
>   if (WITH_JEPSEN)
>       ExternalProject_Add(
> diff --git a/cmake/utils.cmake b/cmake/utils.cmake
> index eaec821b3..e9b5fed5d 100644
> --- a/cmake/utils.cmake
> +++ b/cmake/utils.cmake
> @@ -86,3 +86,25 @@ function(bin_source varname srcfile dstfile)
>   
>   endfunction()
>   
> +#
> +# Whether a file is descendant to a directory.
> +#
> +# If the file is the directory itself, the answer is FALSE.
> +#
> +function(file_is_in_directory varname file dir)
> +    file(RELATIVE_PATH file_relative "${dir}" "${file}")
> +    if (file_relative STREQUAL "")
> +        # <file> and <dir> is the same directory.
> +        set(${varname} FALSE PARENT_SCOPE)
> +    elseif (file_relative STREQUAL "..")
> +        # <dir> inside a <file> (so it is a directory too), not
> +        # vice versa.
> +        set(${varname} FALSE PARENT_SCOPE)
> +    elseif (file_relative MATCHES "^\\.\\./")
> +        # <file> somewhere outside of the <dir>.
> +        set(${varname} FALSE PARENT_SCOPE)
> +    else()
> +        # <file> is descendant to <dir>.
> +        set(${varname} TRUE PARENT_SCOPE)
> +    endif()
> +endfunction()

  reply	other threads:[~2021-02-25 11:59 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-18 16:09 Alexander Turenko via Tarantool-patches
2021-02-25 11:59 ` Sergey Bronnikov via Tarantool-patches [this message]
2021-02-25 16:35   ` Alexander Turenko via Tarantool-patches
2021-02-26  9:25     ` Sergey Bronnikov via Tarantool-patches
2021-02-26 18:11 ` Igor Munkin via Tarantool-patches
2021-03-03 18:02   ` Alexander Turenko via Tarantool-patches
2021-03-04 22:21     ` Igor Munkin via Tarantool-patches
2021-03-05  3:50       ` Alexander Turenko via Tarantool-patches
2021-03-05 20:04         ` Alexander Turenko via Tarantool-patches
2021-03-05 23:24         ` Igor Munkin via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=47b923aa-6d73-451b-169f-ea0c0e48f628@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=alexander.turenko@tarantool.org \
    --cc=sergeyb@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v2] tools: fix luacheck invocation in different cases' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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