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/08/17 13:48:53 UTC

[GitHub] [arrow] nealrichardson opened a new pull request #10950: ARROW-13466: [R] make installation fail if Arrow C++ dependencies cannot be installed

nealrichardson opened a new pull request #10950:
URL: https://github.com/apache/arrow/pull/10950


   The first commit should fail on the "without-arrow" CI build. I'll then add an env var so we can still test that setup. 


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



[GitHub] [arrow] jonkeane commented on a change in pull request #10950: ARROW-13466: [R] make installation fail if Arrow C++ dependencies cannot be installed

Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #10950:
URL: https://github.com/apache/arrow/pull/10950#discussion_r692915500



##########
File path: .github/workflows/r-without-arrow.yml
##########
@@ -73,7 +73,7 @@ jobs:
         run: |
           sudo sysctl -w kernel.core_pattern="core.%e.%p"
           ulimit -c unlimited
-          archery docker run -e LIBARROW_DOWNLOAD=FALSE -e LIBARROW_BUILD=FALSE -e TEST_R_WITH_ARROW=FALSE -e NOT_CRAN=FALSE r
+          archery docker run -e LIBARROW_DOWNLOAD=FALSE -e LIBARROW_BUILD=FALSE -e TEST_R_WITHOUT_LIBARROW=true -e NOT_CRAN=FALSE r

Review comment:
       Very much a nit, but all the others are all-caps `TRUE`/`FALSE`
   
   ```suggestion
             archery docker run -e LIBARROW_DOWNLOAD=FALSE -e LIBARROW_BUILD=FALSE -e TEST_R_WITHOUT_LIBARROW=TRUE -e NOT_CRAN=FALSE r
   ```

##########
File path: r/configure
##########
@@ -255,17 +254,19 @@ if [ $? -eq 0 ] || [ "$UNAME" = "Darwin" ]; then
   echo "PKG_CFLAGS=$PKG_CFLAGS"
   echo "PKG_LIBS=$PKG_LIBS"
 else
-  if [ "$UNAME" = "Darwin" ]; then
-    # Just for debugging: is this possible?
-    echo "Test to load header failed. Command:"
-    echo "$TEST_CMD"
-  fi
   echo "------------------------- NOTE ---------------------------"
   echo "See https://arrow.apache.org/docs/r/articles/install.html"
   echo "for help installing Arrow C++ libraries"
   echo "---------------------------------------------------------"

Review comment:
       ```suggestion
     echo "------------------------- NOTE ---------------------------"
     echo "There was an issue building or finding the Arrow C++ libraries."
     echo "See https://arrow.apache.org/docs/r/articles/install.html"
     echo "for help installing Arrow C++ libraries"
     echo "---------------------------------------------------------"
   ```
   
   I added a bit to make it a little more clear/obvious what happened there. We have the URL which explains it, but I think stating definitively that there was a problem building/finding is good/makes this more understandable. Open to other wordings of course.

##########
File path: r/vignettes/install.Rmd
##########
@@ -174,16 +174,19 @@ The intent is that `install.packages("arrow")` will just work and handle all C++
 dependencies, but depending on your system, you may have better results if you
 tune one of several parameters. Here are some known complications and ways to address them.
 
-## Package installed without C++ dependencies
+## Package failed to build C++ dependencies
 
-If you get an error like
+If you see a message like
 
 ```
-Cannot call io___MemoryMappedFile__Open(). See https://arrow.apache.org/docs/r/articles/install.html for help installing Arrow C++ libraries.
+------------------------- NOTE ---------------------------
+See https://arrow.apache.org/docs/r/articles/install.html
+for help installing Arrow C++ libraries
+---------------------------------------------------------
 ```
 
-for every `arrow` function you call,
-that means that installing the package failed to retrieve or build C++ libraries
+in the output when the package fails to install,
+that means that installation failed to retrieve or build C++ libraries
 compatible with the current version of the R package.

Review comment:
       A few lines below (though I can't seem to commend on them in the GH UI), we should probably suggest _either_ `arrow::install_arrow(verbose = TRUE)` or `Sys.setenv(ARROW_R_DEV=TRUE); install.packages("arrow")` or however the person was installing it, now that they won't have the shell packages to call `arrow::install_arrow()` from. 

##########
File path: r/tests/testthat/test-arrow.R
##########
@@ -76,4 +77,4 @@ test_that("MemoryPool calls gc() to free memory when allocation fails (ARROW-100
   # but it should gc() and retry (and fail again)
   expect_error(BufferOutputStream$create(2**60))
   expect_true(env$gc_was_called)
-})
+})

Review comment:
       ```suggestion
   })
   
   ```
   
   To appease the lintr, styler _should_ do this, but this is easy enough.




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



[GitHub] [arrow] nealrichardson commented on a change in pull request #10950: ARROW-13466: [R] make installation fail if Arrow C++ dependencies cannot be installed

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #10950:
URL: https://github.com/apache/arrow/pull/10950#discussion_r692933362



##########
File path: r/tests/testthat/test-arrow.R
##########
@@ -76,4 +77,4 @@ test_that("MemoryPool calls gc() to free memory when allocation fails (ARROW-100
   # but it should gc() and retry (and fail again)
   expect_error(BufferOutputStream$create(2**60))
   expect_true(env$gc_was_called)
-})
+})

Review comment:
       Yeah this is a case of vscode sometimes stripping the trailing newline




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



[GitHub] [arrow] nealrichardson commented on a change in pull request #10950: ARROW-13466: [R] make installation fail if Arrow C++ dependencies cannot be installed

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #10950:
URL: https://github.com/apache/arrow/pull/10950#discussion_r692934682



##########
File path: ci/scripts/r_test.sh
##########
@@ -41,12 +41,12 @@ if [ "$ARROW_R_DEV" = "TRUE" ]; then
     export NOT_CRAN=true
   fi
 fi
-: ${TEST_R_WITH_ARROW:=TRUE}
-export TEST_R_WITH_ARROW=$TEST_R_WITH_ARROW
 
 export _R_CHECK_CRAN_INCOMING_REMOTE_=FALSE
-# --run-donttest was used in R < 4.0, this is used now
-export _R_CHECK_DONTTEST_EXAMPLES_=$TEST_R_WITH_ARROW
+if [ "$TEST_R_WITHOUT_LIBARROW" != "true" ]; then

Review comment:
       ```suggestion
   if [ "$TEST_R_WITHOUT_LIBARROW" != "TRUE" ]; then
   ```




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



[GitHub] [arrow] nealrichardson commented on pull request #10950: ARROW-13466: [R] make installation fail if Arrow C++ dependencies cannot be installed

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #10950:
URL: https://github.com/apache/arrow/pull/10950#issuecomment-900325571


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

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

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



[GitHub] [arrow] github-actions[bot] commented on pull request #10950: ARROW-13466: [R] make installation fail if Arrow C++ dependencies cannot be installed

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10950:
URL: https://github.com/apache/arrow/pull/10950#issuecomment-900314997


   https://issues.apache.org/jira/browse/ARROW-13466


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



[GitHub] [arrow] nealrichardson commented on a change in pull request #10950: ARROW-13466: [R] make installation fail if Arrow C++ dependencies cannot be installed

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #10950:
URL: https://github.com/apache/arrow/pull/10950#discussion_r692934867



##########
File path: r/configure
##########
@@ -255,17 +254,19 @@ if [ $? -eq 0 ] || [ "$UNAME" = "Darwin" ]; then
   echo "PKG_CFLAGS=$PKG_CFLAGS"
   echo "PKG_LIBS=$PKG_LIBS"
 else
-  if [ "$UNAME" = "Darwin" ]; then
-    # Just for debugging: is this possible?
-    echo "Test to load header failed. Command:"
-    echo "$TEST_CMD"
-  fi
   echo "------------------------- NOTE ---------------------------"
   echo "See https://arrow.apache.org/docs/r/articles/install.html"
   echo "for help installing Arrow C++ libraries"
   echo "---------------------------------------------------------"
   PKG_LIBS=""
   PKG_CFLAGS=""
+  if [ "$UNAME" != "SunOS" ] && [ "$TEST_R_WITHOUT_LIBARROW" != "true" ]; then

Review comment:
       ```suggestion
     if [ "$UNAME" != "SunOS" ] && [ "$TEST_R_WITHOUT_LIBARROW" != "TRUE" ]; then
   ```




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



[GitHub] [arrow] nealrichardson closed pull request #10950: ARROW-13466: [R] make installation fail if Arrow C++ dependencies cannot be installed

Posted by GitBox <gi...@apache.org>.
nealrichardson closed pull request #10950:
URL: https://github.com/apache/arrow/pull/10950


   


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



[GitHub] [arrow] github-actions[bot] commented on pull request #10950: ARROW-13466: [R] make installation fail if Arrow C++ dependencies cannot be installed

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10950:
URL: https://github.com/apache/arrow/pull/10950#issuecomment-900326364


   Revision: c82f6346f1d083e8a69f315c87a9b9df8a2c192f
   
   Submitted crossbow builds: [ursacomputing/crossbow @ actions-780](https://github.com/ursacomputing/crossbow/branches/all?query=actions-780)
   
   |Task|Status|
   |----|------|
   |test-r-without-arrow|[![Azure](https://dev.azure.com/ursacomputing/crossbow/_apis/build/status/ursacomputing.crossbow?branchName=actions-780-azure-test-r-without-arrow)](https://dev.azure.com/ursacomputing/crossbow/_build/latest?definitionId=1&branchName=actions-780-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.

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

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