Team reviewers is a nice feature that comes with a severe drawback: it
makes testing the workflows incredibly difficult as they won't work
without a similar token by the tester.
Currently, the release build picks up headers in
`/Library/Frameworks/Mono.framework/Headers`. You can verify this by
downloading the latest nightly build and checking the output of `nvim
--version`.
These headers are likely to be from a different version of `libintl` than the
one we link to. Let's avoid usage of them by setting `CMAKE_FIND_FRAMEWORK` to
`NEVER`.
Having multiple release artifacts per platform is a maintenance burden.
Furthermore, it is a maintenance burden that doesn't directly improve
the Nvim editor itself. The releases are meant to be a quick way for
users to try out and use neovim on their platform and was never intended
to be a buffet of releases for every conceivable setup.
Users are encouraged to the following replacements:
- Github action `action-setup-vim` to have neovim installed on their
PATH for their CI jobs. See https://github.com/rhysd/action-setup-vim.
- Use the appimage, either as is or by extracting it
- To use as is, run `chmod u+x nvim.appimage && ./nvim.appimage`
- If your system does not have FUSE you can extract the appimage with
`./nvim.appimage --appimage-extract && ./squashfs-root/usr/bin/nvim`
- Build it manually. See https://github.com/neovim/neovim/wiki/Building-Neovim.
Work on https://github.com/neovim/neovim/issues/22684
The labeler adds "lua" label to too many files. When there is already
a "treesitter" or "lsp" label, a "lua" label isn't useful. Instead it's
better to add the label manually to PRs for general Lua support.
Installing the ruby provider takes anything between 1 and 1.5 minutes on
Windows, which is a big drain on our CI. Remove it until we find a more
sustainable solution.
The lua client is no longer needed after
d6279f9392. One of its dependencies,
mpack, is still needed however. Remove lua-nvim and replace it with
lua-mpack.
The other packages are most likely not needed as we no longer run tests
for external dependencies.
If one uses .deps when DEPS_BUILD_DIR is defined in another location it
leads to very surprising behaviors, as it looks for libraries in other
places other than .deps.
Currently files to install in runtime/ is detected by recursive glob
pattern which has two problems:
- cmake needs to do a of work at config time and
build/runtime/cmake_install.cmake becomes HUGE (2.5MB, biggest config file)
- we need to explicitly specify each file suffix used in the entire
runtime, which is duplication of information.
These globs specify every single file in a subdirectory.
Thus, we can just install every runtime/ subdirectory as a single
install command. Furthermore, at the top-level, only .vim and .lua files
need to be installed.
Further possible refactor: we could move files which does not belong
in $PREFIX/runtime out of $REPO/runtime. Then runtime could be installed
with a single install_helper(DIRECTORY ...) command.
Having separate directory location causes failures to be inconsistent
and ultimately confusing. A common problem is a file with a particular
name is searched for the entire repository, which gives different
results if the dependency directory is inside the neovim directory or
outside of it.
Having to specify CI_BUILD for every CI job requires boilerplate. More
importantly, it's easy to forget to enable CI_BUILD, as seen by
8a20f9f98a. It's simpler to remember to
turn CI_BUILD off when a job errors instead of remembering that every
new job should have CI_BUILD on.
Multi-config generators can be tricky so testing them would be good.
Also test GCC release and MinSizeRel build types as they're prone to
unusual warnings. Remove release testing from test.yml as this is a
sufficient replacement.
Having a workflow that only builds neovim without running all of the
tests is a cheap way to test the build still works without burning too
much CI time.
libtool, autoconf, automake and perl are no longer dependencies of
neovim and doesn't need to be installed in CI anymore. The dependencies
and the commit that removed them as dependencies are the following:
libtool: b05100a9ea
perl: 20a932cb72
autoconf+automake: e23c5fda0a
ci: add GCC release testing
We currently have no release testing, so it's good to check for any
unwanted behavior on release builds as well. Prefer GCC over clang, as
GCC release builds seem to create more warnings on release compared to
debug.
Detect if on CI by checking that the CI environment variable is set to "true".
This is a common pattern among CI providers, including github actions and
cirrus.
Having CI scripts that is separate from the build system causes
tremendous amounts of problems, headaches and bugs. Testing the validity
of the scripts locally become near impossible as time goes on as they're
only vetted if it works on whatever CI provider we happened to have at
the time, with their own quirks and behavior.
The extra indirection between "cmake <-> general CI scripts <-> GHA" is
also a frequent source of problems, as the orchestration needs to be
done with environment variables, cmake flags and github actions matrix
strategy. This combination has turned out to be exceptionally fragile.
Examples:
15394b685513aa23b62ahttps://github.com/neovim/neovim/pull/22072#discussion_r1094390713
A lot of the code was inlined to .github/workflows/ci.yml without
further modifications. While this in itself doesn't integrate with our
build system any more than the current situation, it does
1. remove a level of indirection, and more importantly
2. allow us to slowly start integrating the CI into our build system now
that all the relevant code is in one place.
* ci: show all logs at the end of a run
The current CI won't show the logs on error due to early exit. This will
at least show the logs, although for all tests at once.
More specifically, move the job testing the oldest supported cmake into
its own job. This opens the way for other jobs to use powerful and
advanced cmake features such as choosing which files to use with the -S
flag.
Removed testing from this job as this probably won't reveal anything
that other jobs already doesn't already show, since the only difference
is the cmake version.
Using the base branch as cache means that pull requests won't be able to
use the cache from the master branch, since the master branch cache
doesn't have a base_ref as it's generated from a push. Removing base_ref
makes the cache key from master and PR branch the same, provided the any
build files don't change.
The CI somtimes freezes on a specific test, wasting 45 minutes for the
entire job. Adding a timeout of 15 minutes to functionaltest and 5
minutes to unittests will mitigate the problem.
Clang-tidy already does what check-single-includes does automatically on
top of its regular linting. It is also generator independent, so it
doesn't take an eternity to run on slower generators such as Visual
Studio.
The universal macos release is particularly sensitive to build system
changes. Adding a job that builds a universal binary whenever a cmake
file is changed will help prevent future release breaks.
Use the bundled libvterm dependency as the external package is outdated,
with the hopes of being able to use the external package once its
version meets our required version.
Co-authored-by: Christian Clason <c.clason@uni-graz.at>
This will ensure warnings are treated as errors when using MSVC.
Also fix const correctness warnings. The warnings in mbyte.c are false
positives that triggers this warning on MSVC v19.32 and lower, which our
CI still use. The (void *) casts can be removed once the CI MSVC version
has been upgraded to v19.33 or higher.
Running "make lintlua" will run both stylua and luacheck if both exist.
But this is not necessary as we already lint with stylua with the
stylua-action, so we only need to lint with luacheck on our own.
Problem:
Nvim has Lua but the "nvim" CLI can't easily be used to execute Lua
scripts, especially scripts that take arguments or produce output.
Solution:
- support "nvim -l [args...]" for running scripts. closes#15749
- exit without +q
- remove lua2dox_filter
- remove Doxyfile. This wasn't used anyway, because the doxygen config
is inlined in gen_vimdoc.py (`Doxyfile` variable).
- use "nvim -l" in docs-gen CI job
Examples:
$ nvim -l scripts/lua2dox.lua --help
Lua2DoX (0.2 20130128)
...
$ echo "print(vim.inspect(_G.arg))" | nvim -l - --arg1 --arg2
$ echo 'print(vim.inspect(vim.api.nvim_buf_get_text(1,0,0,-1,-1,{})))' | nvim +"put ='text'" -l -
TODO?
-e executes Lua code
-l loads a module
-i enters REPL _after running the other arguments_.
Problem:
The "system info" fields in the bug report take up a lot of space at the
top. That hides the most relevant part of the bug report. To read the
actual bug, you always have to scroll down.
Solution:
Move the "system info" fields to the bottom.
ci: remove "needs:response" label if author responds
The default behavior of the stale action is to indiscriminately remove
the `needs:response` label for any activity whatsoever, from anyone. The
other option is to turn it off completely, meaning the maintainers needs
to manually remove the label themselves when the author responds for an
issue to not close automatically. Neither of these behaviors are useful
to us.
This was set explicitly to ubuntu.22.04 as ubuntu-latest pointed to
ubuntu.20.04, and we needed 22.04 to have a new enough doxygen version
for this job to work. Now that ubuntu-latest points to 22.04 this
workaround is no longer needed.
asan_symbolize-14 gives a deprecation as it relies on outdated python
features. We can safely stop using asan_symbolize as it's only needed
for special cases such as cross compilation which we don't have to worry
about.
Using team reviewers when possible reduces the churn on the git history
as we'll be able to add or remove reviewers without needing to change
the workflow files.
This requires using Github fine-grained personal access tokens with Pull
Requests set to "Read and write" and Members to "Read-only".
If any commit message in the PR is either of type "feat" or is a
breaking change, then there's a high probability that news.txt should be
updated. Give an error if news.txt hasn't been updated in that case.
This workflow cannot 100% correctly determine if news.txt should be
updated even if the commit messages were exactly correct. The entries in
news.txt is determined by changes between releases, while the commit
messages are based on the master branch. While it is an approximation,
it is still a useful enough one that it's still valuable to have this
job as a reminder even if it gives an error if it shouldn't. In these
cases it is perfectly fine to ignore the failure for this job.
The needs:response label should only be added and removed manually, and
the action's behavior of removing the label on any activity (e.g. title
change, removing reviewers) is unwanted.
Stale strategy is to never automatically stale anything. Only mark stale
issues or PRs if they get the `needs:response` label. In that case close
after 30 days if there hasn't been any activity.
This reverts commit d7e2229b41.
This workflow isn't required to pass to merge a PR any more. Using
pull_request_target to bypass the required check when using [skip ci] is
therefore no longer needed.
It's currently difficult to pinpoint the cause of a failure since all
tests are run even if the build steps fail. But since the build failed
the test will almost always fail as well as it's dependent on a
successful build, leading to many steps being marked as a failure even
though the real problem was the build step. Even worse, the default
behavior of GitHub Actions is to only automatically show the last failed
step, which is misleading if the build process fails since it'll show
the logs of the failing test step.
An easy solution would be to abort all subsequent steps if any steps
fail. This isn't optimal however, as we want all lint and test failures
to show on a single run instead of prematurely aborting on a single test
step.
We can solve both problems by dividing each job into two phases: the
build/installation phase and the test/lint phase, with a checkmark step
in between. The strategy is simple: if any step before the checkmark
step fails (the build phase), then abort all following steps. If any
step after the checkmark fails (the test phase), then show that test as
failed but continue running all tests.
Default is currently clang 14. GHA images are updated at least once per year, so
we don't need to manually install a newer clang version.
Also remove step for installing clang-13 since it's not needed anymore.
This includes both the `lintpy` make target and for CI. We're actively
trying to reduce our python usage, so this only seems to give warnings
for unimportant things such as exceeding the line after deleting python
code.
Switch back to Ubuntu 18.04 for buliding the appimage. This allows for
using the appimage on older systems that do not provide GLIBC_2.29.
Fixes#19711.
Fixes#20113.
Skipping the CI on documentation-only changes is no longer appropriate
as we now rely on CI to test parts of documentation, e.g.
test/functional/lua/help_spec.lua.
Ignore changes in contrib/ as it's for non-essential user contributions
that we don't need to test.
It's a leftover artifact that currently just acts as an unnecessary
intermediary script that calls the Makefile. It can be replaced by just
calling the Makefile directly.
Our previous mangling of gettext broke the `HAVE_WORKING_LIBINTL` test
because it prevented CMake from finding `libintl.h`. Let's fix that by
linking Gettext's `include` directory into `/usr/local` too.
Problem:
Dirs "config", "packaging", and "third-party" are all closely related
but this is not obvious from the layout. This adds friction for new
contributors.
Solution:
- rename config/ to cmake.config/
- rename test/config/ to test/cmakeconfig/ because it is used in Lua
tests: require('test.cmakeconfig.paths').
- rename packaging/ to cmake.packaging/
- rename third-party/ to cmake.deps/ (parallel with .deps/)
After some tweaks to our dep builds, we can now build a universal binary
for macOS by using `CMAKE_OSX_ARCHITECTURES`. So, let's do that. This
requires a number of additional changes:
1. We need to build on macOS 11, since earlier versions do not support
building universal (M1 + Intel) binaries.
2. We need to provision a universal `libintl`. The linker will look for
an ARM64 version of this library when linking the `nvim` binary.
While we're here:
1. Link statically to `libintl`. This allows to to avoid having to do
any install name rewriting or codesigning to package Neovim.
2. Bump the `MACOSX_DEPLOYMENT_TARGET` to `11`. We're already using a
`libintl` built by Homebrew (through the pre-installed version of
`gettext`), and that is built for macOS 11.
In order to ensure we link to `libintl.a` instead of `libintl.dylib`, we
have to make sure that CMake can't find the latter. This ideally should
be a matter of doing `brew unlink gettext`. However, CMake is too adept
at finding things that Homebrew has installed (even when not linked), so
we have to do a bit more than that. This appears in the additional step
ensuring static linkage to `libintl`.
We end up breaking some Homebrew-installed software in the process, and
some of these software is called during our build (e.g. curl, git,
wget). To avoid any adverse effects, let's just uninstall them.
Problem:
The release script bundles a system library (CoreServices) that was
added in #18294, which leads to errors on M1 since the architecture is
different from the Github runner.
Solution:
Skip CoreServices when bundling the libraries (as was done for the
CoreFoundation library that #18294 replaced with CoreServices).
* 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.
Since we're not running tests or other things that are more sensitive to
changes in the VM environment, use ubuntu-latest to avoid the busy work
of updating the VM image.
This lint job will ensure that the C codebase is properly formatted at
all times. This helps eliminate most of clint.py.
To save CI time, it's faster to manually compile uncrustify and cache
the binary instead of using homebrew (the apt-get package is too old).
previous: https://github.com/neovim/neovim/pull/14123
CI tests were disabled on drafts #18566 to manage the
large number of incoming jobs. While this did help, it had the drawback
of making the purpose of the ready-for-review a bit fuzzier. It went
from a clear "my PR is ready" signal to maintainers to somewhere between
"my PR is ready but I need the tests to confirm" to "please don't merge
yet, I just need to see the test results". Worse is that the specific
case of wanting to see the test results but not wanting it merged is
that this needs to be actively conveyed to the maintainers with a [DNM]
or a comment to not merge the PR yet. All of this causes weird
workarounds and noises which I believe isn't necessary.
The reason why I don't think this workaround is needed anymore is that
our CI now aborts a job if a new job from the same pull requests is
created, which makes the "10 simultaneous jobs per PR" situations that
triggered this not possible.
- Removed NSIS installer.
- Prevents undefined behaviour when two installations are performed to the same directory (NSIS + MSI).
- Reduced cost of maintaining two installers that do the same thing.
- Chose Wix MSI due to its better integration with Windows.
- Added Wix patch file to add neovim binaries to the system path during installation.
- Replaced neovim installer icons with better looking versions.
- Renamed neovim installer icons from logo.ico -> neovim.ico for all
icons to better reflect contents.
1. Add new pattern `runtime/doc/**`. This is a common case were the
contributor modifies only the help file but the doc gen would discard
their changes.
2. Add to the output what the changes after running doc gen would be.
[skip ci]
Repurpose the api-docs workflow to also run in all PR's but work only as
a check, if the changes in the PR introduce doc changes that are not
committed fail.
[skip ci]
