Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ci] [R-package] add CI jobs covering more CRAN "additional checks", fix R_NO_REMAP warnings (fixes #6369) #6523

Merged
merged 22 commits into from
Jul 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 0 additions & 74 deletions .ci/install-clang-devel.sh

This file was deleted.

46 changes: 46 additions & 0 deletions .ci/run-r-cmd-check.sh
Original file line number Diff line number Diff line change
@@ -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
31 changes: 7 additions & 24 deletions .ci/test_r_package.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a fix found in #6501. That PR was abandoned accidentally and I'd still like to give @Kunal-Singh-Dadhwal an opportunity to submit that work again.

But I want this particular fix, to get more confidence that the R CI jobs are working as expected.

context: #6501 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jameslamb Thanks for the opportunity will make a commit really soon

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=$(
Expand Down
92 changes: 61 additions & 31 deletions .github/workflows/r_package.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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/[email protected]
Expand Down
6 changes: 6 additions & 0 deletions R-package/src/lightgbm_R.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,14 @@
#include <R_ext/Rdynload.h>
#include <R_ext/Altrep.h>

#ifndef R_NO_REMAP
#define R_NO_REMAP
#endif

#ifndef R_USE_C99_IN_CXX
#define R_USE_C99_IN_CXX
#endif

#include <R_ext/Error.h>

#include <string>
Expand Down
6 changes: 6 additions & 0 deletions R-package/src/lightgbm_R.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,14 @@

#include <LightGBM/c_api.h>

#ifndef R_NO_REMAP
#define R_NO_REMAP
#endif

#ifndef R_USE_C99_IN_CXX
#define R_USE_C99_IN_CXX
#endif

#include <Rinternals.h>

/*!
Expand Down
7 changes: 7 additions & 0 deletions include/LightGBM/utils/log.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,15 @@
#include <string>

#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 <R_ext/Print.h>
extern "C" void R_FlushConsole(void);
#endif
Expand Down
Loading