From 5cec6903e720333e6ca222bb914259c27ee199b2 Mon Sep 17 00:00:00 2001 From: James Lamb Date: Sun, 14 Jul 2024 05:41:38 -0700 Subject: [PATCH] [ci] [R-package] add CI jobs covering more CRAN "additional checks", fix R_NO_REMAP warnings (fixes #6369) (#6523) * [ci] [R-package] add CI jobs testing newer compilers * set up vignette-processing dependencies * remove tinytex * set up pandoc * allow NOTEs, one more R_NO_REMAP guard * restore other CI * one more R_NO_REMAP guard * fail builds producing more than 1 NOTE * update approach for running R CMD check * fix filepath * fix paths * fix variable name * echo lines * fix piping * fix file extension * clean up * make R CMD check stricter --- .ci/install-clang-devel.sh | 74 -------------------------- .ci/run-r-cmd-check.sh | 46 +++++++++++++++++ .ci/test_r_package.sh | 31 +++-------- .github/workflows/r_package.yml | 92 ++++++++++++++++++++++----------- R-package/src/lightgbm_R.cpp | 6 +++ R-package/src/lightgbm_R.h | 6 +++ include/LightGBM/utils/log.h | 7 +++ 7 files changed, 133 insertions(+), 129 deletions(-) delete mode 100755 .ci/install-clang-devel.sh create mode 100755 .ci/run-r-cmd-check.sh diff --git a/.ci/install-clang-devel.sh b/.ci/install-clang-devel.sh deleted file mode 100755 index 3556fccae..000000000 --- a/.ci/install-clang-devel.sh +++ /dev/null @@ -1,74 +0,0 @@ -#!/bin/bash - -# [description] -# -# Installs a development version of clang and the other LLVM tools. -# - -set -e -E -u -o pipefail - -CLANG_VERSION=${1} - -apt-get autoremove -y --purge \ - clang-* \ - libclang-* \ - libunwind-* \ - llvm-* - -apt-get update -y -apt-get install --no-install-recommends -y \ - gnupg \ - lsb-release \ - software-properties-common \ - wget - -wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add - - -# ref: https://apt.llvm.org/ -add-apt-repository -y "deb http://apt.llvm.org/unstable/ llvm-toolchain main" -add-apt-repository -y "deb-src http://apt.llvm.org/unstable/ llvm-toolchain main" -add-apt-repository -y "deb http://apt.llvm.org/unstable/ llvm-toolchain-${CLANG_VERSION} main" || true -add-apt-repository -y "deb-src http://apt.llvm.org/unstable/ llvm-toolchain-${CLANG_VERSION} main" || true -apt-get update -y - -apt-get install -y --no-install-recommends \ - clang-${CLANG_VERSION} \ - clangd-${CLANG_VERSION} \ - clang-format-${CLANG_VERSION} \ - clang-tidy-${CLANG_VERSION} \ - clang-tools-${CLANG_VERSION} \ - lldb-${CLANG_VERSION} \ - lld-${CLANG_VERSION} \ - llvm-${CLANG_VERSION}-dev \ - llvm-${CLANG_VERSION}-tools \ - libomp-${CLANG_VERSION}-dev \ - libc++-${CLANG_VERSION}-dev \ - libc++abi-${CLANG_VERSION}-dev \ - libclang-common-${CLANG_VERSION}-dev \ - libclang-${CLANG_VERSION}-dev \ - libclang-cpp${CLANG_VERSION}-dev \ - libunwind-${CLANG_VERSION}-dev - -# overwriting the stuff in /usr/bin is simpler and more reliable than -# updating PATH, LD_LIBRARY_PATH, etc. -cp --remove-destination /usr/lib/llvm-${CLANG_VERSION}/bin/* /usr/bin/ - -# per https://www.stats.ox.ac.uk/pub/bdr/Rconfig/r-devel-linux-x86_64-fedora-clang -# -# clang was built to use libc++: for a version built to default to libstdc++ -# (as shipped by Fedora/Debian/Ubuntu), add -stdlib=libc++ to CXX -# and install the libcxx-devel/libc++-dev package. -mkdir -p "${HOME}/.R" - -cat << EOF > "${HOME}/.R/Makevars" -CXX += -stdlib=libc++ -CXX11 += -stdlib=libc++ -CXX14 += -stdlib=libc++ -CXX17 += -stdlib=libc++ -CXX20 += -stdlib=libc++ -EOF - -echo "" -echo "done installing clang" -clang --version -echo "" diff --git a/.ci/run-r-cmd-check.sh b/.ci/run-r-cmd-check.sh new file mode 100755 index 000000000..239425cf4 --- /dev/null +++ b/.ci/run-r-cmd-check.sh @@ -0,0 +1,46 @@ +#!/bin/bash + +set -e -u -o pipefail + +PKG_TARBALL="${1}" +declare -i ALLOWED_CHECK_NOTES=${2} + +# 'R CMD check' redirects installation logs to a file, and returns +# a non-0 exit code if ERRORs are raised. +# +# The '||' here gives us an opportunity to echo out the installation +# logs prior to exiting the script. +check_succeeded="yes" +R CMD check "${PKG_TARBALL}" \ + --as-cran \ + --run-donttest \ +|| check_succeeded="no" + +CHECK_LOG_FILE=lightgbm.Rcheck/00check.log +BUILD_LOG_FILE=lightgbm.Rcheck/00install.out + +echo "R CMD check build logs:" +cat "${BUILD_LOG_FILE}" + +if [[ $check_succeeded == "no" ]]; then + echo "R CMD check failed" + exit 1 +fi + +# WARNINGs or ERRORs should be treated as a failure +if grep -q -E "WARNING|ERROR" "${CHECK_LOG_FILE}"; then + echo "WARNINGs or ERRORs have been found by R CMD check" + exit 1 +fi + +# Allow a configurable number of NOTEs. +# Sometimes NOTEs are raised in CI that wouldn't show up on an actual CRAN submission. +set +e +NUM_CHECK_NOTES=$( + grep -o -E '[0-9]+ NOTE' "${CHECK_LOG_FILE}" \ + | sed 's/[^0-9]*//g' +) +if [[ ${NUM_CHECK_NOTES} -gt ${ALLOWED_CHECK_NOTES} ]]; then + echo "Found ${NUM_CHECK_NOTES} NOTEs from R CMD check. Only ${ALLOWED_CHECK_NOTES} are allowed" + exit 1 +fi diff --git a/.ci/test_r_package.sh b/.ci/test_r_package.sh index a7f03c349..a68e006e8 100755 --- a/.ci/test_r_package.sh +++ b/.ci/test_r_package.sh @@ -149,8 +149,8 @@ fi Rscript --vanilla -e "options(install.packages.compile.from.source = '${compile_from_source}'); install.packages(${packages}, repos = '${CRAN_MIRROR}', lib = '${R_LIB_PATH}', dependencies = c('Depends', 'Imports', 'LinkingTo'), Ncpus = parallel::detectCores())" || exit 1 cd "${BUILD_DIRECTORY}" - -PKG_TARBALL="lightgbm_*.tar.gz" +PKG_TARBALL="lightgbm_$(head -1 VERSION.txt).tar.gz" +BUILD_LOG_FILE="lightgbm.Rcheck/00install.out" LOG_FILE_NAME="lightgbm.Rcheck/00check.log" if [[ $R_BUILD_TYPE == "cmake" ]]; then Rscript build_r.R -j4 --skip-install || exit 1 @@ -209,21 +209,10 @@ elif [[ $R_BUILD_TYPE == "cran" ]]; then cd ${R_CMD_CHECK_DIR} fi -# fails tests if either ERRORs or WARNINGs are thrown by -# R CMD CHECK -check_succeeded="yes" -R CMD check ${PKG_TARBALL} \ - --as-cran \ - --run-donttest \ -|| check_succeeded="no" - -echo "R CMD check build logs:" -BUILD_LOG_FILE=lightgbm.Rcheck/00install.out -cat ${BUILD_LOG_FILE} - -if [[ $check_succeeded == "no" ]]; then - exit 1 -fi +declare -i allowed_notes=0 +bash "${BUILD_DIRECTORY}/.ci/run-r-cmd-check.sh" \ + "${PKG_TARBALL}" \ + "${allowed_notes}" # ensure 'grep --count' doesn't cause failures set +e @@ -242,18 +231,12 @@ if [[ $R_BUILD_TYPE == "cmake" ]]; then cat $BUILD_LOG_FILE \ | grep --count "R version passed into FindLibR.cmake: ${R_VERSION}" ) - if [[ $used_correct_r_version -ne 1 ]]; then + if [[ $passed_correct_r_version_to_cmake -ne 1 ]]; then echo "Unexpected R version was passed into cmake. Expected '${R_VERSION}'." exit 1 fi fi - -if grep -q -E "NOTE|WARNING|ERROR" "$LOG_FILE_NAME"; then - echo "NOTEs, WARNINGs, or ERRORs have been found by R CMD check" - exit 1 -fi - # this check makes sure that CI builds of the package actually use OpenMP if [[ $OS_NAME == "macos" ]] && [[ $R_BUILD_TYPE == "cran" ]]; then omp_working=$( diff --git a/.github/workflows/r_package.yml b/.github/workflows/r_package.yml index acf5f2407..fd4567767 100644 --- a/.github/workflows/r_package.yml +++ b/.github/workflows/r_package.yml @@ -18,7 +18,12 @@ env: # # this could be removed (hopefully) when R 3.6 support is removed ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true + # in CMake-driven builds, parallelize compilation CMAKE_BUILD_PARALLEL_LEVEL: 4 + # on Debian-based images, avoid interactive prompts + DEBIAN_FRONTEND: noninteractive + # parallelize compilation (extra important for Linux, where CRAN doesn't supply pre-compiled binaries) + MAKEFLAGS: "-j4" # hack to get around this: # https://stat.ethz.ch/pipermail/r-package-devel/2020q3/005930.html _R_CHECK_SYSTEM_CLOCK_: 0 @@ -258,58 +263,83 @@ jobs: RDscript${{ matrix.r_customization }} testthat.R >> tests.log 2>&1 || exit_code=-1 cat ./tests.log exit ${exit_code} - test-r-debian-clang: - name: r-package (debian, R-devel, clang-${{ matrix.clang-version }}) + test-r-extra-checks: + name: r-package (${{ matrix.image }}, R-devel) timeout-minutes: 60 strategy: fail-fast: false matrix: - # list of versions tested in CRAN "Additional Checks": - # https://cran.r-project.org/web/checks/check_issue_kinds.html - clang-version: - - 16 - - 17 + # references: + # * CRAN "additional checks": https://cran.r-project.org/web/checks/check_issue_kinds.html + # * images: https://r-hub.github.io/containers/containers.html + image: + - clang16 + - clang17 + - clang18 + - clang19 + - gcc14 + - intel runs-on: ubuntu-latest - container: rhub/debian-clang-devel - env: - DEBIAN_FRONTEND: noninteractive + container: ghcr.io/r-hub/containers/${{ matrix.image }}:latest steps: - - name: Install Git before checkout - shell: bash - run: | - apt-get update --allow-releaseinfo-change - apt-get install --no-install-recommends -y git - - name: Trust git cloning LightGBM - run: | - git config --global --add safe.directory "${GITHUB_WORKSPACE}" - name: Checkout repository uses: actions/checkout@v4 with: fetch-depth: 5 submodules: true - - name: install clang + - name: Install pandoc + uses: r-lib/actions/setup-pandoc@v2 + - name: Install LaTeX + shell: bash run: | - ./.ci/install-clang-devel.sh ${{ matrix.clang-version }} + if type -f apt 2>&1 > /dev/null; then + apt-get update + apt-get install --no-install-recommends -y \ + devscripts \ + texinfo \ + texlive-latex-extra \ + texlive-latex-recommended \ + texlive-fonts-recommended \ + texlive-fonts-extra \ + tidy \ + qpdf + else + yum update -y + yum install -y \ + devscripts \ + qpdf \ + texinfo \ + texinfo-tex \ + texlive-latex \ + tidy + fi - name: Install packages and run tests shell: bash run: | - export PATH=/opt/R-devel/bin/:${PATH} Rscript -e "install.packages(c('R6', 'data.table', 'jsonlite', 'knitr', 'markdown', 'Matrix', 'RhpcBLASctl', 'testthat'), repos = 'https://cran.rstudio.com', Ncpus = parallel::detectCores())" sh build-cran-package.sh - R CMD check --as-cran --run-donttest lightgbm_*.tar.gz || exit 1 - echo "" - echo "install logs:" - echo "" - cat lightgbm.Rcheck/00install.out - echo "" - if grep -q -E "NOTE|WARNING|ERROR" lightgbm.Rcheck/00check.log; then - echo "NOTEs, WARNINGs, or ERRORs have been found by R CMD check" - exit 1 + if [[ "${{ matrix.image }}" =~ "clang" ]]; then + # allowing the following NOTEs (produced by default in the clang images): + # + # * checking compilation flags used ... NOTE + # Compilation used the following non-portable flag(s): + # ‘-Wp,-D_FORTIFY_SOURCE=3’ + # + # even though CRAN itself sets that: + # https://www.stats.ox.ac.uk/pub/bdr/Rconfig/r-devel-linux-x86_64-fedora-clang + # + declare -i allowed_notes=1 + else + declare -i allowed_notes=0 fi + + bash .ci/run-r-cmd-check.sh \ + "$(echo lightgbm_$(head -1 VERSION.txt).tar.gz)" \ + "${allowed_notes}" all-r-package-jobs-successful: if: always() runs-on: ubuntu-latest - needs: [test, test-r-sanitizers, test-r-debian-clang] + needs: [test, test-r-sanitizers, test-r-extra-checks] steps: - name: Note that all tests succeeded uses: re-actors/alls-green@v1.2.2 diff --git a/R-package/src/lightgbm_R.cpp b/R-package/src/lightgbm_R.cpp index 045f0a9da..cc18ecfc3 100644 --- a/R-package/src/lightgbm_R.cpp +++ b/R-package/src/lightgbm_R.cpp @@ -13,8 +13,14 @@ #include #include +#ifndef R_NO_REMAP #define R_NO_REMAP +#endif + +#ifndef R_USE_C99_IN_CXX #define R_USE_C99_IN_CXX +#endif + #include #include diff --git a/R-package/src/lightgbm_R.h b/R-package/src/lightgbm_R.h index 9d100d095..1004640fd 100644 --- a/R-package/src/lightgbm_R.h +++ b/R-package/src/lightgbm_R.h @@ -7,8 +7,14 @@ #include +#ifndef R_NO_REMAP #define R_NO_REMAP +#endif + +#ifndef R_USE_C99_IN_CXX #define R_USE_C99_IN_CXX +#endif + #include /*! diff --git a/include/LightGBM/utils/log.h b/include/LightGBM/utils/log.h index 7cab31c15..45fd6fc26 100644 --- a/include/LightGBM/utils/log.h +++ b/include/LightGBM/utils/log.h @@ -16,8 +16,15 @@ #include #ifdef LGB_R_BUILD + +#ifndef R_NO_REMAP #define R_NO_REMAP +#endif + +#ifndef R_USE_C99_IN_CXX #define R_USE_C99_IN_CXX +#endif + #include extern "C" void R_FlushConsole(void); #endif