Eliminates lua-client and non-static libluv as test time dependencies
Note: the API for a public lua-client is not yet finished.
The interface needs to be adjusted to work in the embedded loop
of a nvim instance (to use it to talk between instances)
This makes it easier to see that -D is referring to the entire
"<variable>=<value>", rather than only <variable>. It also help syntax
highlighters highlight built-in variables.
Any logic involving CMAKE_BUILD_TYPE is automatically broken as it won't
work with multi-config generators. The only exception is if we
explicitly check whether the current generator is single-config as well.
Instead, use generator expressions or cmake variables that allows to set
options for certain build types only such as
INTERPROCEDURAL_OPTIMIZATION_<CONFIG>.
Opt to generate all headers with optimization level O2 with no debug
information for all build types as that is the simplest way to make it
behave the same for all generators.
Find modules should only link to libraries defined in the find module,
and not the main project. This helps the find modules be more self-contained and easier to understand.
* build: various cmake refactors and simplifications
- Add STATUS keyword to message to ensure messages are shown in the
correct order.
- Remove DEPS_CXX_COMPILER as we don't rely on C++ for any of our
dependencies.
- Simplify how msgpack and luv configure options are constructed.
- Rely on the default installation for luv instead of manually passing
configure, build and install commands.
- Simplify return code conditional.
* build: remove CMAKE_OSX_ARCHITECTURES_ALT_SEP workaround
CMAKE_OSX_ARCHITECTURES_ALT_SEP was defined as a workaround to prevent
the shell from interpreting `;`, which CMake uses as a list separator.
However, the same thing can be achieved by instead passing
CMAKE_OSX_ARCHITECTURES as a cache variable instead, which is a more
idiomatic way of achieving the same thing.
* build: define CMAKE_BUILD_TYPE before adding it to BUILD_TYPE_STRING
The problem with the current setup is that CMAKE_BUILD_TYPE is defined
after BUILD_TYPE_STRING. BUILD_TYPE_STRING will then be empty on the
first run, meaning that dependencies are built without a build type.
However, since CMAKE_BUILD_TYPE is a cache variable its value will
persist in subsequent runs. On the second run BUILD_TYPE_STRING will
have the correct value, but it's a different value from the ones the
dependencies were built with. This will force some dependencies to be
built again.
Fixes https://github.com/neovim/neovim/issues/21672.
Problem: On Windows, neovim's version is generated every time nvim is
built, even if code hasn't been changed. That is because version
generation is done based on a hash matching of a file and the content of
the file. And in Windows they don't match, because of the DOS
line-endings.
Solution: Write the file containing nvim version with UNIX line-endings.
clint takes around 5-10 seconds to lint tui/terminfo_defs.h. For CI this
is negligible, but it's annoying for local development as touching
terminfo_defs.h will skyrocket lint times. Furthermore, we have no
reason to touch or modify terminfo_defs.h as it's a generated file, so
linting it shouldn't be necessary. This should speed up "make lint" by
the same amount, so around 5-10 seconds.
Fix remaining clint errors and remove error suppression completely.
Rename the lint targets to align with the established naming convention:
- lintc-clint lints with clint.py.
- lintc-uncrustify lints with uncrustify.
- lintc runs both targets.
lintc is also provided as a make target for convenience.
After this change we can remove these files:
https://github.com/neovim/doc/tree/gh-pages/reports/clinthttps://github.com/neovim/doc/blob/main/ci/clint-errors.sh
Replace old-school cmake with the so-called "Modern CMake", meaning
preferring using targets and properties over directory settings and
variables. This allows greater flexibility, robustness and clarity over
how the code works.
The following deprecated commands will be replaced with their modern
alternatives that operates on a specific target, rather than all targets
in the current directory:
- add_compile_options -> target_compile_options
- include_directories -> target_include_directories
- link_libraries -> target_link_libraries
- add_definitions -> target_compile_definitions
There are mainly four main targets that we currently use: nvim, libnvim,
nvim-test (used by unittests) and ${texe} (used by
check-single-includes). The goal is to explicitly define the
dependencies of each target fully, rather than having everything be
dependent on everything else.
EXCLUDE filters out all elements containing regex, meaning it works on
both files and directories.
Also rename add_glob_targets to add_glob_target since only one target is
being created.
- Prevent duplicate version strings such as v0.8.0-v0.8.0.
- Change the format for git releases from v0.9.0-dev-67-g625ba79be to
v0.9.0-dev-67+g625ba79be.
Nvim versions are now:
release : v0.9.0
prerelease without git info: v0.9.0-dev
prerelease with git info : v0.9.0-dev-67+g625ba79be
This partially reverts commit 42aeb5c5b1.
Setting cmake policies is normally not required as
cmake_minimum_required automatically sets these. One exception is cmake
script mode (-P) since it automatically resets all policy changes.
Closes: https://github.com/neovim/neovim/issues/20286
Add --always flag to `git describe` so version generation succeeds if
current directory is in a git repo. If not in git repo, fall back to a
default version in the format vx.y.z-dev
Change the default build type to always be Debug, and allow only four
predefined build types: Debug, Release, RelWithDebInfo and MinRelSize.
Furthermore, flags meant for single-configuration generator (make,
ninja) will not be used for multi-configuration generators (visual
studio, Xcode), and flags meant for multi-configuration generators will
not be used for single-configuration generators.
This will allow Debug builds to be built with MSVC which requires that
all dependencies are also built with the Debug build type to avoid
runtime library mismatch.
The correct way to specify build type (for example Release) for
single-configuration generators (Make and Ninja) is to run
cmake -B build -D CMAKE_BUILD_TYPE=Release
cmake --build build
while for multi-configuration generators (Visual Studio, Xcode and Ninja
Multi-Config) is to run
cmake -B build
cmake --build build --config Release
Passing CMAKE_BUILD_TYPE for multi-config generators will now not only
not be used, but also generate a warning for the user.
Co-authored-by: dundargoc <gocundar@gmail.com>
- Use DIRECTORY instead of PATH in get_filename_component
- Use COMPILE_OPTIONS instead of COMPILE_FLAGS. COMPILE_FLAGS is treated
as a single string while COMPILE_OPTIONS is a list, meaning that cmake
will take care of any escaping and quoting automatically.
The targets will only format files that have been changed in current
branch compared to the master branch. This includes unstaged, staged and
committed files.
Add following make and cmake targets:
formatc - format changed c files
formatlua - format changed lua files
format - run formatc and formatlua
Remove scripts/uncrustify.sh as this deprecates it.
Problem:
Build fails without git or .git/.
ref #19289
Solution:
Fix the version generation logic.
Test cases:
If `git` is missing:
-- Using NVIM_VERSION_MEDIUM: v0.8.0-dev
If `.git/` is missing:
-- Git tag extraction failed:
fatal: not a git repository (or any of the parent directories): .git
-- Using NVIM_VERSION_MEDIUM: v0.8.0-dev
If `git describe` fails
-- Git tag extraction failed:
fatal: ...
-- Using NVIM_VERSION_MEDIUM: v0.8.0-dev
Co-authored-by: Justin M. Keyes <justinkz@gmail.com>
- only update git-version if both of these conditions are met:
- `git` command succeeds
- `versiondef_git.h` would change (SHA1-diff)
- else print a status/warning message
also move version generation out of Lua into cmake.
The general idea is that add_glob_targets creates a "touch file", a
dummy file that acts as a dependency in order to check which files are
outdated since the last time the target was run.
Remove RunUncrustify.cmake as it's no longer necessary. It was initially
introduced to silence its noisy output. The per-file targets will
suppress the noisy output from uncrustify, except for the very first
run.
Also remove DefCmdTarget.cmake since add_glob_target already
incorporates its functionality.
If `libintl` is a static library on macOS, we also need to explicitly
link with `libiconv` and the `CoreFoundation` framework. Otherwise, our
`HAVE_WORKING_LIBINTL` test erroneously fails.
Closes#19127Closes#19138
Problem:
RunTests.cmake adds $TEST_PATH to $TMPDIR with the implication that it
gives more isolation. But this is misleading because $TEST_PATH is only
defined once. Full test runs use the same $TMPDIR for all tests.
This was likely added with the intention of invoking RunTests.cmake
once-per-testfile from a wrapper than does the isolation/orchestration.
But even so, Nvim's vim_maketempdir() / vim_mktempdir() _already_
creates a unique tempdir per session.
Solution:
Don't append $TEST_PATH to $TMPDIR. Avoids confusion and makes the path
shorter.
FindLua.cmake is a copy from the cmake repo:
0419ecbcad/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
e322b5c864.
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.
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.
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.
Problem:
Since 6d57bb89c1#18543, luacheck is not found on some systems when
running the "lintlua" target.
Solution:
- Move the find_program() to the top-level CMakeLists.txt
- Define a def_cmd_target() function with fewer assumptions than the old
lint() function.
- Move "lintuncrustify" to src/nvim/CMakeLists.txt so it can reuse the
$LINT_NVIM_SOURCES already defined there.
- Make the lint targets _fatal_ by default. There is little reason for
the "lint" umbrella target defined in Makefile to exist if it's going
to ignore the absence of the actual linters.
- For now, keep the uncrustify call in a separate cmake script so that
it can be silenced (too noisy).
* build: move the logic for linters to cmake
Cmake is our source of truth. We should have as much of our build
process there as possible so everyone can make use of it.
* build: remove redundant check for ninja generator
The minimum cmake version as of writing this is 3.10, which has ninja
support.
Problem:
Since 22b52dd462#11501, log_path_init is called in log_init, so it is
now called at a deterministic time. So the "just in time" complexity of
log_path_init is no longer needed.
Solution:
Remove logic intended to try to "heal" partial initialization.
Problem:
winpty is only needed for Windows 8.1. Removing it reduces our build and code
complexity.
Solution:
- Remove winpty.
- Require Windows 10.
closes#18252
Problem:
Some tests were not passing on my machine, specifically in
`test/functional/api/vim_spec.lua` the two tests under
`describe('nvim_get_runtime_file...`
Solution:
Unset `XDG_DATA_DIRS` in the test runner.
Note: Window CI failed if we set it to the same value as `XDG_DATA_HOME`.
Set a maximum test run-time of 20 minutes to:
- fail faster
- avoid wasting CI resources
- set a bound on the test time (if tests take longer than 20 minutes we
need to invest in parallelizing them...)
Timeout looks like:
-- Tests exited non-zero: Process terminated due to timeout
-- No output to stderr.
CMake Error at /…/neovim/cmake/RunTests.cmake:86
(message):
functional tests failed with error: Process terminated due to timeout
When libluv is built statically it appends an '_a' suffix to the library
name. This affects CMake's ability to find the bundled version of libluv
(which is built statically) when there is a dynamic version of the
library also built on the system, which does not have the '_a' suffix.
Prioritize searching for 'libluv_a' first before falling back to
'libluv'. This will ensure the bundled version of libluv is always found
first before any system versions of libluv. In cases where we are not
using bundled libraries, CMake will still safely fall through to finding
'libluv' since package managers do not typically provide static system
libraries.
`find_package(Foo ...)` expects to find a file FindFoo.cmake and the
resulting variables to be named `Foo_...`. If those don't all match up,
then the detection does not work properly.
Closes#13262
Problem: On Windows with the MSVC build, gettext-translation
"Questa è già la" displays as "Questa <e8> gi<e0> la".
Solution: Fix iconv detection iconv when building gettext.
So HAVE_ICONV is correctly defined when building nvim.
* fix gettext mb chars on MSVC
* fix libintl detection failure on MSVC
fixes#11749
This avoids invoking CMake after a new commit, which might take 15s on
some systems.
Skipped on CMake < 3.2.0 (missing BYPRODUCTS support).
Co-Authored-By: Justin M. Keyes <justinkz@gmail.com>
We are using "LIBFOO" prefixes for `LIBFOO_INCLUDE_DIRS` etc, and
therefore need to use this for the PREFIX with LibFindMacros also, so
that `"${${PREFIX}_FIND_VERSION}"` gets handled there properly.
The alternative would be to either manually handle/set the upper/mixed case
variants of those variable additionally, which is not as easy.
Keeping the existing names is useful for packagers.
Before this the version requirements with
`find_package(Unibilium 2.0 REQUIRED)` was not handled (a49cf5126), and
it prepares for using a required version with libvterm
(initially/wrongly ported in 1896c72a5).
* build: require unibilium>=2.0
This also ports FindUnibilium to LibFindMacros, which was planned
anyway, and makes the version check easier.
With an older Unibilium our fallback code in `terminfo_from_builtin`
will not work (because it assumes the new data structures from 2.0.0 [1]),
and nvim would crash later because of `ut` being NUL.
1: 42f3cdd284
* tests: move "busted" dir to "test"
It is used for outputHandlers only, and clearly belongs to the tests.
Use the full module name with the `-o` option to `busted` then for
clarity.
* luacheck
* test/busted/outputHandlers/TAP.lua: use/extend upstream
* build: FindLibIntl: fix warning about CMP0075
The common pattern elsewhere to set this only during the check, and here
it was not unset, resulting in a warning later (on Alpine 3.10):
-- Found Iconv
-- Looking for pthread.h
CMake Warning (dev) at /usr/share/cmake/Modules/CheckIncludeFile.cmake:80 (message):
Policy CMP0075 is not set: Include file check macros honor
CMAKE_REQUIRED_LIBRARIES. Run "cmake --help-policy CMP0075" for policy
details. Use the cmake_policy command to set the policy and suppress this
warning.
CMAKE_REQUIRED_LIBRARIES is set to:
/usr/lib/libintl.so
For compatibility with CMake 3.11 and below this check is ignoring it.
Call Stack (most recent call first):
/usr/share/cmake/Modules/FindThreads.cmake:105 (CHECK_INCLUDE_FILE)
CMakeLists.txt:482 (find_package)
This warning is for project developers. Use -Wno-dev to suppress it.
-- Looking for pthread.h - found
* build: remove lists / REMOVE_ITEM around check_c_source_compiles
- Move .luacheckrc to root, add read_globals=vim
- Simplify lualint target, run it on all lua files
- Lint preload.lua, but ignore W211
- Remove testlint target, included in lualint (and lint)
- Clean up .luacheckrc
Keeps using add_definitions for compatibility with older CMake.
Newer CMake (3.12) would have `add_compile_definitions`, but it is not
required, since `add_defitions` was meant to be used for
compile/preprocessor definitions initially anyway.
Ref: https://github.com/neovim/neovim/pull/4389
The variables are not meant to be defined there really, but only with
the third-party project.
Using them, e.g. with the following, would actually result in libvterm
not being found then:
make CMAKE_EXTRA_FLAGS="-DUSE_BUNDLED_LIBVTERM=ON" \
DEPS_CMAKE_FLAGS="-DUSE_BUNDLED=OFF -DUSE_BUNDLED_LIBVTERM=ON"
In https://github.com/neovim/neovim/pull/6357 they were renamed to
`USE_BUNDLED_X` from `X_USE_BUNDLED`, but the above reasoning applies
to the old names, too.
Internally `CMAKE_PREFIX_PATH` is used to add the built/bundled third
party packages for `find_package`, so there is no reason to e.g. query
the values via `load_cache` for example from the third-party project.
Just set it from `${CMAKE_HOST_SYSTEM_NAME}` directly, instead of passing it
from the main CMake file (CMAKE_SYSTEM_NAME defaults to it, but is empty
in script mode).
Initially added in 9ce81f7b2, but then even used with unrelated commands
(that do not use RunTests.cmake, e.g. 221f6fffa).
The feature is very rudimentary [1], and causes problems when not using
bundled deps, where it might then fail to find `busted.runner` due to
this.
E.g. with "-DUSE_BUNDLED=OFF -DUSE_BUNDLED_LUAROCKS=ON":
`.deps/usr/bin/busted` contains `exec '/usr/bin/luajit' -e …`, i.e. it
run luajit with adjusted lpath etc.
But using `--lua /usr/bin/luajit` then causes this `busted` wrapper to
be replaced with just `/usr/bin/luajit`, i.e. it is missing the adjusted
lpath then.
1: 9eb6c9cf93
- allow for passing in BUSTED_ARGS via env
- quote values of TEST_TAG/TEST_FILTER
Previously TEST_FILTER="'foo bar'" was required.
This allows for:
make functionaltest TEST_FILE=test/functional/terminal/tui_spec.lua \
BUSTED_ARGS="--no-keep-going --shuffle" \
TEST_FILTER="TUI background color handles"
* RunTests.cmake: BUILD_DIR for Xtest files, isolated TMPDIR
Assume relative path for given TEST_PATH.
The package argument is case sensitive, which is important to handle
X_FIND_REQUIRED properly, i.e. error out early if it is not found:
CMake Error at /usr/share/cmake-3.14/Modules/FindPackageHandleStandardArgs.cmake:137 (message):
Could NOT find Unibilium (missing: UNIBILIUM_LIBRARY UNIBILIUM_INCLUDE_DIR)
Otherwise it would continue until:
CMake Error: The following variables are used in this project, but they
are set to NOTFOUND.
Please set them or make sure they are set and tested correctly in the
CMake files:
UNIBILIUM_INCLUDE_DIR (ADVANCED)
Quickly checked via `rg 'find_package_handle_standard|find_package.*REQUIRED' -I | sort`.
Ref: https://gitlab.kitware.com/cmake/cmake/issues/19413
There was never any investigation done to determine whether using
jemalloc was actually a net benefit for nvim. It has been a portability
limitation and adds another factor to consider when triaging issues.
