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 2022/04/23 22:38:07 UTC

[GitHub] [arrow] karldw opened a new pull request, #12973: ARROW-16297: [R] Improve detection of ARROW_*_URL variables for offline build

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

   As Neal mentioned in https://github.com/apache/arrow/pull/12849#issuecomment-1101489333, the current code in nixlibs.R doesn't handle URL variable names components that have multiple words (because of the way it parses variable names from filenames). Until now, we've had a special case for the AWS variables, but `ARROW_GOOGLE_CLOUD_CPP_URL` and `ARROW_NLOHMANN_JSON_URL` also need handling. Instead of adding special cases, we can provide the correct `ARROW_*_URL` values with the new bash script added as part of ARROW-15092 (in PR #12849).
   
   Please let me know what you think!


-- 
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 #12973: ARROW-16297: [R] Improve detection of ARROW_*_URL variables for offline build

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

   ```
   Unable to match any tasks for `test-r-install-offline`
   The Archery job run can be found at: https://github.com/apache/arrow/actions/runs/2215646916```


-- 
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 #12973: ARROW-16297: [R] Improve detection of ARROW_*_URL variables for offline build

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

   ```
   Unable to match any tasks for `test-r-offline`
   The Archery job run can be found at: https://github.com/apache/arrow/actions/runs/2215654487```


-- 
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] karldw commented on a diff in pull request #12973: ARROW-16297: [R] Improve detection of ARROW_*_URL variables for offline build

Posted by GitBox <gi...@apache.org>.
karldw commented on code in PR #12973:
URL: https://github.com/apache/arrow/pull/12973#discussion_r866790627


##########
r/tools/nixlibs.R:
##########
@@ -435,13 +435,22 @@ turn_off_all_optional_features <- function(env_var_list) {
   # Because these are done as environment variables (as opposed to build flags),
   # setting these to "OFF" overrides any previous setting. We don't need to
   # check the existing value.
+  # Some features turn on other features (e.g. engine -> substrait -> protobuf),
+  # So the list of things to turn off is long. See:
+  # https://github.com/apache/arrow/blob/master/cpp/cmake_modules/ThirdpartyToolchain.cmake#L275
   turn_off <- c(
     "ARROW_MIMALLOC" = "OFF",
     "ARROW_JEMALLOC" = "OFF",
     "ARROW_JSON" = "OFF",
     "ARROW_PARQUET" = "OFF", # depends on thrift
     "ARROW_DATASET" = "OFF", # depends on parquet
     "ARROW_S3" = "OFF",
+    "ARROW_GCS" = "OFF",

Review Comment:
   @nealrichardson, a quick ping here so this doesn't get lost. If the user has a non-default environment variable like ARROW_GCS set to ON and the dependencies aren't available, should the build:
   1. Turn ARROW_GCS OFF or
   2. Fail later when it tries to build the ARROW_GCS component?
   
   Thanks!



-- 
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 #12973: ARROW-16297: [R] Improve detection of ARROW_*_URL variables for offline build

Posted by GitBox <gi...@apache.org>.
nealrichardson closed pull request #12973: ARROW-16297: [R] Improve detection of ARROW_*_URL variables for offline build
URL: https://github.com/apache/arrow/pull/12973


-- 
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] karldw commented on pull request #12973: ARROW-16297: [R] Improve detection of ARROW_*_URL variables for offline build

Posted by GitBox <gi...@apache.org>.
karldw commented on PR #12973:
URL: https://github.com/apache/arrow/pull/12973#issuecomment-1107836590

   @github-actions crossbow submit install-offline


-- 
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] karldw commented on a diff in pull request #12973: ARROW-16297: [R] Improve detection of ARROW_*_URL variables for offline build

Posted by GitBox <gi...@apache.org>.
karldw commented on code in PR #12973:
URL: https://github.com/apache/arrow/pull/12973#discussion_r861385399


##########
r/tools/nixlibs.R:
##########
@@ -435,13 +435,22 @@ turn_off_all_optional_features <- function(env_var_list) {
   # Because these are done as environment variables (as opposed to build flags),
   # setting these to "OFF" overrides any previous setting. We don't need to
   # check the existing value.
+  # Some features turn on other features (e.g. engine -> substrait -> protobuf),
+  # So the list of things to turn off is long. See:
+  # https://github.com/apache/arrow/blob/master/cpp/cmake_modules/ThirdpartyToolchain.cmake#L275
   turn_off <- c(
     "ARROW_MIMALLOC" = "OFF",
     "ARROW_JEMALLOC" = "OFF",
     "ARROW_JSON" = "OFF",
     "ARROW_PARQUET" = "OFF", # depends on thrift
     "ARROW_DATASET" = "OFF", # depends on parquet
     "ARROW_S3" = "OFF",
+    "ARROW_GCS" = "OFF",

Review Comment:
   If the user has set ARROW_GCS to ON and we've reached this function, the build is going to fail unless we turn it OFF. But maybe that's better than turning it off and providing a successful build that doesn't include that component? Let me know what you think behavior should be here.



-- 
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] karldw commented on a diff in pull request #12973: ARROW-16297: [R] Improve detection of ARROW_*_URL variables for offline build

Posted by GitBox <gi...@apache.org>.
karldw commented on code in PR #12973:
URL: https://github.com/apache/arrow/pull/12973#discussion_r861386651


##########
r/tools/nixlibs.R:
##########
@@ -462,27 +471,41 @@ turn_off_all_optional_features <- function(env_var_list) {
   replace(env_var_list, names(turn_off), turn_off)
 }
 
+get_component_names <- function() {
+  if (!isTRUE(Sys.which("bash") != "")) {
+    stop("nixlibs.R requires bash to be installed and available in your PATH")
+  }
+  deps_bash <- normalizePath("tools/download_dependencies_R.sh", mustWork = TRUE)

Review Comment:
   Good point - I was using it as a check that the file exists, but bash error if it doesn't.



-- 
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 #12973: ARROW-16297: [R] Improve detection of ARROW_*_URL variables for offline build

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

   :warning: Ticket **has not been started in JIRA**, please click 'Start Progress'.


-- 
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] karldw commented on a diff in pull request #12973: ARROW-16297: [R] Improve detection of ARROW_*_URL variables for offline build

Posted by GitBox <gi...@apache.org>.
karldw commented on code in PR #12973:
URL: https://github.com/apache/arrow/pull/12973#discussion_r861386651


##########
r/tools/nixlibs.R:
##########
@@ -462,27 +471,41 @@ turn_off_all_optional_features <- function(env_var_list) {
   replace(env_var_list, names(turn_off), turn_off)
 }
 
+get_component_names <- function() {
+  if (!isTRUE(Sys.which("bash") != "")) {
+    stop("nixlibs.R requires bash to be installed and available in your PATH")
+  }
+  deps_bash <- normalizePath("tools/download_dependencies_R.sh", mustWork = TRUE)

Review Comment:
   Good point -- I was using it as a check that the file exists, but bash will error if it doesn't.



-- 
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] karldw commented on pull request #12973: ARROW-16297: [R] Improve detection of ARROW_*_URL variables for offline build

Posted by GitBox <gi...@apache.org>.
karldw commented on PR #12973:
URL: https://github.com/apache/arrow/pull/12973#issuecomment-1107837103

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


-- 
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 diff in pull request #12973: ARROW-16297: [R] Improve detection of ARROW_*_URL variables for offline build

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on code in PR #12973:
URL: https://github.com/apache/arrow/pull/12973#discussion_r857605477


##########
r/tools/nixlibs.R:
##########
@@ -462,27 +471,41 @@ turn_off_all_optional_features <- function(env_var_list) {
   replace(env_var_list, names(turn_off), turn_off)
 }
 
+get_component_names <- function() {
+  if (!isTRUE(Sys.which("bash") != "")) {
+    stop("nixlibs.R requires bash to be installed and available in your PATH")
+  }
+  deps_bash <- normalizePath("tools/download_dependencies_R.sh", mustWork = TRUE)

Review Comment:
   Do you need to normalizePath? Relative path should be fine here



##########
r/tools/nixlibs.R:
##########
@@ -435,13 +435,22 @@ turn_off_all_optional_features <- function(env_var_list) {
   # Because these are done as environment variables (as opposed to build flags),
   # setting these to "OFF" overrides any previous setting. We don't need to
   # check the existing value.
+  # Some features turn on other features (e.g. engine -> substrait -> protobuf),

Review Comment:
   ```suggestion
     # Some features turn on other features (e.g. substrait -> protobuf),
   ```



##########
r/tools/nixlibs.R:
##########
@@ -435,13 +435,22 @@ turn_off_all_optional_features <- function(env_var_list) {
   # Because these are done as environment variables (as opposed to build flags),
   # setting these to "OFF" overrides any previous setting. We don't need to
   # check the existing value.
+  # Some features turn on other features (e.g. engine -> substrait -> protobuf),
+  # So the list of things to turn off is long. See:
+  # https://github.com/apache/arrow/blob/master/cpp/cmake_modules/ThirdpartyToolchain.cmake#L275
   turn_off <- c(
     "ARROW_MIMALLOC" = "OFF",
     "ARROW_JEMALLOC" = "OFF",
     "ARROW_JSON" = "OFF",
     "ARROW_PARQUET" = "OFF", # depends on thrift
     "ARROW_DATASET" = "OFF", # depends on parquet
     "ARROW_S3" = "OFF",
+    "ARROW_GCS" = "OFF",
+    "ARROW_WITH_GOOGLE_CLOUD_CPP" = "OFF",
+    "ARROW_WITH_NLOHMANN_JSON" = "OFF",
+    "ARROW_ENGINE" = "OFF",
+    "ARROW_WITH_SUBSTRAIT" = "OFF",

Review Comment:
   ARROW_ENGINE was just renamed
   
   ```suggestion
       "ARROW_SUBSTRAIT" = "OFF",
   ```



##########
r/tools/nixlibs.R:
##########
@@ -435,13 +435,22 @@ turn_off_all_optional_features <- function(env_var_list) {
   # Because these are done as environment variables (as opposed to build flags),
   # setting these to "OFF" overrides any previous setting. We don't need to
   # check the existing value.
+  # Some features turn on other features (e.g. engine -> substrait -> protobuf),
+  # So the list of things to turn off is long. See:
+  # https://github.com/apache/arrow/blob/master/cpp/cmake_modules/ThirdpartyToolchain.cmake#L275
   turn_off <- c(
     "ARROW_MIMALLOC" = "OFF",
     "ARROW_JEMALLOC" = "OFF",
     "ARROW_JSON" = "OFF",
     "ARROW_PARQUET" = "OFF", # depends on thrift
     "ARROW_DATASET" = "OFF", # depends on parquet
     "ARROW_S3" = "OFF",
+    "ARROW_GCS" = "OFF",

Review Comment:
   These already are OFF by default. Why do we need them OFF here too?



-- 
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 #12973: ARROW-16297: [R] Improve detection of ARROW_*_URL variables for offline build

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

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


-- 
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] karldw commented on pull request #12973: ARROW-16297: [R] Improve detection of ARROW_*_URL variables for offline build

Posted by GitBox <gi...@apache.org>.
karldw commented on PR #12973:
URL: https://github.com/apache/arrow/pull/12973#issuecomment-1107837688

   @github-actions crossbow submit test-r-offline


-- 
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 diff in pull request #12973: ARROW-16297: [R] Improve detection of ARROW_*_URL variables for offline build

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on code in PR #12973:
URL: https://github.com/apache/arrow/pull/12973#discussion_r866827985


##########
r/tools/nixlibs.R:
##########
@@ -435,13 +435,22 @@ turn_off_all_optional_features <- function(env_var_list) {
   # Because these are done as environment variables (as opposed to build flags),
   # setting these to "OFF" overrides any previous setting. We don't need to
   # check the existing value.
+  # Some features turn on other features (e.g. engine -> substrait -> protobuf),
+  # So the list of things to turn off is long. See:
+  # https://github.com/apache/arrow/blob/master/cpp/cmake_modules/ThirdpartyToolchain.cmake#L275
   turn_off <- c(
     "ARROW_MIMALLOC" = "OFF",
     "ARROW_JEMALLOC" = "OFF",
     "ARROW_JSON" = "OFF",
     "ARROW_PARQUET" = "OFF", # depends on thrift
     "ARROW_DATASET" = "OFF", # depends on parquet
     "ARROW_S3" = "OFF",
+    "ARROW_GCS" = "OFF",

Review Comment:
   It's fine to turn it off



-- 
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] ursabot commented on pull request #12973: ARROW-16297: [R] Improve detection of ARROW_*_URL variables for offline build

Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #12973:
URL: https://github.com/apache/arrow/pull/12973#issuecomment-1124786664

   Benchmark runs are scheduled for baseline = 7373b9806acd596882a69e7743cbdbe0aefeaa19 and contender = 1b9f698e6e8424512606590b4a9f8e58266b5a18. 1b9f698e6e8424512606590b4a9f8e58266b5a18 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/083193e373d4432996f6877491d916e6...d4e2b0f105a244a5af169c14237d36fa/)
   [Finished :arrow_down:0.23% :arrow_up:0.04%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/09f9d6b4407d4dcaa69ed10a84f5a1a4...8408ecbc24bc45e8bc0e9a88fa57a1c5/)
   [Finished :arrow_down:0.36% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/24b7232c1ba14d6da84f359c94a698bc...c387afc7cb3048dca62f3e8a8fa934c7/)
   [Finished :arrow_down:0.2% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/0ece99d44a174638bf6a1192d1259b93...fde67707b6974bab905cafff9a88f92c/)
   Buildkite builds:
   [Finished] [`1b9f698e` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/732)
   [Finished] [`1b9f698e` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/729)
   [Finished] [`1b9f698e` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/719)
   [Finished] [`1b9f698e` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/734)
   [Finished] [`7373b980` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/731)
   [Finished] [`7373b980` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/728)
   [Finished] [`7373b980` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/718)
   [Finished] [`7373b980` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/733)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
   test-mac-arm: Supported benchmark langs: C++, Python, R
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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 #12973: ARROW-16297: [R] Improve detection of ARROW_*_URL variables for offline build

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

   Revision: 168b035676cf0ffba2cceb384bba086c3e79238a
   
   Submitted crossbow builds: [ursacomputing/crossbow @ actions-1934](https://github.com/ursacomputing/crossbow/branches/all?query=actions-1934)
   
   |Task|Status|
   |----|------|
   |test-r-offline-maximal|[![Github Actions](https://github.com/ursacomputing/crossbow/workflows/Crossbow/badge.svg?branch=actions-1934-github-test-r-offline-maximal)](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-1934-github-test-r-offline-maximal)|


-- 
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] karldw commented on pull request #12973: ARROW-16297: [R] Improve detection of ARROW_*_URL variables for offline build

Posted by GitBox <gi...@apache.org>.
karldw commented on PR #12973:
URL: https://github.com/apache/arrow/pull/12973#issuecomment-1107838360

   @github-actions crossbow submit test-r-offline-maximal


-- 
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 #12973: ARROW-16297: [R] Improve detection of ARROW_*_URL variables for offline build

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

   ```
   Unable to match any tasks for `install-offline`
   The Archery job run can be found at: https://github.com/apache/arrow/actions/runs/2215640489```


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