From 1e025db6dcd57909f225ed15b2e891cec129668a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Thu, 13 Feb 2025 10:28:54 +0100 Subject: [PATCH 1/7] GH-45521: [CI][Dev][R] Install required cyclocomp package to be used with R lintr --- .github/workflows/r.yml | 1 + ci/docker/linux-apt-lint.dockerfile | 1 + 2 files changed, 2 insertions(+) diff --git a/.github/workflows/r.yml b/.github/workflows/r.yml index a7d59c32d8afc..8bd9e95409d17 100644 --- a/.github/workflows/r.yml +++ b/.github/workflows/r.yml @@ -406,6 +406,7 @@ jobs: # we use pak for package installation since it is faster, safer and more convenient pak::local_install() pak::pak("lintr") + pak::pak("cyclocomp") lintr::expect_lint_free() - name: Dump install logs shell: cmd diff --git a/ci/docker/linux-apt-lint.dockerfile b/ci/docker/linux-apt-lint.dockerfile index 9ec80440a3c21..b73cc585ea74e 100644 --- a/ci/docker/linux-apt-lint.dockerfile +++ b/ci/docker/linux-apt-lint.dockerfile @@ -58,6 +58,7 @@ RUN cat /arrow/ci/etc/rprofile >> $(R RHOME)/etc/Rprofile.site RUN echo "MAKEFLAGS=-j$(R -s -e 'cat(parallel::detectCores())')" >> $(R RHOME)/etc/Renviron.site # We don't need arrow's dependencies, only lintr (and its dependencies) RUN R -e "install.packages('lintr')" +RUN R -e "install.packages('cyclocomp')" # Docker linter COPY --from=hadolint /bin/hadolint /usr/bin/hadolint From 06ec414d00d29e11db0d8e97ad34f6a00a8c0f5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Thu, 13 Feb 2025 11:26:15 +0100 Subject: [PATCH 2/7] Try to please return_linter --- r/R/arrow-info.R | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/r/R/arrow-info.R b/r/R/arrow-info.R index ddeb0f04efac1..81e2412d5fab6 100644 --- a/r/R/arrow-info.R +++ b/r/R/arrow-info.R @@ -84,7 +84,7 @@ arrow_available <- function() { #' @export arrow_with_acero <- function() { tryCatch(.Call(`_acero_available`), error = function(e) { - return(FALSE) + FALSE }) } @@ -92,7 +92,7 @@ arrow_with_acero <- function() { #' @export arrow_with_dataset <- function() { tryCatch(.Call(`_dataset_available`), error = function(e) { - return(FALSE) + FALSE }) } @@ -100,7 +100,7 @@ arrow_with_dataset <- function() { #' @export arrow_with_substrait <- function() { tryCatch(.Call(`_substrait_available`), error = function(e) { - return(FALSE) + FALSE }) } @@ -108,7 +108,7 @@ arrow_with_substrait <- function() { #' @export arrow_with_parquet <- function() { tryCatch(.Call(`_parquet_available`), error = function(e) { - return(FALSE) + FALSE }) } @@ -116,7 +116,7 @@ arrow_with_parquet <- function() { #' @export arrow_with_s3 <- function() { tryCatch(.Call(`_s3_available`), error = function(e) { - return(FALSE) + FALSE }) } @@ -124,7 +124,7 @@ arrow_with_s3 <- function() { #' @export arrow_with_gcs <- function() { tryCatch(.Call(`_gcs_available`), error = function(e) { - return(FALSE) + FALSE }) } @@ -132,7 +132,7 @@ arrow_with_gcs <- function() { #' @export arrow_with_json <- function() { tryCatch(.Call(`_json_available`), error = function(e) { - return(FALSE) + FALSE }) } From 8d853e28617b0dbad6a4f7fdbdb6661bbc89725a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Thu, 13 Feb 2025 12:19:03 +0100 Subject: [PATCH 3/7] Try to temporarily disable return_linter --- r/.lintr | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/r/.lintr b/r/.lintr index d920f77e9bbd3..78dfa94fe3b86 100644 --- a/r/.lintr +++ b/r/.lintr @@ -23,8 +23,10 @@ linters: linters_with_defaults( # object_name_linter = object_name_linter(styles = c("snake_case", "camelCase", "CamelCase", "symbols", "dotted.case", "UPPERCASE", "SNAKE_CASE")), object_length_linter = object_length_linter(40), object_usage_linter = NULL, # R6 methods are flagged, - cyclocomp_linter = cyclocomp_linter(26) # TODO: reduce to default of 15 + cyclocomp_linter = cyclocomp_linter(26), # TODO: reduce to default of 15 # See also https://github.com/r-lib/lintr/issues/804 for cyclocomp issues with R6 + # TODO: Fix newly added return_linter. See: https://github.com/apache/arrow/pull/45524#issuecomment-2656149531 + return_linter = NULL ) exclusions: list( "R/arrowExports.R", From de3d2cc0952e23ff6b4486ebb5ea01f21819f643 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Thu, 13 Feb 2025 12:19:05 +0100 Subject: [PATCH 4/7] Revert "Try to please return_linter" This reverts commit 06ec414d00d29e11db0d8e97ad34f6a00a8c0f5b. --- r/R/arrow-info.R | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/r/R/arrow-info.R b/r/R/arrow-info.R index 81e2412d5fab6..ddeb0f04efac1 100644 --- a/r/R/arrow-info.R +++ b/r/R/arrow-info.R @@ -84,7 +84,7 @@ arrow_available <- function() { #' @export arrow_with_acero <- function() { tryCatch(.Call(`_acero_available`), error = function(e) { - FALSE + return(FALSE) }) } @@ -92,7 +92,7 @@ arrow_with_acero <- function() { #' @export arrow_with_dataset <- function() { tryCatch(.Call(`_dataset_available`), error = function(e) { - FALSE + return(FALSE) }) } @@ -100,7 +100,7 @@ arrow_with_dataset <- function() { #' @export arrow_with_substrait <- function() { tryCatch(.Call(`_substrait_available`), error = function(e) { - FALSE + return(FALSE) }) } @@ -108,7 +108,7 @@ arrow_with_substrait <- function() { #' @export arrow_with_parquet <- function() { tryCatch(.Call(`_parquet_available`), error = function(e) { - FALSE + return(FALSE) }) } @@ -116,7 +116,7 @@ arrow_with_parquet <- function() { #' @export arrow_with_s3 <- function() { tryCatch(.Call(`_s3_available`), error = function(e) { - FALSE + return(FALSE) }) } @@ -124,7 +124,7 @@ arrow_with_s3 <- function() { #' @export arrow_with_gcs <- function() { tryCatch(.Call(`_gcs_available`), error = function(e) { - FALSE + return(FALSE) }) } @@ -132,7 +132,7 @@ arrow_with_gcs <- function() { #' @export arrow_with_json <- function() { tryCatch(.Call(`_json_available`), error = function(e) { - FALSE + return(FALSE) }) } From feb940f8c71ef3f7dba86953abc58fa145fb788c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Thu, 13 Feb 2025 13:24:13 +0100 Subject: [PATCH 5/7] Try pinning lintr --- .github/workflows/r.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/r.yml b/.github/workflows/r.yml index 8bd9e95409d17..a3769dc66de8f 100644 --- a/.github/workflows/r.yml +++ b/.github/workflows/r.yml @@ -405,7 +405,7 @@ jobs: ) # we use pak for package installation since it is faster, safer and more convenient pak::local_install() - pak::pak("lintr") + pak::pak("lintr@3.1.2") pak::pak("cyclocomp") lintr::expect_lint_free() - name: Dump install logs From bfc6b12e8583e8b61fc1a290716b87d97a33f2fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Thu, 13 Feb 2025 14:33:42 +0100 Subject: [PATCH 6/7] Use older lintr version --- .github/workflows/r.yml | 1 - ci/docker/linux-apt-lint.dockerfile | 3 +-- r/.lintr | 4 +--- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/.github/workflows/r.yml b/.github/workflows/r.yml index a3769dc66de8f..5c373d5615844 100644 --- a/.github/workflows/r.yml +++ b/.github/workflows/r.yml @@ -406,7 +406,6 @@ jobs: # we use pak for package installation since it is faster, safer and more convenient pak::local_install() pak::pak("lintr@3.1.2") - pak::pak("cyclocomp") lintr::expect_lint_free() - name: Dump install logs shell: cmd diff --git a/ci/docker/linux-apt-lint.dockerfile b/ci/docker/linux-apt-lint.dockerfile index b73cc585ea74e..38f94546fd004 100644 --- a/ci/docker/linux-apt-lint.dockerfile +++ b/ci/docker/linux-apt-lint.dockerfile @@ -57,8 +57,7 @@ RUN cat /arrow/ci/etc/rprofile >> $(R RHOME)/etc/Rprofile.site # Also ensure parallel compilation of C/C++ code RUN echo "MAKEFLAGS=-j$(R -s -e 'cat(parallel::detectCores())')" >> $(R RHOME)/etc/Renviron.site # We don't need arrow's dependencies, only lintr (and its dependencies) -RUN R -e "install.packages('lintr')" -RUN R -e "install.packages('cyclocomp')" +RUN R -e "install.packages('lintr@3.1.2')" # Docker linter COPY --from=hadolint /bin/hadolint /usr/bin/hadolint diff --git a/r/.lintr b/r/.lintr index 78dfa94fe3b86..d920f77e9bbd3 100644 --- a/r/.lintr +++ b/r/.lintr @@ -23,10 +23,8 @@ linters: linters_with_defaults( # object_name_linter = object_name_linter(styles = c("snake_case", "camelCase", "CamelCase", "symbols", "dotted.case", "UPPERCASE", "SNAKE_CASE")), object_length_linter = object_length_linter(40), object_usage_linter = NULL, # R6 methods are flagged, - cyclocomp_linter = cyclocomp_linter(26), # TODO: reduce to default of 15 + cyclocomp_linter = cyclocomp_linter(26) # TODO: reduce to default of 15 # See also https://github.com/r-lib/lintr/issues/804 for cyclocomp issues with R6 - # TODO: Fix newly added return_linter. See: https://github.com/apache/arrow/pull/45524#issuecomment-2656149531 - return_linter = NULL ) exclusions: list( "R/arrowExports.R", From 046ae1d3fb898b6dd99b6b885341276c4451b27b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Thu, 13 Feb 2025 14:46:47 +0100 Subject: [PATCH 7/7] Add comment and revert back lintr version --- .github/workflows/r.yml | 2 ++ ci/docker/linux-apt-lint.dockerfile | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/r.yml b/.github/workflows/r.yml index 5c373d5615844..4bcac220535a9 100644 --- a/.github/workflows/r.yml +++ b/.github/workflows/r.yml @@ -405,6 +405,8 @@ jobs: ) # we use pak for package installation since it is faster, safer and more convenient pak::local_install() + # Pin the lintr version to avoid breaking changes with newer 3.2.0 version + # See: https://github.com/apache/arrow/pull/45524 pak::pak("lintr@3.1.2") lintr::expect_lint_free() - name: Dump install logs diff --git a/ci/docker/linux-apt-lint.dockerfile b/ci/docker/linux-apt-lint.dockerfile index 38f94546fd004..b73cc585ea74e 100644 --- a/ci/docker/linux-apt-lint.dockerfile +++ b/ci/docker/linux-apt-lint.dockerfile @@ -57,7 +57,8 @@ RUN cat /arrow/ci/etc/rprofile >> $(R RHOME)/etc/Rprofile.site # Also ensure parallel compilation of C/C++ code RUN echo "MAKEFLAGS=-j$(R -s -e 'cat(parallel::detectCores())')" >> $(R RHOME)/etc/Renviron.site # We don't need arrow's dependencies, only lintr (and its dependencies) -RUN R -e "install.packages('lintr@3.1.2')" +RUN R -e "install.packages('lintr')" +RUN R -e "install.packages('cyclocomp')" # Docker linter COPY --from=hadolint /bin/hadolint /usr/bin/hadolint