You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "fernandomayer (via GitHub)" <gi...@apache.org> on 2023/09/30 00:19:22 UTC

[GitHub] [arrow] fernandomayer opened a new pull request, #37961: MINOR: [R] Failing installation script

fernandomayer opened a new pull request, #37961:
URL: https://github.com/apache/arrow/pull/37961

   <!--
   Thanks for opening a pull request!
   If this is your first pull request you can find detailed information on how 
   to contribute here:
     * [New Contributor's Guide](https://arrow.apache.org/docs/dev/developers/guide/step_by_step/pr_lifecycle.html#reviews-and-merge-of-the-pull-request)
     * [Contributing Overview](https://arrow.apache.org/docs/dev/developers/overview.html)
   
   
   If this is not a [minor PR](https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose
   
   Opening GitHub issues ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project.
   
   Then could you also rename the pull request title in the following format?
   
       GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
   
   or
   
       MINOR: [${COMPONENT}] ${SUMMARY}
   
   In the case of PARQUET issues on JIRA the title also supports:
   
       PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
   
   -->
   
   ### 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"
   ````
   
   <!--
    Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
    Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.  
   -->
   
   ### 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.
   
   <!--
   There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
   -->
   
   ### Are these changes tested?
   
   Yes. It was tested with the current code and the proposed PR. The proposed PR works as expected.
   
   <!--
   We typically require tests for all PRs in order to:
   1. Prevent the code from being accidentally broken by subsequent changes
   2. Serve as another way to document the expected behavior of the code
   
   If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
   -->
   
   ### Are there any user-facing changes?
   
   No.
   
   <!--
   If there are user-facing changes then we may require documentation to be updated before approving the PR.
   -->
   
   <!--
   If there are any breaking changes to public APIs, please uncomment the line below and explain which changes are breaking.
   -->
   <!-- **This PR includes breaking changes to public APIs.** -->
   
   No.
   
   <!--
   Please uncomment the line below (and provide explanation) if the changes fix either (a) a security vulnerability, (b) a bug that caused incorrect or invalid data to be produced, or (c) a bug that causes a crash (even when the API contract is upheld). We use this to highlight fixes to issues that may affect users without their knowledge. For this reason, fixing bugs that cause errors don't count, since those are usually obvious.
   -->
   <!-- **This PR contains a "Critical Fix".** -->


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-37907: [R] Setting rosetta variable is missing [arrow]

Posted by "jonkeane (via GitHub)" <gi...@apache.org>.
jonkeane commented on PR #37961:
URL: https://github.com/apache/arrow/pull/37961#issuecomment-1760315088

   I hope you don't mind that I pushed those changes requested to this branch so that we can get this in before the upcoming release.


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-37907: [R] Setting rosetta variable is missing [arrow]

Posted by "jonkeane (via GitHub)" <gi...@apache.org>.
jonkeane commented on code in PR #37961:
URL: https://github.com/apache/arrow/pull/37961#discussion_r1353475618


##########
r/R/install-arrow.R:
##########
@@ -267,3 +268,8 @@ wslify_path <- function(path) {
   end_path <- strsplit(path, drive_expr)[[1]][-1]
   file.path(wslified_drive, end_path)
 }
+
+on_rosetta <- function() {
+  identical(tolower(Sys.info()[["sysname"]]), "darwin") &&
+    identical(system("sysctl -n sysctl.proc_translated", intern = TRUE), "1")

Review Comment:
   We do actually need / want to `suppressWarnings()` and `ignore.stderr = TRUE` here as well. On x86 machines this call will throw a warning + print to the console complaining that it can't find the `proc_translated` argument there. 
   
   ```suggestion
       identical(suppressWarnings(system("sysctl -n sysctl.proc_translated", intern = TRUE, ignore.stderr = TRUE), "1")
   ```



##########
r/R/install-arrow.R:
##########
@@ -267,3 +268,8 @@ wslify_path <- function(path) {
   end_path <- strsplit(path, drive_expr)[[1]][-1]
   file.path(wslified_drive, end_path)
 }
+
+on_rosetta <- function() {
+  identical(tolower(Sys.info()[["sysname"]]), "darwin") &&

Review Comment:
     It would be nice to also keep this comment (though edited a bit for clarity):
   
   ```suggestion
     # make sure to suppress warnings and ignore the stderr so that this is silent on x86
     identical(tolower(Sys.info()[["sysname"]]), "darwin") &&
   ```



##########
r/R/install-arrow.R:
##########
@@ -79,7 +80,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 <- identical(sysname, "darwin") && identical(system("sysctl -n sysctl.proc_translated", intern = TRUE), "1")
+    if (rosetta) {

Review Comment:
   > So my proposal would be to just change the on_rosetta() function from r/R/arrow-package.R to r/R/install-arrow.R, as this would not affect the former, but it would make the latter self-contained again.
   
   This is a good catch, I'm working on a review, but overall I'm pro moving this function to `install-arrow.R` so that it remains self-contained. Thank you for catching it!
   
   >  In this way, the on_rosetta() function is not found, as the script is not self-contained anymore.
   
   We don't have to do this here, but we should at least make an issue that we should write a test that tests that the script remains self-contained. It would have caught this bug, for example!
   



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-37907: [R] Setting rosetta variable is missing [arrow]

Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on PR #37961:
URL: https://github.com/apache/arrow/pull/37961#issuecomment-1742944463

   Thanks for making this PR! One small change, otherwise looks good to go :)


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-37907: [R] Setting rosetta variable is missing [arrow]

Posted by "jonkeane (via GitHub)" <gi...@apache.org>.
jonkeane commented on PR #37961:
URL: https://github.com/apache/arrow/pull/37961#issuecomment-1760315177

   @github-actions crossbow submit test-r-install-local


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-37907: [R] Setting rosetta variable is missing [arrow]

Posted by "jonkeane (via GitHub)" <gi...@apache.org>.
jonkeane commented on PR #37961:
URL: https://github.com/apache/arrow/pull/37961#issuecomment-1761580859

   Thanks again @fernandomayer for the issue + PR


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-37907: [R] Setting rosetta variable is missing [arrow]

Posted by "jonkeane (via GitHub)" <gi...@apache.org>.
jonkeane commented on PR #37961:
URL: https://github.com/apache/arrow/pull/37961#issuecomment-1760316601

   Once the crossbow build is done I'm going to merge this, unless there are objections


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-37907: [R] Setting rosetta variable is missing [arrow]

Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #37961:
URL: https://github.com/apache/arrow/pull/37961#issuecomment-1765523095

   After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 7b7bbdc49250a1a91df079995a38208d55d45d42.
   
   There were no benchmark performance regressions. 🎉
   
   The [full Conbench report](https://github.com/apache/arrow/runs/17762999835) has more details. It also includes information about 6 possible false positives for unstable benchmarks that are known to sometimes produce 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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-37907: [R] Setting rosetta variable is missing [arrow]

Posted by "fernandomayer (via GitHub)" <gi...@apache.org>.
fernandomayer commented on PR #37961:
URL: https://github.com/apache/arrow/pull/37961#issuecomment-1761576746

   Thanks @jonkeane and @thisisnic 
   
   I can confirm that now the command 
   
   ```r
   source("https://raw.githubusercontent.com/apache/arrow/main/r/R/install-arrow.R")
   install_arrow()
   ```
   
   works as expected again, as stated in the documentation https://arrow.apache.org/docs/r/articles/install.html?q=install_arrow#using-install_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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-37907: [R] Setting rosetta variable is missing [arrow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37961:
URL: https://github.com/apache/arrow/pull/37961#issuecomment-1760318335

   Revision: dfec06a3dbc7c42ad96dece52ff35fe72b82617c
   
   Submitted crossbow builds: [ursacomputing/crossbow @ actions-efacb091fb](https://github.com/ursacomputing/crossbow/branches/all?query=actions-efacb091fb)
   
   |Task|Status|
   |----|------|
   |test-r-install-local|[![Github Actions](https://github.com/ursacomputing/crossbow/actions/workflows/crossbow.yml/badge.svg?branch=actions-efacb091fb-github-test-r-install-local)](https://github.com/ursacomputing/crossbow/actions/runs/6500562289/job/17656169537)|


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-37907: [R] Setting rosetta variable is missing [arrow]

Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on code in PR #37961:
URL: https://github.com/apache/arrow/pull/37961#discussion_r1342634870


##########
r/R/install-arrow.R:
##########
@@ -61,6 +61,7 @@ install_arrow <- function(nightly = FALSE,
                           verbose = Sys.getenv("ARROW_R_DEV", FALSE),
                           repos = getOption("repos"),
                           ...) {
+  sysname <- tolower(Sys.info()[["sysname"]])

Review Comment:
   ```suggestion
   ```



##########
r/R/install-arrow.R:
##########
@@ -79,7 +80,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 <- identical(sysname, "darwin") && identical(system("sysctl -n sysctl.proc_translated", intern = TRUE), "1")
+    if (rosetta) {

Review Comment:
   ```suggestion
       rosetta <- on_rosetta()
       if (rosetta) {
   ```
   
   Another PR implemented a function `on_rosetta()` that already does these checks, so we can simplify this bit here, and remove the creation of the `sysname` variable above.



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-37907: [R] Setting rosetta variable is missing [arrow]

Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on code in PR #37961:
URL: https://github.com/apache/arrow/pull/37961#discussion_r1347650593


##########
r/R/install-arrow.R:
##########
@@ -79,7 +80,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 <- identical(sysname, "darwin") && identical(system("sysctl -n sysctl.proc_translated", intern = TRUE), "1")
+    if (rosetta) {

Review Comment:
   I don't see the standalone use being suggested, maybe we need to clarify the docs? As I understand it `install_arrow` is for use when you have the arrow package installed but want to do one of the following:
   ```
   You have arrow installed and want to upgrade to a different version
   You want to try to reinstall and fix issues with Linux C++ binaries
   You want to install a development build
   ```
   @thisisnic please correct me if my assessment on that is wrong?



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-37907: [R] Setting rosetta variable is missing [arrow]

Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on PR #37961:
URL: https://github.com/apache/arrow/pull/37961#issuecomment-1756286287

   @jonkeane You most recently worked on the code that this PR touches and I'm not sure where is best for this function - I don't suppose you'd mind giving this a quick look over?


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-37907: [R] Setting rosetta variable is missing [arrow]

Posted by "jonkeane (via GitHub)" <gi...@apache.org>.
jonkeane commented on PR #37961:
URL: https://github.com/apache/arrow/pull/37961#issuecomment-1760322701

   I've also created https://github.com/apache/arrow/issues/38251 to track adding a test for this self-containedness


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-37907: [R] Setting rosetta variable is missing [arrow]

Posted by "fernandomayer (via GitHub)" <gi...@apache.org>.
fernandomayer commented on code in PR #37961:
URL: https://github.com/apache/arrow/pull/37961#discussion_r1344457832


##########
r/R/install-arrow.R:
##########
@@ -79,7 +80,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 <- identical(sysname, "darwin") && identical(system("sysctl -n sysctl.proc_translated", intern = TRUE), "1")
+    if (rosetta) {

Review Comment:
   Hi @thisisnic 
   
   I believe that for the package this is fine. I saw that the `on_rosetta()` function was defined in `r/R/arrow-package.R` and that should work.
   
   However, I'm thinking of the case where the user just uses the `r/R/install-arrow.R` as a standalone script, as suggested [here](https://arrow.apache.org/docs/r/articles/install.html#using-install_arrow) (as it's the way I use, for example). In this way, the `on_rosetta()` function is not found, as the script is not self-contained anymore.
   
   So my proposal would be to just change the `in_rosetta()` function from `r/R/arrow-package.R` to `r/R/install-arrow.R`, as this would not affect the former, but it would make the latter self-contained again.



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-37907: [R] Setting rosetta variable is missing [arrow]

Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on PR #37961:
URL: https://github.com/apache/arrow/pull/37961#issuecomment-1756145353

   @thisisnic should we merge this before the release?


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-37907: [R] Setting rosetta variable is missing [arrow]

Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on PR #37961:
URL: https://github.com/apache/arrow/pull/37961#issuecomment-1756288146

   > @thisisnic should we merge this before the release?
   
   Yes, but it'll need a final change before merging


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-37907: [R] Setting rosetta variable is missing [arrow]

Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on code in PR #37961:
URL: https://github.com/apache/arrow/pull/37961#discussion_r1350198070


##########
r/R/install-arrow.R:
##########
@@ -79,7 +80,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 <- identical(sysname, "darwin") && identical(system("sysctl -n sysctl.proc_translated", intern = TRUE), "1")
+    if (rosetta) {

Review Comment:
   @assignUser It's here: https://arrow.apache.org/docs/r/articles/install.html?q=install_arrow#using-install_arrow
   
   > Although this function is part of the arrow package, it is also available as a standalone script, so you can access it without first installing the package:
   
   In the case of "You want to install a development build", folks may not already have arrow installed.  
   
   On a different note, there have been other PRs in this area of the codebase merged recently, so this PR may need a rebase to see how the changes affect it, and what needs doing.



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-37907: [R] Setting rosetta variable is missing [arrow]

Posted by "fernandomayer (via GitHub)" <gi...@apache.org>.
fernandomayer commented on code in PR #37961:
URL: https://github.com/apache/arrow/pull/37961#discussion_r1344457832


##########
r/R/install-arrow.R:
##########
@@ -79,7 +80,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 <- identical(sysname, "darwin") && identical(system("sysctl -n sysctl.proc_translated", intern = TRUE), "1")
+    if (rosetta) {

Review Comment:
   Hi @thisisnic 
   
   I believe that for the package this is fine. I saw that the `on_rosetta()` function was defined in `r/R/arrow-package.R` and that should work.
   
   However, I'm thinking of the case where the user just uses the `r/R/install-arrow.R` as a standalone script, as suggested [here](https://arrow.apache.org/docs/r/articles/install.html#using-install_arrow) (as it's the way I use, for example). In this way, the `on_rosetta()` function is not found, as the script is not self-contained anymore.
   
   So my proposal would be to just change the `on_rosetta()` function from `r/R/arrow-package.R` to `r/R/install-arrow.R`, as this would not affect the former, but it would make the latter self-contained again.



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-37907: [R] Setting rosetta variable is missing [arrow]

Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on PR #37961:
URL: https://github.com/apache/arrow/pull/37961#issuecomment-1756646162

   It looks like this would also fix a nightly fail/cran warning: https://github.com/ursacomputing/crossbow/actions/runs/6463099167/job/17545762151#step:6:3794


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-37907: [R] Setting rosetta variable is missing [arrow]

Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic merged PR #37961:
URL: https://github.com/apache/arrow/pull/37961


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-37907: [R] Setting rosetta variable is missing [arrow]

Posted by "jonkeane (via GitHub)" <gi...@apache.org>.
jonkeane commented on code in PR #37961:
URL: https://github.com/apache/arrow/pull/37961#discussion_r1357460372


##########
r/R/install-arrow.R:
##########
@@ -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")
+}

Review Comment:
   ```suggestion
   }
   
   ```
   
   To appease the linter



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org