You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/04/05 18:39:13 UTC
[GitHub] [arrow] nealrichardson opened a new pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
nealrichardson opened a new pull request #9896:
URL: https://github.com/apache/arrow/pull/9896
Also adds an .onAttach message for the arrow-without-arrow case to hopefully alert users earlier that they have an incomplete/useless build (requires #9689 or similar to restore the without-arrow build)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#issuecomment-814499902
Revision: 773331ad156f6eae817414db53cf77eb11c204fe
Submitted crossbow builds: [ursacomputing/crossbow @ actions-269](https://github.com/ursacomputing/crossbow/branches/all?query=actions-269)
|Task|Status|
|----|------|
|test-r-without-arrow|[](https://dev.azure.com/ursacomputing/crossbow/_build/latest?definitionId=1&branchName=actions-269-azure-test-r-without-arrow)|
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#issuecomment-815139719
@github-actions crossbow submit test-r-without-arrow test-r-rstudio-r-base-3.6-bionic
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#issuecomment-816812751
Revision: a21ed61b7d58604bf42fb48cb03362fc565364bb
Submitted crossbow builds: [ursacomputing/crossbow @ actions-303](https://github.com/ursacomputing/crossbow/branches/all?query=actions-303)
|Task|Status|
|----|------|
|test-r-without-arrow|[](https://dev.azure.com/ursacomputing/crossbow/_build/latest?definitionId=1&branchName=actions-303-azure-test-r-without-arrow)|
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#issuecomment-813634151
@github-actions crossbow submit test-r-without-arrow
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on a change in pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#discussion_r607351892
##########
File path: r/R/arrow-package.R
##########
@@ -47,12 +47,27 @@
}
# Create these once, at package build time
- dplyr_functions$dataset <- build_function_list(build_dataset_expression)
- dplyr_functions$array <- build_function_list(build_array_expression)
-
+ if (arrow_available()) {
+ dplyr_functions$dataset <- build_function_list(build_dataset_expression)
+ dplyr_functions$array <- build_function_list(build_array_expression)
+ }
invisible()
}
+.onAttach <- function(libname, pkgname) {
+ if (!arrow_available()) {
+ msg <- paste(
+ 'The Arrow C++ library is not available. To retry installation with debug output, run:',
+
+ ' install_arrow(verbose = TRUE)',
+
+ 'See `vignette("install", package = "arrow")` for more guidance and troubleshooting.',
Review comment:
> I'm not sure. It's different from "dependencies" that R users are used to where if it's missing, you install it and now things work. And likewise with reticulate packages, blogdown, etc., where you have this external thing you have to install after you install the R package (install_tensorflow(), install_hugo(), etc.), this is different because you have to re-install the R package.
Fair enough, `C++ library` is probably sufficient here
> Right. We want to convey the message that the fix for this is to reinstall the R package. But really, if they're here, what we probably want is for them to reinstall with verbosity and then file a JIRA with that output (should we add a report_installation_issue() function to do that? ;). Users shouldn't be here normally.
`report_installation_issue()` would be slick, especially if it could turn on `ARROW_R_DEV` and copy the input/output automatically.
> We could, but if you look at arrow_info(), there are lots of features you might not have, and I don't know that we want to message about all of them.
>
> What if we checked arrow_info()$capabilities, possibly excluded some we don't think are interesting (for example, I don't have lzo, whatever that is, and maybe it's not interesting whether you have all of the memory allocators built), and if any of the rest are false, emit a message like "Some Arrow features are not enabled in this build; see arrow_info() for details and ?arrow_info (or link to vignette) for how to install them"?
Yeah, honestly I think the only two that are important are snappy and S3 support. Those are the only two that I've run into or seen issues with on Jira. What if we only alert on those two and leave the other ones be for now?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on a change in pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#discussion_r607335099
##########
File path: r/R/arrow-package.R
##########
@@ -47,12 +47,27 @@
}
# Create these once, at package build time
- dplyr_functions$dataset <- build_function_list(build_dataset_expression)
- dplyr_functions$array <- build_function_list(build_array_expression)
-
+ if (arrow_available()) {
+ dplyr_functions$dataset <- build_function_list(build_dataset_expression)
+ dplyr_functions$array <- build_function_list(build_array_expression)
+ }
invisible()
}
+.onAttach <- function(libname, pkgname) {
+ if (!arrow_available()) {
+ msg <- paste(
+ 'The Arrow C++ library is not available. To retry installation with debug output, run:',
+
+ ' install_arrow(verbose = TRUE)',
+
+ 'See `vignette("install", package = "arrow")` for more guidance and troubleshooting.',
Review comment:
I think this is a good general message. A few thoughts:
* Should we refer to the arrow c++ as a dependency? I think that's a word that people with less experience might look for.
* Should we mention that in typical installations the arrow c++ library is either bundled with the installation (e.g. our binaries/CRAN) or built as part of the package installation process. I can't think of a short and pithy way to say this, but what I think we _don't_ want is someone to think that this means they need to now go and `apt install arrow` or the like (that should work for releases as long as the versions match up, but there are lots of ways that could be wrong).
* Is there any prohibition on offering the url https://arrow.apache.org/docs/r/articles/install.html it's the same content as the vignette, but I suspect more people are familiar with packagdown sites than they are with vignettes these days.
This is separate from this message, but I wonder if we should have another message onload (ideally onload, only the first time like many of the tidyverse packages do) that tests for `codec_is_available("snappy")` (or all of the ones not in the minimal build) that gently nudges that in the case of linux you'll want to set libarrow_binary=true or libarrow_minimal=false to get a more fully featured build? I know that a few times when I was managing rstudio server on linux I installed arrow, thought everything was fine and then later someone had to bug me that snappy wasn't enabled anymore so I had to go back and install it. It's not a huge deal but getting it out of the way when I'm in the installing mindset would have been nice compared to finding out later and having to go back to it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#issuecomment-815363951
The without-cran build still needs some work, and unfortunately we need to go back to wrapping every function individually because without it it fails (well, not fail per se, but CRAN would reject this if it saw it) like this:
```
* checking R code for possible problems ... NOTE
all_record_batches: no visible binding for global variable
‘_arrow_all_record_batches’
allocate_arrow_array: no visible binding for global variable
‘_arrow_allocate_arrow_array’
allocate_arrow_schema: no visible binding for global variable
‘_arrow_allocate_arrow_schema’
...
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on a change in pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#discussion_r607269004
##########
File path: r/R/install-arrow.R
##########
@@ -62,55 +62,50 @@ install_arrow <- function(nightly = FALSE,
sysname <- tolower(Sys.info()[["sysname"]])
conda <- isTRUE(grepl("conda", R.Version()$platform))
- if (sysname %in% c("windows", "darwin", "linux")) {
- if (conda) {
- if (nightly) {
- system("conda install -y -c arrow-nightlies -c conda-forge --strict-channel-priority r-arrow")
- } else {
- system("conda install -y -c conda-forge --strict-channel-priority r-arrow")
- }
+ if (conda) {
+ if (nightly) {
+ system("conda install -y -c arrow-nightlies -c conda-forge --strict-channel-priority r-arrow")
} else {
- Sys.setenv(
- LIBARROW_DOWNLOAD = "true",
- LIBARROW_BINARY = binary,
- LIBARROW_MINIMAL = minimal,
- ARROW_R_DEV = verbose,
- ARROW_USE_PKG_CONFIG = use_system
- )
- # On the M1, we can't use the usual autobrew, which pulls Intel dependencies
- apple_m1 <- grepl("arm-apple|aarch64.*darwin", R.Version()$platform)
- # On Rosetta, we have to build without JEMALLOC, so we also can't autobrew
- rosetta <- identical(sysname, "darwin") && identical(system("sysctl -n sysctl.proc_translated", intern = TRUE), "1")
- if (rosetta) {
- Sys.setenv(ARROW_JEMALLOC = "OFF")
- }
- if (apple_m1 || rosetta) {
- Sys.setenv(FORCE_BUNDLED_BUILD = "true")
- }
+ system("conda install -y -c conda-forge --strict-channel-priority r-arrow")
+ }
+ } else {
+ Sys.setenv(
+ LIBARROW_DOWNLOAD = "true",
+ LIBARROW_BINARY = binary,
+ LIBARROW_MINIMAL = minimal,
+ ARROW_R_DEV = verbose,
+ ARROW_USE_PKG_CONFIG = use_system
+ )
+ # On the M1, we can't use the usual autobrew, which pulls Intel dependencies
+ apple_m1 <- grepl("arm-apple|aarch64.*darwin", R.Version()$platform)
+ # On Rosetta, we have to build without JEMALLOC, so we also can't autobrew
+ rosetta <- identical(sysname, "darwin") && identical(system("sysctl -n sysctl.proc_translated", intern = TRUE), "1")
+ if (rosetta) {
+ Sys.setenv(ARROW_JEMALLOC = "OFF")
+ }
+ if (apple_m1 || rosetta) {
+ Sys.setenv(FORCE_BUNDLED_BUILD = "true")
+ }
- opts <- list()
- if (apple_m1 || rosetta) {
- # Skip binaries (esp. for rosetta)
- opts$pkgType <- "source"
- } else if (isTRUE(binary)) {
- # Unless otherwise directed, don't consider newer source packages when
- # options(pkgType) == "both" (default on win/mac)
- opts$install.packages.check.source <- "no"
- opts$install.packages.compile.from.source <- "never"
- }
- if (length(opts)) {
- old <- options(opts)
- on.exit(options(old))
- }
- install.packages("arrow", repos = arrow_repos(repos, nightly), ...)
+ opts <- list()
+ if (apple_m1 || rosetta) {
+ # Skip binaries (esp. for rosetta)
+ opts$pkgType <- "source"
+ } else if (isTRUE(binary)) {
+ # Unless otherwise directed, don't consider newer source packages when
+ # options(pkgType) == "both" (default on win/mac)
+ opts$install.packages.check.source <- "no"
+ opts$install.packages.compile.from.source <- "never"
}
- if ("arrow" %in% loadedNamespaces()) {
- # If you've just sourced this file, "arrow" won't be (re)loaded
- reload_arrow()
+ if (length(opts)) {
+ old <- options(opts)
+ on.exit(options(old))
Review comment:
Sure 🤷
FTR all I did here is remove the `if` indentation, no substantive change to this code
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#issuecomment-814375992
@github-actions crossbow submit test-r-without-arrow
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on a change in pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#discussion_r607269799
##########
File path: r/R/install-arrow.R
##########
@@ -62,55 +62,50 @@ install_arrow <- function(nightly = FALSE,
sysname <- tolower(Sys.info()[["sysname"]])
conda <- isTRUE(grepl("conda", R.Version()$platform))
- if (sysname %in% c("windows", "darwin", "linux")) {
- if (conda) {
- if (nightly) {
- system("conda install -y -c arrow-nightlies -c conda-forge --strict-channel-priority r-arrow")
- } else {
- system("conda install -y -c conda-forge --strict-channel-priority r-arrow")
- }
+ if (conda) {
+ if (nightly) {
+ system("conda install -y -c arrow-nightlies -c conda-forge --strict-channel-priority r-arrow")
} else {
- Sys.setenv(
- LIBARROW_DOWNLOAD = "true",
- LIBARROW_BINARY = binary,
- LIBARROW_MINIMAL = minimal,
- ARROW_R_DEV = verbose,
- ARROW_USE_PKG_CONFIG = use_system
- )
- # On the M1, we can't use the usual autobrew, which pulls Intel dependencies
- apple_m1 <- grepl("arm-apple|aarch64.*darwin", R.Version()$platform)
- # On Rosetta, we have to build without JEMALLOC, so we also can't autobrew
- rosetta <- identical(sysname, "darwin") && identical(system("sysctl -n sysctl.proc_translated", intern = TRUE), "1")
- if (rosetta) {
- Sys.setenv(ARROW_JEMALLOC = "OFF")
- }
- if (apple_m1 || rosetta) {
- Sys.setenv(FORCE_BUNDLED_BUILD = "true")
- }
+ system("conda install -y -c conda-forge --strict-channel-priority r-arrow")
+ }
+ } else {
+ Sys.setenv(
+ LIBARROW_DOWNLOAD = "true",
+ LIBARROW_BINARY = binary,
+ LIBARROW_MINIMAL = minimal,
+ ARROW_R_DEV = verbose,
+ ARROW_USE_PKG_CONFIG = use_system
+ )
+ # On the M1, we can't use the usual autobrew, which pulls Intel dependencies
+ apple_m1 <- grepl("arm-apple|aarch64.*darwin", R.Version()$platform)
+ # On Rosetta, we have to build without JEMALLOC, so we also can't autobrew
+ rosetta <- identical(sysname, "darwin") && identical(system("sysctl -n sysctl.proc_translated", intern = TRUE), "1")
+ if (rosetta) {
+ Sys.setenv(ARROW_JEMALLOC = "OFF")
+ }
+ if (apple_m1 || rosetta) {
+ Sys.setenv(FORCE_BUNDLED_BUILD = "true")
+ }
- opts <- list()
- if (apple_m1 || rosetta) {
- # Skip binaries (esp. for rosetta)
- opts$pkgType <- "source"
- } else if (isTRUE(binary)) {
- # Unless otherwise directed, don't consider newer source packages when
- # options(pkgType) == "both" (default on win/mac)
- opts$install.packages.check.source <- "no"
- opts$install.packages.compile.from.source <- "never"
- }
- if (length(opts)) {
- old <- options(opts)
- on.exit(options(old))
- }
- install.packages("arrow", repos = arrow_repos(repos, nightly), ...)
+ opts <- list()
+ if (apple_m1 || rosetta) {
+ # Skip binaries (esp. for rosetta)
+ opts$pkgType <- "source"
+ } else if (isTRUE(binary)) {
+ # Unless otherwise directed, don't consider newer source packages when
+ # options(pkgType) == "both" (default on win/mac)
+ opts$install.packages.check.source <- "no"
+ opts$install.packages.compile.from.source <- "never"
}
- if ("arrow" %in% loadedNamespaces()) {
- # If you've just sourced this file, "arrow" won't be (re)loaded
- reload_arrow()
+ if (length(opts)) {
+ old <- options(opts)
+ on.exit(options(old))
Review comment:
I don't think we should just for this + right now, but reading it and making sure what was intended took a bit. Maybe next time we reach for a similar pattern
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#issuecomment-815402560
Revision: 7aede6acf31886b5b05bfe5927a3fee95ad03393
Submitted crossbow builds: [ursacomputing/crossbow @ actions-283](https://github.com/ursacomputing/crossbow/branches/all?query=actions-283)
|Task|Status|
|----|------|
|test-r-without-arrow|[](https://dev.azure.com/ursacomputing/crossbow/_build/latest?definitionId=1&branchName=actions-283-azure-test-r-without-arrow)|
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on a change in pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#discussion_r607351892
##########
File path: r/R/arrow-package.R
##########
@@ -47,12 +47,27 @@
}
# Create these once, at package build time
- dplyr_functions$dataset <- build_function_list(build_dataset_expression)
- dplyr_functions$array <- build_function_list(build_array_expression)
-
+ if (arrow_available()) {
+ dplyr_functions$dataset <- build_function_list(build_dataset_expression)
+ dplyr_functions$array <- build_function_list(build_array_expression)
+ }
invisible()
}
+.onAttach <- function(libname, pkgname) {
+ if (!arrow_available()) {
+ msg <- paste(
+ 'The Arrow C++ library is not available. To retry installation with debug output, run:',
+
+ ' install_arrow(verbose = TRUE)',
+
+ 'See `vignette("install", package = "arrow")` for more guidance and troubleshooting.',
Review comment:
> I'm not sure. It's different from "dependencies" that R users are used to where if it's missing, you install it and now things work. And likewise with reticulate packages, blogdown, etc., where you have this external thing you have to install after you install the R package (install_tensorflow(), install_hugo(), etc.), this is different because you have to re-install the R package.
Fair enough, `C++ library` is probably sufficient here
> Right. We want to convey the message that the fix for this is to reinstall the R package. But really, if they're here, what we probably want is for them to reinstall with verbosity and then file a JIRA with that output (should we add a report_installation_issue() function to do that? ;). Users shouldn't be here normally.
`report_installation_issue()` would be slick, especially if it could turn on `ARROW_R_DEV` and copy the input/output automatically.
> We could, but if you look at arrow_info(), there are lots of features you might not have, and I don't know that we want to message about all of them.
>
> What if we checked arrow_info()$capabilities, possibly excluded some we don't think are interesting (for example, I don't have lzo, whatever that is, and maybe it's not interesting whether you have all of the memory allocators built), and if any of the rest are false, emit a message like "Some Arrow features are not enabled in this build; see arrow_info() for details and ?arrow_info (or link to vignette) for how to install them"?
Yeah, honestly I think the only two that are important are snappy and S3 support. Those are the only two that I've run into or seen issues with on Jira. Snappy because it's the default compression when available, and S3 cause everything in the world is stored on S3. What if we only alert on those two and leave the other ones be for now?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#issuecomment-814318416
Revision: c4e4c6bc1ea0f1b39d7fc0015317190cb6ca0e8a
Submitted crossbow builds: [ursacomputing/crossbow @ actions-267](https://github.com/ursacomputing/crossbow/branches/all?query=actions-267)
|Task|Status|
|----|------|
|test-r-rstudio-r-base-3.6-bionic|[](https://dev.azure.com/ursacomputing/crossbow/_build/latest?definitionId=1&branchName=actions-267-azure-test-r-rstudio-r-base-3.6-bionic)|
|test-r-without-arrow|[](https://dev.azure.com/ursacomputing/crossbow/_build/latest?definitionId=1&branchName=actions-267-azure-test-r-without-arrow)|
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson closed pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
nealrichardson closed pull request #9896:
URL: https://github.com/apache/arrow/pull/9896
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on a change in pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#discussion_r607265506
##########
File path: r/R/install-arrow.R
##########
@@ -62,55 +62,50 @@ install_arrow <- function(nightly = FALSE,
sysname <- tolower(Sys.info()[["sysname"]])
conda <- isTRUE(grepl("conda", R.Version()$platform))
- if (sysname %in% c("windows", "darwin", "linux")) {
- if (conda) {
- if (nightly) {
- system("conda install -y -c arrow-nightlies -c conda-forge --strict-channel-priority r-arrow")
- } else {
- system("conda install -y -c conda-forge --strict-channel-priority r-arrow")
- }
+ if (conda) {
+ if (nightly) {
+ system("conda install -y -c arrow-nightlies -c conda-forge --strict-channel-priority r-arrow")
} else {
- Sys.setenv(
- LIBARROW_DOWNLOAD = "true",
- LIBARROW_BINARY = binary,
- LIBARROW_MINIMAL = minimal,
- ARROW_R_DEV = verbose,
- ARROW_USE_PKG_CONFIG = use_system
- )
- # On the M1, we can't use the usual autobrew, which pulls Intel dependencies
- apple_m1 <- grepl("arm-apple|aarch64.*darwin", R.Version()$platform)
- # On Rosetta, we have to build without JEMALLOC, so we also can't autobrew
- rosetta <- identical(sysname, "darwin") && identical(system("sysctl -n sysctl.proc_translated", intern = TRUE), "1")
- if (rosetta) {
- Sys.setenv(ARROW_JEMALLOC = "OFF")
- }
- if (apple_m1 || rosetta) {
- Sys.setenv(FORCE_BUNDLED_BUILD = "true")
- }
+ system("conda install -y -c conda-forge --strict-channel-priority r-arrow")
+ }
+ } else {
+ Sys.setenv(
+ LIBARROW_DOWNLOAD = "true",
+ LIBARROW_BINARY = binary,
+ LIBARROW_MINIMAL = minimal,
+ ARROW_R_DEV = verbose,
+ ARROW_USE_PKG_CONFIG = use_system
+ )
+ # On the M1, we can't use the usual autobrew, which pulls Intel dependencies
+ apple_m1 <- grepl("arm-apple|aarch64.*darwin", R.Version()$platform)
+ # On Rosetta, we have to build without JEMALLOC, so we also can't autobrew
+ rosetta <- identical(sysname, "darwin") && identical(system("sysctl -n sysctl.proc_translated", intern = TRUE), "1")
+ if (rosetta) {
+ Sys.setenv(ARROW_JEMALLOC = "OFF")
+ }
+ if (apple_m1 || rosetta) {
+ Sys.setenv(FORCE_BUNDLED_BUILD = "true")
+ }
- opts <- list()
- if (apple_m1 || rosetta) {
- # Skip binaries (esp. for rosetta)
- opts$pkgType <- "source"
- } else if (isTRUE(binary)) {
- # Unless otherwise directed, don't consider newer source packages when
- # options(pkgType) == "both" (default on win/mac)
- opts$install.packages.check.source <- "no"
- opts$install.packages.compile.from.source <- "never"
- }
- if (length(opts)) {
- old <- options(opts)
- on.exit(options(old))
- }
- install.packages("arrow", repos = arrow_repos(repos, nightly), ...)
+ opts <- list()
+ if (apple_m1 || rosetta) {
+ # Skip binaries (esp. for rosetta)
+ opts$pkgType <- "source"
+ } else if (isTRUE(binary)) {
+ # Unless otherwise directed, don't consider newer source packages when
+ # options(pkgType) == "both" (default on win/mac)
+ opts$install.packages.check.source <- "no"
+ opts$install.packages.compile.from.source <- "never"
}
- if ("arrow" %in% loadedNamespaces()) {
- # If you've just sourced this file, "arrow" won't be (re)loaded
- reload_arrow()
+ if (length(opts)) {
+ old <- options(opts)
+ on.exit(options(old))
Review comment:
This isn't enough to justify it, and I don't think that it's a recursive dependency of anything we currently depend on, but this makes me long for `withr::with_options()`
##########
File path: r/tools/nixlibs.R
##########
@@ -333,10 +333,10 @@ build_libarrow <- function(src_dir, dst_dir) {
env_vars <- paste(env_vars, "ARROW_JEMALLOC=OFF ARROW_PARQUET=OFF ARROW_DATASET=OFF ARROW_WITH_RE2=OFF ARROW_WITH_UTF8PROC=OFF EXTRA_CMAKE_FLAGS=-DARROW_SIMD_LEVEL=NONE")
}
cat("**** arrow", ifelse(quietly, "", paste("with", env_vars)), "\n")
- status <- system(
+ status <- suppressWarnings(system(
Review comment:
`suppressWarnings()` makes sense here, though I'm curious what warnings you've seen here in the past?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on a change in pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#discussion_r607306343
##########
File path: r/R/install-arrow.R
##########
@@ -62,55 +62,50 @@ install_arrow <- function(nightly = FALSE,
sysname <- tolower(Sys.info()[["sysname"]])
conda <- isTRUE(grepl("conda", R.Version()$platform))
- if (sysname %in% c("windows", "darwin", "linux")) {
- if (conda) {
- if (nightly) {
- system("conda install -y -c arrow-nightlies -c conda-forge --strict-channel-priority r-arrow")
- } else {
- system("conda install -y -c conda-forge --strict-channel-priority r-arrow")
- }
+ if (conda) {
+ if (nightly) {
+ system("conda install -y -c arrow-nightlies -c conda-forge --strict-channel-priority r-arrow")
} else {
- Sys.setenv(
- LIBARROW_DOWNLOAD = "true",
- LIBARROW_BINARY = binary,
- LIBARROW_MINIMAL = minimal,
- ARROW_R_DEV = verbose,
- ARROW_USE_PKG_CONFIG = use_system
- )
- # On the M1, we can't use the usual autobrew, which pulls Intel dependencies
- apple_m1 <- grepl("arm-apple|aarch64.*darwin", R.Version()$platform)
- # On Rosetta, we have to build without JEMALLOC, so we also can't autobrew
- rosetta <- identical(sysname, "darwin") && identical(system("sysctl -n sysctl.proc_translated", intern = TRUE), "1")
- if (rosetta) {
- Sys.setenv(ARROW_JEMALLOC = "OFF")
- }
- if (apple_m1 || rosetta) {
- Sys.setenv(FORCE_BUNDLED_BUILD = "true")
- }
+ system("conda install -y -c conda-forge --strict-channel-priority r-arrow")
+ }
+ } else {
+ Sys.setenv(
+ LIBARROW_DOWNLOAD = "true",
+ LIBARROW_BINARY = binary,
+ LIBARROW_MINIMAL = minimal,
+ ARROW_R_DEV = verbose,
+ ARROW_USE_PKG_CONFIG = use_system
+ )
+ # On the M1, we can't use the usual autobrew, which pulls Intel dependencies
+ apple_m1 <- grepl("arm-apple|aarch64.*darwin", R.Version()$platform)
+ # On Rosetta, we have to build without JEMALLOC, so we also can't autobrew
+ rosetta <- identical(sysname, "darwin") && identical(system("sysctl -n sysctl.proc_translated", intern = TRUE), "1")
+ if (rosetta) {
+ Sys.setenv(ARROW_JEMALLOC = "OFF")
+ }
+ if (apple_m1 || rosetta) {
+ Sys.setenv(FORCE_BUNDLED_BUILD = "true")
+ }
- opts <- list()
- if (apple_m1 || rosetta) {
- # Skip binaries (esp. for rosetta)
- opts$pkgType <- "source"
- } else if (isTRUE(binary)) {
- # Unless otherwise directed, don't consider newer source packages when
- # options(pkgType) == "both" (default on win/mac)
- opts$install.packages.check.source <- "no"
- opts$install.packages.compile.from.source <- "never"
- }
- if (length(opts)) {
- old <- options(opts)
- on.exit(options(old))
- }
- install.packages("arrow", repos = arrow_repos(repos, nightly), ...)
+ opts <- list()
+ if (apple_m1 || rosetta) {
+ # Skip binaries (esp. for rosetta)
+ opts$pkgType <- "source"
+ } else if (isTRUE(binary)) {
+ # Unless otherwise directed, don't consider newer source packages when
+ # options(pkgType) == "both" (default on win/mac)
+ opts$install.packages.check.source <- "no"
+ opts$install.packages.compile.from.source <- "never"
}
- if ("arrow" %in% loadedNamespaces()) {
- # If you've just sourced this file, "arrow" won't be (re)loaded
- reload_arrow()
+ if (length(opts)) {
+ old <- options(opts)
+ on.exit(options(old))
Review comment:
We Suggest packages that require withr but don't Import any. We could, in the meantime, add a `with_options()` to util.R, though FWIW this is the only place we'd be using it right now in the package.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on a change in pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#discussion_r608105023
##########
File path: r/R/arrow-package.R
##########
@@ -200,6 +195,9 @@ print.arrow_info <- function(x, ...) {
jemalloc = "jemalloc" %in% x$memory_pool$available_backends,
mimalloc = "mimalloc" %in% x$memory_pool$available_backends
))
+ if (some_features_are_off(x$capabilities)) {
+ cat("To reinstall with more features enabled, see\n https://arrow.apache.org/docs/r/articles/install.html\n\n")
Review comment:
```suggestion
cat("If you need any of the capabilities marked FALSE, you can reinstall with more features enabled, see\n https://arrow.apache.org/docs/r/articles/install.html\n\n")
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#issuecomment-815396109
@github-actions crossbow submit test-r-without-arrow
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#issuecomment-816112221
I believe the last remaining issue here is that we are doing `R CMD check --as-cran --run-donttest` and the `\donttest` examples don't pass without arrow C++. I could either (1) hack the test script to be able to turn off `--run-donttest` for *this* job, or (2) flip all of the examples to `\dontrun`. Opinions @ianmcook @jonkeane ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
jonkeane commented on pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#issuecomment-815315108
LGTM!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on a change in pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#discussion_r607341142
##########
File path: r/R/arrow-package.R
##########
@@ -47,12 +47,27 @@
}
# Create these once, at package build time
- dplyr_functions$dataset <- build_function_list(build_dataset_expression)
- dplyr_functions$array <- build_function_list(build_array_expression)
-
+ if (arrow_available()) {
+ dplyr_functions$dataset <- build_function_list(build_dataset_expression)
+ dplyr_functions$array <- build_function_list(build_array_expression)
+ }
invisible()
}
+.onAttach <- function(libname, pkgname) {
+ if (!arrow_available()) {
+ msg <- paste(
+ 'The Arrow C++ library is not available. To retry installation with debug output, run:',
+
+ ' install_arrow(verbose = TRUE)',
+
+ 'See `vignette("install", package = "arrow")` for more guidance and troubleshooting.',
Review comment:
> * Should we refer to the arrow c++ as a dependency? I think that's a word that people with less experience might look for.
I'm not sure. It's different from "dependencies" that R users are used to where if it's missing, you install it and now things work. And likewise with reticulate packages, blogdown, etc., where you have this external thing you have to install after you install the R package (`install_tensorflow()`, `install_hugo()`, etc.), this is different because you have to re-install the R package.
> * Should we mention that in typical installations the arrow c++ library is either bundled with the installation (e.g. our binaries/CRAN) or built as part of the package installation process. I can't think of a short and pithy way to say this, but what I think we _don't_ want is someone to think that this means they need to now go and `apt install arrow` or the like (that should work for releases as long as the versions match up, but there are lots of ways that could be wrong).
Right. We want to convey the message that the fix for this is to reinstall the R package. But really, if they're here, what we probably want is for them to reinstall with verbosity and then file a JIRA with that output (should we add a `report_installation_issue()` function to do that? ;). Users shouldn't be here normally.
> * Is there any prohibition on offering the url https://arrow.apache.org/docs/r/articles/install.html it's the same content as the vignette, but I suspect more people are familiar with packagdown sites than they are with vignettes these days.
Sure, that's probably better.
> This is separate from this message, but I wonder if we should have another message onload (ideally onload, only the first time like many of the tidyverse packages do) that tests for `codec_is_available("snappy")` (or all of the ones not in the minimal build) that gently nudges that in the case of linux you'll want to set libarrow_binary=true or libarrow_minimal=false to get a more fully featured build? I know that a few times when I was managing rstudio server on linux I installed arrow, thought everything was fine and then later someone had to bug me that snappy wasn't enabled anymore so I had to go back and install it. It's not a huge deal but getting it out of the way when I'm in the installing mindset would have been nice compared to finding out later and having to go back to it.
We could, but if you look at `arrow_info()`, there are lots of features you might not have, and I don't know that we want to message about all of them.
What if we checked `arrow_info()$capabilities`, possibly excluded some we don't think are interesting (for example, I don't have `lzo`, whatever that is, and maybe it's not interesting whether you have all of the memory allocators built), and if any of the rest are false, emit a message like "Some Arrow features are not enabled in this build; see arrow_info() for details and ?arrow_info (or link to vignette) for how to install them"?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#issuecomment-815268349
Revision: 07188a3f03e1e928f334f362267880056df83328
Submitted crossbow builds: [ursacomputing/crossbow @ actions-281](https://github.com/ursacomputing/crossbow/branches/all?query=actions-281)
|Task|Status|
|----|------|
|test-r-rstudio-r-base-3.6-bionic|[](https://dev.azure.com/ursacomputing/crossbow/_build/latest?definitionId=1&branchName=actions-281-azure-test-r-rstudio-r-base-3.6-bionic)|
|test-r-without-arrow|[](https://dev.azure.com/ursacomputing/crossbow/_build/latest?definitionId=1&branchName=actions-281-azure-test-r-without-arrow)|
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on a change in pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#discussion_r607273869
##########
File path: r/tools/nixlibs.R
##########
@@ -333,10 +333,10 @@ build_libarrow <- function(src_dir, dst_dir) {
env_vars <- paste(env_vars, "ARROW_JEMALLOC=OFF ARROW_PARQUET=OFF ARROW_DATASET=OFF ARROW_WITH_RE2=OFF ARROW_WITH_UTF8PROC=OFF EXTRA_CMAKE_FLAGS=-DARROW_SIMD_LEVEL=NONE")
}
cat("**** arrow", ifelse(quietly, "", paste("with", env_vars)), "\n")
- status <- system(
+ status <- suppressWarnings(system(
Review comment:
Ah, ah right of course
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#issuecomment-816238924
> I believe the last remaining issue here is that we are doing `R CMD check --as-cran --run-donttest` and the `\donttest` examples don't pass without arrow C++. I could either (1) hack the test script to be able to turn off `--run-donttest` for _this_ job, or (2) flip all of the examples to `\dontrun`. Opinions @ianmcook @jonkeane ?
If it's easier to just change them to `\dontrun`, I think that's fine. We have ARROW-11849 to switch them to use `@examplesIf` once there's a new roxygen2 on CRAN, and we can change them back at that point.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#issuecomment-813683595
https://issues.apache.org/jira/browse/ARROW-12098
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#issuecomment-816808004
@github-actions crossbow submit test-r-without-arrow
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#issuecomment-814187668
@github-actions crossbow submit test-r-without-arrow test-r-rstudio-r-base-3.6-bionic
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on a change in pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#discussion_r607351892
##########
File path: r/R/arrow-package.R
##########
@@ -47,12 +47,27 @@
}
# Create these once, at package build time
- dplyr_functions$dataset <- build_function_list(build_dataset_expression)
- dplyr_functions$array <- build_function_list(build_array_expression)
-
+ if (arrow_available()) {
+ dplyr_functions$dataset <- build_function_list(build_dataset_expression)
+ dplyr_functions$array <- build_function_list(build_array_expression)
+ }
invisible()
}
+.onAttach <- function(libname, pkgname) {
+ if (!arrow_available()) {
+ msg <- paste(
+ 'The Arrow C++ library is not available. To retry installation with debug output, run:',
+
+ ' install_arrow(verbose = TRUE)',
+
+ 'See `vignette("install", package = "arrow")` for more guidance and troubleshooting.',
Review comment:
> I'm not sure. It's different from "dependencies" that R users are used to where if it's missing, you install it and now things work. And likewise with reticulate packages, blogdown, etc., where you have this external thing you have to install after you install the R package (install_tensorflow(), install_hugo(), etc.), this is different because you have to re-install the R package.
Fair enough, `C++ library` is probably sufficient here
> Right. We want to convey the message that the fix for this is to reinstall the R package. But really, if they're here, what we probably want is for them to reinstall with verbosity and then file a JIRA with that output (should we add a report_installation_issue() function to do that? ;). Users shouldn't be here normally.
`report_installation_issue()` would be slick, especially if it could turn on `ARROW_R_DEV` and copy the input/output automatically.
> We could, but if you look at arrow_info(), there are lots of features you might not have, and I don't know that we want to message about all of them.
>
> What if we checked arrow_info()$capabilities, possibly excluded some we don't think are interesting (for example, I don't have lzo, whatever that is, and maybe it's not interesting whether you have all of the memory allocators built), and if any of the rest are false, emit a message like "Some Arrow features are not enabled in this build; see arrow_info() for details and ?arrow_info (or link to vignette) for how to install them"?
Yeah, honestly I think the only two that are the only important ones are snappy and S3 support. Those are the only two that I've run into or seen issues with on Jira. What if we only alert on those two and leave the other ones be for now?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#issuecomment-816263341
Revision: bb88a58989b3f093d625fc2e1adec60bad80b91c
Submitted crossbow builds: [ursacomputing/crossbow @ actions-300](https://github.com/ursacomputing/crossbow/branches/all?query=actions-300)
|Task|Status|
|----|------|
|test-r-without-arrow|[](https://dev.azure.com/ursacomputing/crossbow/_build/latest?definitionId=1&branchName=actions-300-azure-test-r-without-arrow)|
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#issuecomment-816119170
@github-actions crossbow submit test-r-without-arrow
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on a change in pull request #9896: ARROW-12098: [R] Catch cpp build failures on linux
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #9896:
URL: https://github.com/apache/arrow/pull/9896#discussion_r607269422
##########
File path: r/tools/nixlibs.R
##########
@@ -333,10 +333,10 @@ build_libarrow <- function(src_dir, dst_dir) {
env_vars <- paste(env_vars, "ARROW_JEMALLOC=OFF ARROW_PARQUET=OFF ARROW_DATASET=OFF ARROW_WITH_RE2=OFF ARROW_WITH_UTF8PROC=OFF EXTRA_CMAKE_FLAGS=-DARROW_SIMD_LEVEL=NONE")
}
cat("**** arrow", ifelse(quietly, "", paste("with", env_vars)), "\n")
- status <- system(
+ status <- suppressWarnings(system(
Review comment:
If the command errors, it will print a warning that the command had nonzero exit. But we're handling the error with another message right after, so it's redundant/confusing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org