Skip to content

Commit

Permalink
apacheGH-37907: [R] Setting rosetta variable is missing (apache#37961)
Browse files Browse the repository at this point in the history
### Rationale for this change

The latest version of `r/R/install-arrow.R`  was not working properly, since it was relying on the `on_rosetta()` function, which is not defined elsewhere. I just fixed the identification of rosetta in the script.

With the current code, the following gives an error

````r
> source("https://raw.githubusercontent.com/apache/arrow/master/r/R/install-arrow.R") 
> install_arrow()
Error in on_rosetta() : could not find function "on_rosetta"
````

### What changes are included in this PR?

It only removed the `on_rosetta()` function, which was not defined elsewhere, and reverted back to the `rosetta` object to identify if rosetta is present or not on a user's system.

### Are these changes tested?

Yes. It was tested with the current code and the proposed PR. The proposed PR works as expected.

### Are there any user-facing changes?

No.

* Closes: apache#37907

Lead-authored-by: Fernando Mayer <[email protected]>
Co-authored-by: Jonathan Keane <[email protected]>
Signed-off-by: Nic Crane <[email protected]>
  • Loading branch information
fernandomayer and jonkeane authored Oct 12, 2023
1 parent 01b42d5 commit 7b7bbdc
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 28 deletions.
6 changes: 0 additions & 6 deletions r/R/arrow-package.R
Original file line number Diff line number Diff line change
Expand Up @@ -241,12 +241,6 @@ on_macos_10_13_or_lower <- function() {
package_version(unname(Sys.info()["release"])) < "18.0.0"
}

on_rosetta <- function() {
# make sure to suppress warnings and ignore the stdout + stderr so that this is silent
identical(tolower(Sys.info()[["sysname"]]), "darwin") &&
identical(suppressWarnings(system("sysctl -n sysctl.proc_translated", intern = TRUE, ignore.stderr = TRUE)), "1")
}

option_use_threads <- function() {
!is_false(getOption("arrow.use_threads"))
}
Expand Down
9 changes: 8 additions & 1 deletion r/R/install-arrow.R
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ install_arrow <- function(nightly = FALSE,
# 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
if (on_rosetta()) {
rosetta <- on_rosetta()
if (rosetta) {
Sys.setenv(ARROW_JEMALLOC = "OFF")
}
if (apple_m1 || rosetta) {
Expand Down Expand Up @@ -267,3 +268,9 @@ wslify_path <- function(path) {
end_path <- strsplit(path, drive_expr)[[1]][-1]
file.path(wslified_drive, end_path)
}

on_rosetta <- function() {
# make sure to suppress warnings and ignore the stderr so that this is silent where proc_translated doesn't exist
identical(tolower(Sys.info()[["sysname"]]), "darwin") &&
identical(suppressWarnings(system("sysctl -n sysctl.proc_translated", intern = TRUE, ignore.stderr = TRUE)), "1")
}
21 changes: 0 additions & 21 deletions r/tests/testthat/test-arrow-pacakge.R

This file was deleted.

5 changes: 5 additions & 0 deletions r/tests/testthat/test-install-arrow.R
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,8 @@ test_that("arrow_repos", {
expect_identical(arrow_repos(c(ours, other), nightly = TRUE), c(ours, other))
})
})

test_that("on_rosetta() does not warn", {
# There is no warning
expect_warning(on_rosetta(), NA)
})

0 comments on commit 7b7bbdc

Please sign in to comment.