From 8a6f728315d5c87dcc091fb2ae723d91852b2102 Mon Sep 17 00:00:00 2001 From: Dundar Goc Date: Fri, 17 Jun 2022 09:58:48 +0200 Subject: [PATCH 1/5] build(cmake): remove unnecessary *-prereqs targets Running the tests on their own works just fine. --- CMakeLists.txt | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2224ddd5c6..73f418a96e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -676,17 +676,6 @@ if(BUSTED_PRG) set(FUNCTIONALTEST_PREREQS nvim printenv-test printargs-test shell-test streams-test tty-test ${GENERATED_HELP_TAGS}) set(BENCHMARK_PREREQS nvim tty-test) - # Useful for automated build systems, if they want to manually run the tests. - add_custom_target(unittest-prereqs - DEPENDS ${UNITTEST_PREREQS}) - set_target_properties(unittest-prereqs PROPERTIES FOLDER test) - - add_custom_target(functionaltest-prereqs - DEPENDS ${FUNCTIONALTEST_PREREQS}) - - add_custom_target(benchmark-prereqs - DEPENDS ${BENCHMARK_PREREQS}) - check_lua_module(${LUA_PRG} "ffi" LUA_HAS_FFI) if(LUA_HAS_FFI) add_custom_target(unittest @@ -731,8 +720,7 @@ if(BUSTED_PRG) -P ${PROJECT_SOURCE_DIR}/cmake/RunTests.cmake DEPENDS ${FUNCTIONALTEST_PREREQS} ${TEST_TARGET_ARGS}) - set_target_properties(functionaltest functionaltest-prereqs - PROPERTIES FOLDER test) + set_target_properties(functionaltest PROPERTIES FOLDER test) add_custom_target(benchmark COMMAND ${CMAKE_COMMAND} @@ -747,7 +735,7 @@ if(BUSTED_PRG) -P ${PROJECT_SOURCE_DIR}/cmake/RunTests.cmake DEPENDS ${BENCHMARK_PREREQS} ${TEST_TARGET_ARGS}) - set_target_properties(benchmark benchmark-prereqs PROPERTIES FOLDER test) + set_target_properties(benchmark PROPERTIES FOLDER test) endif() if(BUSTED_LUA_PRG) From 668591ae04b3578d48d916680e4641c3c4afa525 Mon Sep 17 00:00:00 2001 From: Dundar Goc Date: Sat, 18 Jun 2022 15:22:27 +0200 Subject: [PATCH 2/5] build(cmake): remove unnecessary globbing with file There's no need to use globbing since there's only one file. --- CMakeLists.txt | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 73f418a96e..39659d6469 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -636,11 +636,8 @@ endif() include(InstallHelpers) -file(GLOB MANPAGES - RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} - man/nvim.1) install_helper( - FILES ${MANPAGES} + FILES ${CMAKE_SOURCE_DIR}/man/nvim.1 DESTINATION ${CMAKE_INSTALL_MANDIR}/man1) # From cd1b2998d394ad85f1f48a5f2a8cb064ae31b521 Mon Sep 17 00:00:00 2001 From: Dundar Goc Date: Fri, 17 Jun 2022 14:11:08 +0200 Subject: [PATCH 3/5] build(cmake): simplify and speed up the uninstall target More specifically, replace exec_program with file(REMOVE ...) so that the uninstall target is run during the build stage instead of the configure stage, significantly speeding up the target. The code snippet that was removed is taken from the cmake FAQ https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake. However, this uses undocumented features such as IMMEDIATE when calling configure_file, which is an artifact from cmake 2.x (it's so old it's difficult to find information on it). Similarly, this particular code snippet has been around for a long time and originated from the cmake mailing lists. Based on this I believe the in-file was a workaround for the limitations of cmake back then and that it's not required anymore. --- CMakeLists.txt | 13 ++----------- cmake/UninstallHelper.cmake | 13 +++++++++++++ cmake/UninstallHelper.cmake.in | 21 --------------------- 3 files changed, 15 insertions(+), 32 deletions(-) create mode 100644 cmake/UninstallHelper.cmake delete mode 100644 cmake/UninstallHelper.cmake.in diff --git a/CMakeLists.txt b/CMakeLists.txt index 39659d6469..ac67dad75c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -752,17 +752,8 @@ if(BUSTED_LUA_PRG) set_target_properties(functionaltest-lua PROPERTIES FOLDER test) endif() -#add uninstall target -if(NOT TARGET uninstall) - configure_file( - "cmake/UninstallHelper.cmake.in" - "${CMAKE_CURRENT_BINARY_DIR}/UninstallHelper.cmake" - IMMEDIATE @ONLY) - - add_custom_target(uninstall - COMMAND ${CMAKE_COMMAND} -P ${CMAKE_CURRENT_BINARY_DIR}/UninstallHelper.cmake) -endif() - +add_custom_target(uninstall + COMMAND ${CMAKE_COMMAND} -P ${PROJECT_SOURCE_DIR}/cmake/UninstallHelper.cmake) if(${CMAKE_SOURCE_DIR} STREQUAL ${CMAKE_CURRENT_SOURCE_DIR}) add_subdirectory(packaging) diff --git a/cmake/UninstallHelper.cmake b/cmake/UninstallHelper.cmake new file mode 100644 index 0000000000..9a3d30af59 --- /dev/null +++ b/cmake/UninstallHelper.cmake @@ -0,0 +1,13 @@ +if(NOT EXISTS "${CMAKE_BINARY_DIR}/install_manifest.txt") + message(FATAL_ERROR "Cannot find install manifest: ${CMAKE_BINARY_DIR}/install_manifest.txt") +endif() + +file(STRINGS "${CMAKE_BINARY_DIR}/install_manifest.txt" files) +foreach(file ${files}) + message(STATUS "Uninstalling $ENV{DESTDIR}${file}") + if(IS_SYMLINK "$ENV{DESTDIR}${file}" OR EXISTS "$ENV{DESTDIR}${file}") + file(REMOVE $ENV{DESTDIR}${file}) + else() + message(STATUS "File $ENV{DESTDIR}${file} does not exist.") + endif() +endforeach() diff --git a/cmake/UninstallHelper.cmake.in b/cmake/UninstallHelper.cmake.in deleted file mode 100644 index c2d34d4796..0000000000 --- a/cmake/UninstallHelper.cmake.in +++ /dev/null @@ -1,21 +0,0 @@ -if(NOT EXISTS "@CMAKE_BINARY_DIR@/install_manifest.txt") - message(FATAL_ERROR "Cannot find install manifest: @CMAKE_BINARY_DIR@/install_manifest.txt") -endif() - -file(READ "@CMAKE_BINARY_DIR@/install_manifest.txt" files) -string(REGEX REPLACE "\n" ";" files "${files}") -foreach(file ${files}) - message(STATUS "Uninstalling $ENV{DESTDIR}${file}") - if(IS_SYMLINK "$ENV{DESTDIR}${file}" OR EXISTS "$ENV{DESTDIR}${file}") - exec_program( - "@CMAKE_COMMAND@" ARGS "-E remove \"$ENV{DESTDIR}${file}\"" - OUTPUT_VARIABLE rm_out - RETURN_VALUE rm_retval - ) - if(NOT "${rm_retval}" STREQUAL 0) - message(FATAL_ERROR "Problem when removing $ENV{DESTDIR}${file}") - endif() - else(IS_SYMLINK "$ENV{DESTDIR}${file}" OR EXISTS "$ENV{DESTDIR}${file}") - message(STATUS "File $ENV{DESTDIR}${file} does not exist.") - endif() -endforeach() From 636a30998166668f49a8ccca9daa193f6e7ca432 Mon Sep 17 00:00:00 2001 From: Dundar Goc Date: Sat, 18 Jun 2022 17:31:01 +0200 Subject: [PATCH 4/5] build(cmake): simplify def_cmd_target function Instead of appending to a command output, append to an existing target instead. The primary benefit is intermediary ...-cmd targets aren't needed, we can instead append commands to the relevant target directly. --- CMakeLists.txt | 18 +++++++++++++----- cmake/DefCmdTarget.cmake | 32 ++++++++++++-------------------- src/nvim/CMakeLists.txt | 2 +- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ac67dad75c..dad0093cf0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -618,20 +618,28 @@ find_program(SHELLCHECK_PRG shellcheck) include(DefCmdTarget) def_cmd_target(lintlua ${LUACHECK_PRG} LUACHECK_PRG true) if(LUACHECK_PRG) - add_custom_command(OUTPUT lintlua-cmd APPEND COMMAND ${LUACHECK_PRG} -q runtime/ scripts/ src/ test/) + add_custom_command(TARGET lintlua + WORKING_DIRECTORY ${CMAKE_SOURCE_DIR} + COMMAND ${LUACHECK_PRG} -q runtime/ scripts/ src/ test/) endif() if(STYLUA_PRG) - add_custom_command(OUTPUT lintlua-cmd APPEND COMMAND ${STYLUA_PRG} --color=always --check runtime/) + add_custom_command(TARGET lintlua + WORKING_DIRECTORY ${CMAKE_SOURCE_DIR} + COMMAND ${STYLUA_PRG} --color=always --check runtime/) else() - add_custom_command(OUTPUT lintlua-cmd APPEND COMMAND ${CMAKE_COMMAND} -E echo "STYLUA_PRG not found") + add_custom_command(TARGET lintlua COMMAND ${CMAKE_COMMAND} -E echo "STYLUA_PRG not found") endif() def_cmd_target(lintpy ${FLAKE8_PRG} FLAKE8_PRG false) if(FLAKE8_PRG) - add_custom_command(OUTPUT lintpy-cmd APPEND COMMAND ${FLAKE8_PRG} contrib/ scripts/ src/ test/) + add_custom_command(TARGET lintpy + WORKING_DIRECTORY ${CMAKE_SOURCE_DIR} + COMMAND ${FLAKE8_PRG} contrib/ scripts/ src/ test/) endif() def_cmd_target(lintsh ${SHELLCHECK_PRG} SHELLCHECK_PRG false) if(SHELLCHECK_PRG) - add_custom_command(OUTPUT lintsh-cmd APPEND COMMAND ${SHELLCHECK_PRG} scripts/vim-patch.sh) + add_custom_command(TARGET lintsh + WORKING_DIRECTORY ${CMAKE_SOURCE_DIR} + COMMAND ${SHELLCHECK_PRG} scripts/vim-patch.sh) endif() include(InstallHelpers) diff --git a/cmake/DefCmdTarget.cmake b/cmake/DefCmdTarget.cmake index 1ee5cdd60e..48e90cf5c8 100644 --- a/cmake/DefCmdTarget.cmake +++ b/cmake/DefCmdTarget.cmake @@ -1,27 +1,19 @@ -# Defines a target named ${target} and a command with (symbolic) output -# ${target}-cmd. If ${prg} is undefined the target prints "not found". +# Defines a target named ${target}. If ${prg} is undefined the target prints +# "not found". # -# - Use add_custom_command(…APPEND) to build the command after this. -# - Use add_custom_target(…DEPENDS) to run the command from a target. +# - Use add_custom_command(TARGET ...) to append a command to the +# target. function(def_cmd_target target prg prg_name prg_fatal) - # Define a mostly-empty command, which can be appended-to. - add_custom_command(OUTPUT ${target}-cmd - WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} - COMMAND ${CMAKE_COMMAND} -E echo "${target}" - ) - # Symbolic (does not generate an artifact). - set_source_files_properties(${target}-cmd PROPERTIES SYMBOLIC "true") + add_custom_target(${target}) - if(prg OR NOT prg_fatal) - add_custom_target(${target} - WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} - DEPENDS ${target}-cmd) - if(NOT prg) - add_custom_command(OUTPUT ${target}-cmd APPEND + if(NOT prg) + if(prg_fatal) + add_custom_command(TARGET ${target} + COMMAND ${CMAKE_COMMAND} -E echo "${target}: ${prg_name} not found" + COMMAND false) + else() + add_custom_command(TARGET ${target} COMMAND ${CMAKE_COMMAND} -E echo "${target}: SKIP: ${prg_name} not found") endif() - else() - add_custom_target(${target} false - COMMENT "${target}: ${prg_name} not found") endif() endfunction() diff --git a/src/nvim/CMakeLists.txt b/src/nvim/CMakeLists.txt index 0d0fba265b..f465d911f9 100755 --- a/src/nvim/CMakeLists.txt +++ b/src/nvim/CMakeLists.txt @@ -805,7 +805,7 @@ add_custom_target(lintc DEPENDS ${LINT_TARGETS}) def_cmd_target(lintuncrustify ${UNCRUSTIFY_PRG} UNCRUSTIFY_PRG false) if(UNCRUSTIFY_PRG) - add_custom_command(OUTPUT lintuncrustify-cmd APPEND + add_custom_command(TARGET lintuncrustify COMMAND ${CMAKE_COMMAND} -D UNCRUSTIFY_PRG=${UNCRUSTIFY_PRG} -D PROJECT_SOURCE_DIR=${PROJECT_SOURCE_DIR} From ae7a4ad3d70c5a7653d2211dd89884d401186ab2 Mon Sep 17 00:00:00 2001 From: Dundar Goc Date: Sat, 18 Jun 2022 21:48:07 +0200 Subject: [PATCH 5/5] build: remove FindLua.cmake since it's already built into cmake FindLua.cmake is a copy from the cmake repo: https://github.com/Kitware/CMake/blob/0419ecbcad7719614349a07189b45e341a8f2c69/Modules/FindLua.cmake. It's a cmake module, meaning it's already shipped with cmake by default. There have been two changes done to our version of FindLua.cmake. The first change is that include(${CMAKE_CURRENT_LIST_DIR}/FindPackageHandleStandardArgs.cmake) was changed to include(FindPackageHandleStandardArgs.cmake) This change is required only because we have imported FindLua.cmake module but not FindPackageHandleStandardArgs module. Had FindLua been called as a module as intended then this file would not need changing. The second change is that support for Lua 5.4 is added. However, support for any version of Lua except for 5.1 is disabled since https://github.com/neovim/neovim/pull/16633/commits/e322b5c864c771f774ddfea32f6cd5190a1af419. Because these changes from the upstream FindLua.cmake is unnecessary I believe we can and should use the builtin FindLua.cmake instead of our own. --- cmake/FindLua.cmake | 197 -------------------------------------------- 1 file changed, 197 deletions(-) delete mode 100644 cmake/FindLua.cmake diff --git a/cmake/FindLua.cmake b/cmake/FindLua.cmake deleted file mode 100644 index 7ba13e1f56..0000000000 --- a/cmake/FindLua.cmake +++ /dev/null @@ -1,197 +0,0 @@ -# Distributed under the OSI-approved BSD 3-Clause License. See accompanying -# file Copyright.txt or https://cmake.org/licensing for details. - -#.rst: -# FindLua -# ------- -# -# -# -# Locate Lua library This module defines -# -# :: -# -# LUA_FOUND - if false, do not try to link to Lua -# LUA_LIBRARIES - both lua and lualib -# LUA_INCLUDE_DIR - where to find lua.h -# LUA_VERSION_STRING - the version of Lua found -# LUA_VERSION_MAJOR - the major version of Lua -# LUA_VERSION_MINOR - the minor version of Lua -# LUA_VERSION_PATCH - the patch version of Lua -# -# -# -# Note that the expected include convention is -# -# :: -# -# #include "lua.h" -# -# and not -# -# :: -# -# #include -# -# This is because, the lua location is not standardized and may exist in -# locations other than lua/ - -unset(_lua_include_subdirs) -unset(_lua_library_names) -unset(_lua_append_versions) - -# this is a function only to have all the variables inside go away automatically -function(_lua_set_version_vars) - set(LUA_VERSIONS5 5.4 5.3 5.2 5.1 5.0) - - if (Lua_FIND_VERSION_EXACT) - if (Lua_FIND_VERSION_COUNT GREATER 1) - set(_lua_append_versions ${Lua_FIND_VERSION_MAJOR}.${Lua_FIND_VERSION_MINOR}) - endif () - elseif (Lua_FIND_VERSION) - # once there is a different major version supported this should become a loop - if (NOT Lua_FIND_VERSION_MAJOR GREATER 5) - if (Lua_FIND_VERSION_COUNT EQUAL 1) - set(_lua_append_versions ${LUA_VERSIONS5}) - else () - foreach (subver IN LISTS LUA_VERSIONS5) - if (NOT subver VERSION_LESS ${Lua_FIND_VERSION}) - list(APPEND _lua_append_versions ${subver}) - endif () - endforeach () - endif () - endif () - else () - # once there is a different major version supported this should become a loop - set(_lua_append_versions ${LUA_VERSIONS5}) - endif () - - list(APPEND _lua_include_subdirs "include/lua" "include") - - foreach (ver IN LISTS _lua_append_versions) - string(REGEX MATCH "^([0-9]+)\\.([0-9]+)$" _ver "${ver}") - list(APPEND _lua_include_subdirs - include/lua${CMAKE_MATCH_1}${CMAKE_MATCH_2} - include/lua${CMAKE_MATCH_1}.${CMAKE_MATCH_2} - include/lua-${CMAKE_MATCH_1}.${CMAKE_MATCH_2} - ) - endforeach () - - set(_lua_include_subdirs "${_lua_include_subdirs}" PARENT_SCOPE) - set(_lua_append_versions "${_lua_append_versions}" PARENT_SCOPE) -endfunction(_lua_set_version_vars) - -function(_lua_check_header_version _hdr_file) - # At least 5.[012] have different ways to express the version - # so all of them need to be tested. Lua 5.2 defines LUA_VERSION - # and LUA_RELEASE as joined by the C preprocessor, so avoid those. - file(STRINGS "${_hdr_file}" lua_version_strings - REGEX "^#define[ \t]+LUA_(RELEASE[ \t]+\"Lua [0-9]|VERSION([ \t]+\"Lua [0-9]|_[MR])).*") - - string(REGEX REPLACE ".*;#define[ \t]+LUA_VERSION_MAJOR[ \t]+\"([0-9])\"[ \t]*;.*" "\\1" LUA_VERSION_MAJOR ";${lua_version_strings};") - if (LUA_VERSION_MAJOR MATCHES "^[0-9]+$") - string(REGEX REPLACE ".*;#define[ \t]+LUA_VERSION_MINOR[ \t]+\"([0-9])\"[ \t]*;.*" "\\1" LUA_VERSION_MINOR ";${lua_version_strings};") - string(REGEX REPLACE ".*;#define[ \t]+LUA_VERSION_RELEASE[ \t]+\"([0-9])\"[ \t]*;.*" "\\1" LUA_VERSION_PATCH ";${lua_version_strings};") - set(LUA_VERSION_STRING "${LUA_VERSION_MAJOR}.${LUA_VERSION_MINOR}.${LUA_VERSION_PATCH}") - else () - string(REGEX REPLACE ".*;#define[ \t]+LUA_RELEASE[ \t]+\"Lua ([0-9.]+)\"[ \t]*;.*" "\\1" LUA_VERSION_STRING ";${lua_version_strings};") - if (NOT LUA_VERSION_STRING MATCHES "^[0-9.]+$") - string(REGEX REPLACE ".*;#define[ \t]+LUA_VERSION[ \t]+\"Lua ([0-9.]+)\"[ \t]*;.*" "\\1" LUA_VERSION_STRING ";${lua_version_strings};") - endif () - string(REGEX REPLACE "^([0-9]+)\\.[0-9.]*$" "\\1" LUA_VERSION_MAJOR "${LUA_VERSION_STRING}") - string(REGEX REPLACE "^[0-9]+\\.([0-9]+)[0-9.]*$" "\\1" LUA_VERSION_MINOR "${LUA_VERSION_STRING}") - string(REGEX REPLACE "^[0-9]+\\.[0-9]+\\.([0-9]).*" "\\1" LUA_VERSION_PATCH "${LUA_VERSION_STRING}") - endif () - foreach (ver IN LISTS _lua_append_versions) - if (ver STREQUAL "${LUA_VERSION_MAJOR}.${LUA_VERSION_MINOR}") - set(LUA_VERSION_MAJOR ${LUA_VERSION_MAJOR} PARENT_SCOPE) - set(LUA_VERSION_MINOR ${LUA_VERSION_MINOR} PARENT_SCOPE) - set(LUA_VERSION_PATCH ${LUA_VERSION_PATCH} PARENT_SCOPE) - set(LUA_VERSION_STRING ${LUA_VERSION_STRING} PARENT_SCOPE) - return() - endif () - endforeach () -endfunction(_lua_check_header_version) - -_lua_set_version_vars() - -if (LUA_INCLUDE_DIR AND EXISTS "${LUA_INCLUDE_DIR}/lua.h") - _lua_check_header_version("${LUA_INCLUDE_DIR}/lua.h") -endif () - -if (NOT LUA_VERSION_STRING) - foreach (subdir IN LISTS _lua_include_subdirs) - unset(LUA_INCLUDE_PREFIX CACHE) - find_path(LUA_INCLUDE_PREFIX ${subdir}/lua.h - HINTS - ENV LUA_DIR - PATHS - ~/Library/Frameworks - /Library/Frameworks - /sw # Fink - /opt/local # DarwinPorts - /opt/csw # Blastwave - /opt - ) - if (LUA_INCLUDE_PREFIX) - _lua_check_header_version("${LUA_INCLUDE_PREFIX}/${subdir}/lua.h") - if (LUA_VERSION_STRING) - set(LUA_INCLUDE_DIR "${LUA_INCLUDE_PREFIX}/${subdir}") - break() - endif () - endif () - endforeach () -endif () -unset(_lua_include_subdirs) -unset(_lua_append_versions) - -if (LUA_VERSION_STRING) - set(_lua_library_names - lua${LUA_VERSION_MAJOR}${LUA_VERSION_MINOR} - lua${LUA_VERSION_MAJOR}.${LUA_VERSION_MINOR} - lua-${LUA_VERSION_MAJOR}.${LUA_VERSION_MINOR} - lua.${LUA_VERSION_MAJOR}.${LUA_VERSION_MINOR} - ) -endif () - -find_library(LUA_LIBRARY - NAMES ${_lua_library_names} lua - HINTS - ENV LUA_DIR - PATH_SUFFIXES lib - PATHS - ~/Library/Frameworks - /Library/Frameworks - /sw - /opt/local - /opt/csw - /opt -) -unset(_lua_library_names) - -if (LUA_LIBRARY) - # include the math library for Unix - if (UNIX AND NOT APPLE AND NOT BEOS) - find_library(LUA_MATH_LIBRARY m) - set(LUA_LIBRARIES "${LUA_LIBRARY};${LUA_MATH_LIBRARY}") - - # include dl library for statically-linked Lua library - get_filename_component(LUA_LIB_EXT ${LUA_LIBRARY} EXT) - if(LUA_LIB_EXT STREQUAL CMAKE_STATIC_LIBRARY_SUFFIX) - list(APPEND LUA_LIBRARIES ${CMAKE_DL_LIBS}) - endif() - - # For Windows and Mac, don't need to explicitly include the math library - else () - set(LUA_LIBRARIES "${LUA_LIBRARY}") - endif () -endif () - -include(FindPackageHandleStandardArgs) -# handle the QUIETLY and REQUIRED arguments and set LUA_FOUND to TRUE if -# all listed variables are TRUE -FIND_PACKAGE_HANDLE_STANDARD_ARGS(Lua - REQUIRED_VARS LUA_LIBRARIES LUA_INCLUDE_DIR - VERSION_VAR LUA_VERSION_STRING) - -mark_as_advanced(LUA_INCLUDE_DIR LUA_LIBRARY LUA_MATH_LIBRARY)