[GitHub] [arrow] jonkeane commented on a change in pull request #11870: ARROW-14992 [R] Installation can't use prebuilt Arrow binaries on Pop! OS

[GitBox <gi...@apache.org>]

jonkeane commented on a change in pull request #11870:
URL: https://github.com/apache/arrow/pull/11870#discussion_r763065486



##########
File path: r/tools/nixlibs.R
##########
@@ -47,7 +47,7 @@ quietly <- !env_is("ARROW_R_DEV", "true")
 
 # Default is build from source, not download a binary
 build_ok <- !env_is("LIBARROW_BUILD", "false")
-binary_ok <- env_is("LIBARROW_BINARY", "true")
+binary_ok <- env_is("LIBARROW_BINARY", "true") || env_is("NOT_CRAN", "true")

Review comment:
       Would something like `!(env_is("LIBARROW_BINARY", "false") || env_is("LIBARROW_BINARY", ""))` (what was there before work? That way we are really only testing that `LIBARROW_BINARY` is set and not false for determining if binaries are ok.
   
   It's not _super_ common, but I've seen other cases where someone does want to specify one of the more sweeping configuration variables, but then disable something underneath it. (The one that came up a bunch was wanting `LIBARROW_MINIMAL=false`, but then specifically disable S3 with `ARROW_S3=OFF`, and I could imagine someone trying `NOT_CRAN=false` (for a full build) and `LIBARROW_BINARY=false` (because they know they need to build from source). That _could_ be accomplished with `LIBARROW_MINIMAL=false` instead of `NOT_CRAN`, of course, but with the above line I believe that both will work)e




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