ci: simplify how environment variables are used (#22067)

Having a clear separation between when we manipulate variables and when
we export them to GITHUB_ENV makes it less error-prone.

.github/workflows/ci.yml

@@ -11,12 +11,12 @@ on:
     paths-ignore:
       - 'contrib/**'
 
-# Cancel any in-progress CI runs for a PR if it is updated
 concurrency:
   group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.head_ref || github.sha }}
   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
@@ -59,9 +59,9 @@ jobs:
         run: |
           source_dir=uncrustify
           build_dir=uncrustify/build
-          cmake -S $source_dir -B $build_dir -G Ninja -DCMAKE_BUILD_TYPE=Release
+          cmake -S $source_dir -B $build_dir -G Ninja -D CMAKE_BUILD_TYPE=Release
           cmake --build $build_dir
-          mkdir -p $HOME/.cache
+          mkdir -p $CACHE_DIR
           cp $build_dir/uncrustify ${{ env.CACHE_UNCRUSTIFY }}
 
       - uses: ./.github/actions/cache
@@ -302,12 +302,12 @@ jobs:
 
       - name: Build deps
         run: |
-          cmake -S cmake.deps -B $env:DEPS_BUILD_DIR -G Ninja -DCMAKE_BUILD_TYPE='RelWithDebInfo'
+          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
         run: |
-          cmake -B build -G Ninja -DCMAKE_BUILD_TYPE='RelWithDebInfo' -DDEPS_PREFIX="$env:DEPS_PREFIX" -DCI_BUILD=ON
+          cmake -B build -G Ninja -D CMAKE_BUILD_TYPE='RelWithDebInfo' -D DEPS_PREFIX="$env:DEPS_PREFIX" -D CI_BUILD=ON
           cmake --build build
 
       - name: Install test deps

.github/workflows/env.sh

@@ -3,67 +3,77 @@ set -e -u
 
 FLAVOR=${1:-}
 
-cat <<EOF >> "$GITHUB_PATH"
-$HOME/.local/bin
-EOF
-
-cat <<EOF >> "$GITHUB_ENV"
-CI_BUILD_DIR=$GITHUB_WORKSPACE
-BUILD_DIR=$GITHUB_WORKSPACE/build
+BUILD_DIR=$CI_BUILD_DIR/build
+BIN_DIR=$HOME/.local/bin
 DEPS_BUILD_DIR=$HOME/nvim-deps
 INSTALL_PREFIX=$HOME/nvim-install
-LOG_DIR=$GITHUB_WORKSPACE/build/log
-NVIM_LOG_FILE=$GITHUB_WORKSPACE/build/.nvimlog
-VALGRIND_LOG=$GITHUB_WORKSPACE/build/log/valgrind-%p.log
-CACHE_NVIM_DEPS_DIR=$HOME/.cache/nvim-deps
-CACHE_MARKER=$HOME/.cache/nvim-deps/.ci_cache_marker
-CACHE_UNCRUSTIFY=$HOME/.cache/uncrustify
-EOF
-
+LOG_DIR=$BUILD_DIR/log
+NVIM_LOG_FILE=$BUILD_DIR/.nvimlog
+VALGRIND_LOG=$LOG_DIR/valgrind-%p.log
+CACHE_DIR=$HOME/.cache
+CACHE_NVIM_DEPS_DIR=$CACHE_DIR/nvim-deps
+CACHE_MARKER=$CACHE_NVIM_DEPS_DIR/.ci_cache_marker
+CACHE_UNCRUSTIFY=$CACHE_DIR/uncrustify
 DEPS_CMAKE_FLAGS=
 FUNCTIONALTEST=functionaltest
-BUILD_FLAGS="CMAKE_FLAGS=-DCI_BUILD=ON -DCMAKE_BUILD_TYPE=Debug -DCMAKE_INSTALL_PREFIX:PATH=$HOME/nvim-install -DBUSTED_OUTPUT_TYPE=nvim -DDEPS_PREFIX=$HOME/nvim-deps/usr -DMIN_LOG_LEVEL=3"
+CMAKE_FLAGS="-D CI_BUILD=ON -D CMAKE_BUILD_TYPE=Debug -D CMAKE_INSTALL_PREFIX:PATH=$INSTALL_PREFIX -D BUSTED_OUTPUT_TYPE=nvim -D DEPS_PREFIX=$DEPS_BUILD_DIR/usr -D MIN_LOG_LEVEL=3"
+CLANG_SANITIZER=
+ASAN_OPTIONS=
+UBSAN_OPTIONS=
+TSAN_OPTIONS=
 
 case "$FLAVOR" in
   asan)
-    cat <<EOF >> "$GITHUB_ENV"
-CLANG_SANITIZER=ASAN_UBSAN
-ASAN_OPTIONS=detect_leaks=1:check_initialization_order=1:log_path=$GITHUB_WORKSPACE/build/log/asan:intercept_tls_get_addr=0
-UBSAN_OPTIONS=print_stacktrace=1 log_path=$GITHUB_WORKSPACE/build/log/ubsan
-EOF
+    CLANG_SANITIZER=ASAN_UBSAN
+    ASAN_OPTIONS="detect_leaks=1:check_initialization_order=1:log_path=$LOG_DIR/asan:intercept_tls_get_addr=0"
+    UBSAN_OPTIONS="print_stacktrace=1 log_path=$LOG_DIR/ubsan"
     ;;
   tsan)
-    cat <<EOF >> "$GITHUB_ENV"
-TSAN_OPTIONS=log_path=$GITHUB_WORKSPACE/build/log/tsan
-CLANG_SANITIZER=TSAN
-EOF
+    TSAN_OPTIONS=log_path=$LOG_DIR/tsan
+    CLANG_SANITIZER=TSAN
     ;;
   uchar)
-    cat <<EOF >> "$GITHUB_ENV"
-BUILD_UCHAR=1
-EOF
+    CMAKE_FLAGS+=" -D UNSIGNED_CHAR=ON"
     ;;
   lintc)
-# Re-enable once system deps are available
-#    BUILD_FLAGS="$BUILD_FLAGS -DLIBLUV_LIBRARY:FILEPATH=/usr/lib/$(dpkg-architecture -qDEB_HOST_MULTIARCH)/lua/5.1/luv.so -DLIBLUV_INCLUDE_DIR:PATH=/usr/include/lua5.1"
+    # Re-enable once system deps are available
+    #    CMAKE_FLAGS+=" -D LIBLUV_LIBRARY:FILEPATH=/usr/lib/$(dpkg-architecture -qDEB_HOST_MULTIARCH)/lua/5.1/luv.so -D LIBLUV_INCLUDE_DIR:PATH=/usr/include/lua5.1"
 
     # Ideally all dependencies should external for this job, but some
     # dependencies don't have the required version available. We use the
     # bundled versions for these with the hopes of being able to remove them
     # later on.
-    DEPS_CMAKE_FLAGS="$DEPS_CMAKE_FLAGS -DUSE_BUNDLED=OFF -DUSE_BUNDLED_LUV=ON -DUSE_BUNDLED_LIBVTERM=ON"
+    DEPS_CMAKE_FLAGS+=" -D USE_BUNDLED=OFF -D USE_BUNDLED_LUV=ON -D USE_BUNDLED_LIBVTERM=ON"
     ;;
   functionaltest-lua)
-    BUILD_FLAGS="$BUILD_FLAGS -DPREFER_LUA=ON"
+    CMAKE_FLAGS+=" -D PREFER_LUA=ON"
     FUNCTIONALTEST=functionaltest-lua
-    DEPS_CMAKE_FLAGS="$DEPS_CMAKE_FLAGS -DUSE_BUNDLED_LUAJIT=OFF"
+    DEPS_CMAKE_FLAGS+=" -D USE_BUNDLED_LUAJIT=OFF"
     ;;
   *)
     ;;
 esac
 
 cat <<EOF >> "$GITHUB_ENV"
-$BUILD_FLAGS
+CMAKE_FLAGS=$CMAKE_FLAGS
+BUILD_DIR=$BUILD_DIR
+DEPS_BUILD_DIR=$DEPS_BUILD_DIR
 DEPS_CMAKE_FLAGS=$DEPS_CMAKE_FLAGS
 FUNCTIONALTEST=$FUNCTIONALTEST
+INSTALL_PREFIX=$INSTALL_PREFIX
+LOG_DIR=$LOG_DIR
+NVIM_LOG_FILE=$NVIM_LOG_FILE
+VALGRIND_LOG=$VALGRIND_LOG
+CACHE_DIR=$HOME/.cache
+CACHE_NVIM_DEPS_DIR=$CACHE_NVIM_DEPS_DIR
+CACHE_MARKER=$CACHE_MARKER
+CACHE_UNCRUSTIFY=$CACHE_UNCRUSTIFY
+CLANG_SANITIZER=$CLANG_SANITIZER
+ASAN_OPTIONS=$ASAN_OPTIONS
+UBSAN_OPTIONS=$UBSAN_OPTIONS
+TSAN_OPTIONS=$TSAN_OPTIONS
+EOF
+
+cat <<EOF >> "$GITHUB_PATH"
+$BIN_DIR
 EOF

ci/before_cache.sh

@@ -7,10 +7,7 @@ CI_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 # shellcheck source-path=SCRIPTDIR
 source "${CI_DIR}/common/suite.sh"
 
-mkdir -p "${HOME}/.cache"
-
-echo "before_cache.sh: cache size"
-du -chd 1 "${HOME}/.cache" | sort -rh | head -20
+mkdir -p "$CACHE_DIR"
 
 # Update the third-party dependency cache only if the build was successful.
 if ended_successfully && [ -d "${DEPS_BUILD_DIR}" ]; then

ci/before_script.sh

@@ -10,7 +10,7 @@ if [[ -n "${GCOV}" ]] && [[ ! $(type -P "${GCOV}") ]]; then
 fi
 
 if test "${FUNCTIONALTEST}" = "functionaltest-lua" ; then
-  DEPS_CMAKE_FLAGS="${DEPS_CMAKE_FLAGS} -DUSE_BUNDLED_LUA=ON"
+  DEPS_CMAKE_FLAGS="${DEPS_CMAKE_FLAGS} -D USE_BUNDLED_LUA=ON"
 fi
 
 mkdir -p "${DEPS_BUILD_DIR}"

ci/install.sh

@@ -8,7 +8,7 @@ echo "Install neovim module for Python."
 CC=cc python3 -m pip -q install --user --upgrade pynvim
 
 echo "Install neovim RubyGem."
-gem install --no-document --bindir "$HOME/.local/bin" --user-install --pre neovim
+gem install --no-document --bindir "$BIN_DIR" --user-install --pre neovim
 
 echo "Install neovim npm package"
 npm install -g neovim

ci/run_tests.sh

@@ -13,7 +13,7 @@ build_nvim() {
   check_core_dumps --delete quiet
 
   if test -n "${CLANG_SANITIZER}" ; then
-    CMAKE_FLAGS="${CMAKE_FLAGS} -DCLANG_${CLANG_SANITIZER}=ON"
+    CMAKE_FLAGS="${CMAKE_FLAGS} -D CLANG_${CLANG_SANITIZER}=ON"
   fi
 
   mkdir -p "${BUILD_DIR}"

src/nvim/CMakeLists.txt

@@ -166,13 +166,14 @@ if(CI_BUILD)
     target_compile_options(main_lib INTERFACE -WX)
   else()
     target_compile_options(main_lib INTERFACE -Werror)
-    if(DEFINED ENV{BUILD_UCHAR})
-      # Get some test coverage for unsigned char
-      target_compile_options(main_lib INTERFACE -funsigned-char)
-    endif()
   endif()
 endif()
 
+option(UNSIGNED_CHAR "Set char to be unsigned" OFF)
+if(UNSIGNED_CHAR)
+  target_compile_options(main_lib INTERFACE -funsigned-char)
+endif()
+
 list(APPEND CMAKE_REQUIRED_INCLUDES "${MSGPACK_INCLUDE_DIRS}")
 check_c_source_compiles("
 #include <msgpack.h>