ci: inline build commands and remove before_script.sh (#22202)

Abstracting the build commands to a separate script makes it more
difficult to reason about it and more error-prone.

.github/workflows/ci.yml

@@ -16,7 +16,6 @@ concurrency:
   cancel-in-progress: true
 
 env:
-  CI_BUILD_DIR: ${{ github.workspace }}
   UNCRUSTIFY_VERSION: uncrustify-0.75.0
   # TEST_FILE: test/functional/core/startup_spec.lua
   # TEST_FILTER: foo
@@ -26,8 +25,6 @@ jobs:
     if: (github.event_name == 'pull_request' && github.base_ref == 'master') || (github.event_name == 'push' && github.ref == 'refs/heads/master')
     runs-on: ubuntu-22.04
     timeout-minutes: 10
-    env:
-      CC: gcc
     steps:
       - uses: actions/checkout@v3
 
@@ -67,7 +64,9 @@ jobs:
       - uses: ./.github/actions/cache
 
       - name: Build third-party deps
-        run: ./ci/before_script.sh
+        run: |
+          cmake -S cmake.deps -B "${DEPS_BUILD_DIR}" -G Ninja ${DEPS_CMAKE_FLAGS}
+          cmake --build "${DEPS_BUILD_DIR}"
 
       - if: "!cancelled()"
         name: Determine if run should be aborted
@@ -90,7 +89,7 @@ jobs:
 
       - if: success() || failure() && steps.abort_job.outputs.status == 'success'
         name: lintsh
-        run: make lintsh
+        run: cmake --build build --target lintsh
 
       - if: success() || failure() && steps.abort_job.outputs.status == 'success'
         name: uncrustify
@@ -119,8 +118,6 @@ jobs:
     if: (github.event_name == 'pull_request' && github.base_ref == 'master') || (github.event_name == 'push' && github.ref == 'refs/heads/master')
     runs-on: ubuntu-22.04
     timeout-minutes: 10
-    env:
-      CC: gcc
     steps:
       - uses: actions/checkout@v3
 
@@ -153,10 +150,14 @@ jobs:
       - uses: ./.github/actions/cache
 
       - name: Build third-party deps
-        run: ./ci/before_script.sh
+        run: |
+          cmake -S cmake.deps -B "${DEPS_BUILD_DIR}" -G Ninja ${DEPS_CMAKE_FLAGS}
+          cmake --build "${DEPS_BUILD_DIR}"
 
-      - name: Build nvim
+      - name: Build
-        run: make
+        run: |
+          cmake -B build -G Ninja
+          cmake --build build
 
       - if: "!cancelled()"
         name: Determine if run should be aborted
@@ -226,10 +227,23 @@ jobs:
       - uses: ./.github/actions/cache
 
       - name: Build third-party deps
-        run: ./ci/before_script.sh
+        run: |
+          if test "${FUNCTIONALTEST}" = "functionaltest-lua" ; then
+            DEPS_CMAKE_FLAGS="${DEPS_CMAKE_FLAGS} -D USE_BUNDLED_LUA=ON"
+          fi
+          cmake -S cmake.deps -B "${DEPS_BUILD_DIR}" -G Ninja ${DEPS_CMAKE_FLAGS}
+          cmake --build "${DEPS_BUILD_DIR}"
 
       - name: Build
-        run: ./ci/run_tests.sh build_nvim
+        run: |
+          if test -n "${CLANG_SANITIZER}" ; then
+            CMAKE_FLAGS="${CMAKE_FLAGS} -D CLANG_${CLANG_SANITIZER}=ON"
+          fi
+          cmake -B build -G Ninja ${CMAKE_FLAGS}
+          cmake --build build
+
+      - name: Prepare sanitizer
+        run: ./ci/run_tests.sh prepare_sanitizer
 
       - if: "!cancelled()"
         name: Determine if run should be aborted
@@ -251,8 +265,12 @@ jobs:
         run: ./ci/run_tests.sh oldtests
 
       - if: success() || failure() && steps.abort_job.outputs.status == 'success'
-        name: Install nvim
+        name: Install
-        run: ./ci/run_tests.sh install_nvim
+        run: cmake --install build
+
+      - if: success() || failure() && steps.abort_job.outputs.status == 'success'
+        name: Installtests
+        run: ./ci/run_tests.sh installtests
 
   old_cmake:
     name: Test oldest supported cmake
@@ -314,7 +332,7 @@ jobs:
           cmake -S cmake.deps -B $env:DEPS_BUILD_DIR -G Ninja -D CMAKE_BUILD_TYPE='RelWithDebInfo'
           cmake --build $env:DEPS_BUILD_DIR
 
-      - name: Build nvim
+      - name: Build
         run: |
           cmake -B build -G Ninja -D CMAKE_BUILD_TYPE='RelWithDebInfo' -D CI_BUILD=ON
           cmake --build build

.github/workflows/env.sh

@@ -3,14 +3,14 @@ set -e -u
 
 FLAVOR=${1:-}
 
-BUILD_DIR=$CI_BUILD_DIR/build
+BUILD_DIR=$GITHUB_WORKSPACE/build
-BIN_DIR=$CI_BUILD_DIR/bin
+BIN_DIR=$GITHUB_WORKSPACE/bin
 DEPS_BUILD_DIR=$HOME/nvim-deps
-INSTALL_PREFIX=$CI_BUILD_DIR/nvim-install
+INSTALL_PREFIX=$GITHUB_WORKSPACE/nvim-install
 LOG_DIR=$BUILD_DIR/log
 NVIM_LOG_FILE=$BUILD_DIR/.nvimlog
 VALGRIND_LOG=$LOG_DIR/valgrind-%p.log
-CACHE_DIR=$CI_BUILD_DIR/.cache
+CACHE_DIR=$GITHUB_WORKSPACE/.cache
 CACHE_UNCRUSTIFY=$CACHE_DIR/uncrustify
 DEPS_CMAKE_FLAGS=
 FUNCTIONALTEST=functionaltest

ci/before_script.sh

@@ -1,32 +0,0 @@
-#!/usr/bin/env bash
-
-set -e
-set -o pipefail
-
-# Test some of the configuration variables.
-if [[ -n "${GCOV}" ]] && [[ ! $(type -P "${GCOV}") ]]; then
-  echo "\$GCOV: '${GCOV}' is not executable."
-  exit 1
-fi
-
-if test "${FUNCTIONALTEST}" = "functionaltest-lua" ; then
-  DEPS_CMAKE_FLAGS="${DEPS_CMAKE_FLAGS} -D USE_BUNDLED_LUA=ON"
-fi
-
-mkdir -p "${DEPS_BUILD_DIR}"
-cd "${DEPS_BUILD_DIR}"
-echo "Configuring with '${DEPS_CMAKE_FLAGS}'."
-# shellcheck disable=SC2086
-cmake -G Ninja ${DEPS_CMAKE_FLAGS} "${CI_BUILD_DIR}/cmake.deps/"
-
-ninja || exit 1
-
-cd "${CI_BUILD_DIR}"
-
-# Install cluacov for Lua coverage.
-if [[ "$USE_LUACOV" == 1 ]]; then
-  "${DEPS_BUILD_DIR}/usr/bin/luarocks" install cluacov
-fi
-
-rm -rf "${LOG_DIR}"
-mkdir -p "${LOG_DIR}"

ci/run_tests.sh

@@ -3,17 +3,6 @@
 set -e
 set -o pipefail
 
-fail() {
-  local test_name="$1"
-  local message="$2"
-
-  : "${message:=Test $test_name failed}"
-
-  local full_msg="$test_name :: $message"
-  echo "Failed: $full_msg"
-  exit 1
-}
-
 print_core() {
   local app="$1"
   local core="$2"
@@ -58,7 +47,8 @@ check_core_dumps() {
     fi
   done
   if test "$app" != quiet; then
-    fail 'cores' 'Core dumps found'
+    echo 'Core dumps found'
+    exit 1
   fi
 }
 
@@ -81,7 +71,8 @@ check_logs() {
     rm "${log}"
   done
   if test -n "${err}"; then
-    fail 'logs' 'Runtime errors detected.'
+    echo 'Runtime errors detected.'
+    exit 1
   fi
 }
 
@@ -97,17 +88,13 @@ check_sanitizer() {
 
 unittests() {(
   ulimit -c unlimited || true
-  if ! ninja -C "${BUILD_DIR}" unittest; then
+  ninja -C "${BUILD_DIR}" unittest || exit
-    fail 'unittests' 'Unit tests failed'
-  fi
   check_core_dumps "$(command -v luajit)"
 )}
 
 functionaltests() {(
   ulimit -c unlimited || true
-  if ! ninja -C "${BUILD_DIR}" "${FUNCTIONALTEST}"; then
+  ninja -C "${BUILD_DIR}" "${FUNCTIONALTEST}" || exit
-    fail 'functionaltests' 'Functional tests failed'
-  fi
   check_sanitizer "${LOG_DIR}"
   valgrind_check "${LOG_DIR}"
   check_core_dumps
@@ -117,7 +104,7 @@ oldtests() {(
   ulimit -c unlimited || true
   if ! make oldtest; then
     reset
-    fail 'oldtests' 'Legacy tests failed'
+    exit 1
   fi
   check_sanitizer "${LOG_DIR}"
   valgrind_check "${LOG_DIR}"
@@ -125,46 +112,41 @@ oldtests() {(
 )}
 
 check_runtime_files() {(
-  local test_name="$1" ; shift
   local message="$1"; shift
   local tst="$1"; shift
 
-  cd runtime || exit
+  for file in $(git -C runtime ls-files "$@"); do
-  for file in $(git ls-files "$@") ; do
     # Check that test is not trying to work with files with spaces/etc
     # Prefer failing the build over using more robust construct because files
     # with IFS are not welcome.
     if ! test -e "$file"; then
-      fail "$test_name" "It appears that $file is only a part of the file name"
+      echo "It appears that $file is only a part of the file name"
+      exit 1
     fi
     if ! test "$tst" "$INSTALL_PREFIX/share/nvim/runtime/$file"; then
-      fail "$test_name" "$(printf "%s%s" "$message" "$file")"
+      printf "%s%s" "$message" "$file"
+      exit 1
     fi
   done
 )}
 
-install_nvim() {(
+installtests() {(
-  if ! ninja -C "${BUILD_DIR}" install; then
-    fail 'install' 'make install failed'
-  fi
-
   "${INSTALL_PREFIX}/bin/nvim" --version
   if ! "${INSTALL_PREFIX}/bin/nvim" -u NONE -e -c ':help' -c ':qall'; then
     echo "Running ':help' in the installed nvim failed."
     echo "Maybe the helptags have not been generated properly."
-    fail 'help' 'Failed running :help'
+    echo 'Failed running :help'
+    exit 1
   fi
 
   # Check that all runtime files were installed
   check_runtime_files \
-    'runtime-install' \
     'It appears that %s is not installed.' \
     -e \
     '*.vim' '*.ps' '*.dict' '*.py' '*.tutor'
 
   # Check that some runtime files are installed and are executables
   check_runtime_files \
-    'not-exe' \
     'It appears that %s is not installed or is not executable.' \
     -x \
     '*.awk' '*.sh' '*.bat'
@@ -173,59 +155,22 @@ install_nvim() {(
   local genvimsynf=syntax/vim/generated.vim
   local gpat='syn keyword vimFuncName .*eval'
   if ! grep -q "$gpat" "${INSTALL_PREFIX}/share/nvim/runtime/$genvimsynf"; then
-    fail 'funcnames' "It appears that $genvimsynf does not contain $gpat."
+    echo "It appears that $genvimsynf does not contain $gpat."
+    exit 1
   fi
 )}
 
-build_nvim() {
+prepare_sanitizer() {
   check_core_dumps --delete quiet
-
-  if test -n "${CLANG_SANITIZER}" ; then
-    CMAKE_FLAGS="${CMAKE_FLAGS} -D CLANG_${CLANG_SANITIZER}=ON"
-  fi
-
-  mkdir -p "${BUILD_DIR}"
-  cd "${BUILD_DIR}"
-  echo "Configuring with '${CMAKE_FLAGS} $*'."
-  # shellcheck disable=SC2086
-  cmake -G Ninja ${CMAKE_FLAGS} "$@" "${CI_BUILD_DIR}"
-
-  echo "Building nvim."
-  ninja nvim || exit 1
-
   # Invoke nvim to trigger *San early.
-  if ! (bin/nvim --version && bin/nvim -u NONE -e -cq | cat -vet) ; then
+  if ! ("${BUILD_DIR}"/bin/nvim --version && "${BUILD_DIR}"/bin/nvim -u NONE -e -cq | cat -vet); then
     check_sanitizer "${LOG_DIR}"
     exit 1
   fi
   check_sanitizer "${LOG_DIR}"
-
-  cd "${CI_BUILD_DIR}"
 }
 
-# Run all tests (with some caveats) if no input argument is given
+rm -rf "${LOG_DIR}"
-if (($# == 0)); then
+mkdir -p "${LOG_DIR}"
-  tests=('build_nvim')
 
-  # Additional threads aren't created in the unit/old tests
+eval "$*" || exit
-  if test "$CLANG_SANITIZER" != "TSAN"; then
-    if test "${FUNCTIONALTEST}" != "functionaltest-lua"; then
-      tests+=('unittests')
-    fi
-    tests+=('oldtests')
-  fi
-
-  tests+=('functionaltests' 'install_nvim')
-else
-  tests=("$@")
-fi
-
-for i in "${tests[@]}"; do
-  eval "$i" || exit
-done
-
-if [[ -s "${GCOV_ERROR_FILE}" ]]; then
-  echo '=== Unexpected gcov errors: ==='
-  cat "${GCOV_ERROR_FILE}"
-  exit 1
-fi