You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "assignUser (via GitHub)" <gi...@apache.org> on 2023/12/08 18:04:20 UTC
[PR] GH-39147: [R] Add Bootstrap.r [arrow]
assignUser opened a new pull request, #39148:
URL: https://github.com/apache/arrow/pull/39148
### Rationale for this change
We have to complete several different steps before we can `R CMD build` the tarball for CRAN. `bootstrap.R` automates most of them.
### What changes are included in this PR?
When on release version:
- install dev deps for doc building etc.
- download pre-build binary checksums from artifactory
- run {urlchecker}
- remove badges from `DESCRIPTION`
- run `make sync-cpp` and `make doc`
### Are these changes tested?
locally
### Are there any user-facing changes?
No, as it will not be part of the release tarball and if not run with a release version all steps will be skipped to avoid issues with `devtools::install_github` and `pak::pak` that also run the script.
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on code in PR #39148:
URL: https://github.com/apache/arrow/pull/39148#discussion_r1534325428
##########
r/bootstrap.R:
##########
@@ -0,0 +1,77 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+
+
+# exclude_patterns are regular expressions so use ^ to match top-level
+# directories only. Otherwise 'dir' will match 'src/dir' and 'dir/file'.
+rsync <- function(src_dir, dest_dir, exclude_patterns) {
+ all_files <- list.files(src_dir, recursive = TRUE)
+
+ # Filter out excluded patterns
+ for (pattern in exclude_patterns) {
+ all_files <- all_files[!grepl(pattern, all_files)]
+ }
+
+ files_to_vendor_src <- file.path(src_dir, all_files)
+ files_to_vendor_dst <- file.path(dest_dir, all_files)
+
+ # Recreate the directory structure
+ dst_dirs <- unique(dirname(files_to_vendor_dst))
+ for (dst_dir in dst_dirs) {
+ if (!dir.exists(dst_dir)) {
+ dir.create(dst_dir, recursive = TRUE)
+ }
+ }
+
+ # Copy the files
+ if (all(file.copy(files_to_vendor_src, files_to_vendor_dst, overwrite = TRUE))) {
+ cat("All files successfully copied to tools/cpp\n")
+ } else {
+ stop("Failed to vendor all files")
+ }
+}
+
+
+if (dir.exists("../cpp")) {
+ rsync("../cpp",
+ "tools/cpp",
+ exclude_patterns = c(
+ "^apidoc",
+ "^build$",
+ "^build-support/boost-",
+ "^examples",
+ "^submodules",
+ "^src/gandiva",
+ "^src/jni",
+ "^src/skyhook",
+ "_test\\.cc"
+ )
+ )
+
+ # Note: files in tools are available at build time, but not at run time. The thirdparty
+ # cmake expects .env, NOTICE.txt, and LICENSE.txt to be available one level up from cpp/
+ # we must rename .env to dotenv and then replace references to it in cpp/CMakeLists.txt
+ file.copy(from = c("../NOTICE.txt", to = "../LICENSE.txt"), "tools/", overwrite = TRUE)
+ file.copy("../.env", "tools/dotenv", overwrite = TRUE)
+
+ cmake_lists <- readLines("tools/cpp/CMakeLists.txt")
+ cmake_lists <- gsub("\\.env", "dotenv", cmake_lists)
Review Comment:
hm maybe but there is really only one place in the cmake file that this applies to and it would need to use `\B` i think because its a full path? I took the regex from the makefile so it has worked like this up until now :shrug:
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on PR #39148:
URL: https://github.com/apache/arrow/pull/39148#issuecomment-1848102267
> In other words, bootstrap.R will get run on users' machines so it must have a more restricted scope...the makefile commands will only get run on developer machines.
At the moment if run on a dev version it will skip everything and the release tarball won't have the file so a cran source install via pak or so will also not run it. We could make everything outside of `make sync-cpp` explicitly opt in but why install a release version from gh instead of CRAN?
> with bootstrap.R you could change C++ code and have somebody install the pull request version of soemething to see if it fixed an issue)
Ah yes that sounds useful, so we want to change it to always vendor the cpp source (which also means that we probably want to use something other than `rsync` to do it?) and only do the other release stuff on release version/opt-in.
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on PR #39148:
URL: https://github.com/apache/arrow/pull/39148#issuecomment-2011221904
@paleolimbot as discussed last year (wow!?) I have refactored this to just be the R version of `make sync-cpp`. I have compared the list of copied files and this only misses .gitignore files (using `all.files=T` also doesn't fix that?).
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on code in PR #39148:
URL: https://github.com/apache/arrow/pull/39148#discussion_r1534347882
##########
r/bootstrap.R:
##########
@@ -0,0 +1,77 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+
+
+# exclude_patterns are regular expressions so use ^ to match top-level
+# directories only. Otherwise 'dir' will match 'src/dir' and 'dir/file'.
+rsync <- function(src_dir, dest_dir, exclude_patterns) {
+ all_files <- list.files(src_dir, recursive = TRUE)
+
+ # Filter out excluded patterns
+ for (pattern in exclude_patterns) {
+ all_files <- all_files[!grepl(pattern, all_files)]
+ }
+
+ files_to_vendor_src <- file.path(src_dir, all_files)
+ files_to_vendor_dst <- file.path(dest_dir, all_files)
+
+ # Recreate the directory structure
+ dst_dirs <- unique(dirname(files_to_vendor_dst))
+ for (dst_dir in dst_dirs) {
+ if (!dir.exists(dst_dir)) {
+ dir.create(dst_dir, recursive = TRUE)
+ }
+ }
+
+ # Copy the files
+ if (all(file.copy(files_to_vendor_src, files_to_vendor_dst, overwrite = TRUE))) {
+ cat("All files successfully copied to tools/cpp\n")
+ } else {
+ stop("Failed to vendor all files")
+ }
+}
+
+
+if (dir.exists("../cpp")) {
+ rsync("../cpp",
+ "tools/cpp",
+ exclude_patterns = c(
+ "^apidoc",
+ "^build$",
+ "^build-support/boost-",
+ "^examples",
+ "^submodules",
+ "^src/gandiva",
+ "^src/jni",
+ "^src/skyhook",
+ "_test\\.cc"
+ )
+ )
+
+ # Note: files in tools are available at build time, but not at run time. The thirdparty
+ # cmake expects .env, NOTICE.txt, and LICENSE.txt to be available one level up from cpp/
+ # we must rename .env to dotenv and then replace references to it in cpp/CMakeLists.txt
+ file.copy(from = c("../NOTICE.txt", to = "../LICENSE.txt"), "tools/", overwrite = TRUE)
+ file.copy("../.env", "tools/dotenv", overwrite = TRUE)
+
+ cmake_lists <- readLines("tools/cpp/CMakeLists.txt")
+ cmake_lists <- gsub("\\.env", "dotenv", cmake_lists)
Review Comment:
I'll leave it as is for now.
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on code in PR #39148:
URL: https://github.com/apache/arrow/pull/39148#discussion_r1423173827
##########
r/bootstrap.R:
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+if (!dir.exists('../cpp')) {
+ cli::cli_alert_warning("Arrow C++ sources not found, skipping bootstrap.")
+ q(save = "no")
+}
+
+pkg_version <- package_version(desc::desc_get("Version"))
+is_release <- is.na(pkg_version[1, 4]) || pkg_version[1, 4] < "100"
+# for testing and CI
+force_bootstrap <- identical(tolower(Sys.getenv("ARROW_FORCE_BOOTSTRAP", unset = "false")), "true")
+
+get_checksum_version <- function() {
+ url_exists <- function(url) {
+ headers <- curlGetHeaders(url)
+ attr(headers, "status") == 200
+ }
+ # Run update-checksums.R manually if the checksums already exists
+ # but have an unexpected version.
+ if (dir.exists("tools/checksums") || force_bootstrap) {
+ cli::cli_alert_success("Found existing checksums, skipping download.")
+ return(NA)
+ }
+
+ # We currently don't support prebuild binaries with a differing version.
+ jfrog_url <- "https://apache.jfrog.io/artifactory/arrow/r/%s/libarrow"
+ matching_url <- url_exists(sprintf(jfrog_url, pkg_version[1, 1:3]))
+ if (matching_url) {
+ version <- pkg_version[1, 1:3]
+ } else {
+ cli::cli_abort("No matching checksums found, can't proceed.")
+ }
+ version
+}
+
+cli::cli_alert_info("Vendoring C++ sources")
+system2("make", c("sync-cpp"))
Review Comment:
I tried it on a default r install on windows with a default rtools install and it worked but I agree that we shouldn't rely on that.
>The recursive copy written in R I use is here
nice
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "amoeba (via GitHub)" <gi...@apache.org>.
amoeba commented on code in PR #39148:
URL: https://github.com/apache/arrow/pull/39148#discussion_r1421210665
##########
r/bootstrap.R:
##########
@@ -0,0 +1,84 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+git_sc <- system2("git", c("rev-parse", "--is-inside-work-tree"), stdout = FALSE, stderr = FALSE)
+if (git_sc != 0) {
+ cli::cli_alert_warning("Not in a git repository, skipping bootstrap.R")
+ q(save = "no")
+}
+
+pkg_version <- package_version(desc::desc_get("Version"))
+is_release <- is.na(pkg_version[1, 4]) || pkg_version[1, 4] < "100"
+# for testing and CI
+force_bootstrap <- identical(tolower(Sys.getenv("ARROW_FORCE_BOOTSTRAP", default = "false")), "true")
+
+get_checksum_version <- function() {
+ url_exists <- function(url) {
+ headers <- curlGetHeaders(url)
+ attr(headers, "status") == 200
+ }
+ # Run update-checksums.R manually if the checksums already exists
+ # but have an unexpected version.
+ if (dir.exists("tools/checksums") || force_bootstrap) {
+ cli::cli_alert_success("Found existing checksums, skipping download.")
+ return(NA)
+ }
+
+ # We currently don't support prebuild binaries with a differing version.
+ jfrog_url <- "https://apache.jfrog.io/artifactory/arrow/r/%s/libarrow"
+ matching_url <- url_exists(sprintf(jfrog_url, pkg_version[1, 1:3]))
+ if (matching_url) {
+ version <- pkg_version[1, 1:3]
+ } else {
+ stop("No matching checksums found, can't proceed.")
+ }
+ version
+}
+
+if (!is_release) {
+ cli::cli_alert_warning("Skipping releases preparations for dev version.")
+ q(save = "no")
+}
+
+cli::cli_alert_info("Installing dependencies")
+devtools::install_dev_deps(".", upgrade = "never")
+devtools::install_cran("styler")
+
+cli::cli_alert_info("Removing badges from Readme.md")
+system2("sed", c("-i", "'/^<!--- badges: start -->$/,/^<!--- badges: stop -->$/d'", "README.md"))
Review Comment:
macOS uses BSD sed which needs to be called like,
```
sed -i -e '/^<!--- badges: start -->$/,/^<!--- badges: stop -->$/d' README.md
```
The alternative would be to use gsed on macOS and require whoever runs this to install gsed (like from brew).
##########
r/bootstrap.R:
##########
@@ -0,0 +1,84 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+git_sc <- system2("git", c("rev-parse", "--is-inside-work-tree"), stdout = FALSE, stderr = FALSE)
+if (git_sc != 0) {
+ cli::cli_alert_warning("Not in a git repository, skipping bootstrap.R")
+ q(save = "no")
+}
+
+pkg_version <- package_version(desc::desc_get("Version"))
+is_release <- is.na(pkg_version[1, 4]) || pkg_version[1, 4] < "100"
+# for testing and CI
+force_bootstrap <- identical(tolower(Sys.getenv("ARROW_FORCE_BOOTSTRAP", default = "false")), "true")
+
+get_checksum_version <- function() {
+ url_exists <- function(url) {
+ headers <- curlGetHeaders(url)
+ attr(headers, "status") == 200
+ }
+ # Run update-checksums.R manually if the checksums already exists
+ # but have an unexpected version.
+ if (dir.exists("tools/checksums") || force_bootstrap) {
+ cli::cli_alert_success("Found existing checksums, skipping download.")
+ return(NA)
+ }
+
+ # We currently don't support prebuild binaries with a differing version.
+ jfrog_url <- "https://apache.jfrog.io/artifactory/arrow/r/%s/libarrow"
+ matching_url <- url_exists(sprintf(jfrog_url, pkg_version[1, 1:3]))
+ if (matching_url) {
+ version <- pkg_version[1, 1:3]
+ } else {
+ stop("No matching checksums found, can't proceed.")
+ }
+ version
+}
+
+if (!is_release) {
+ cli::cli_alert_warning("Skipping releases preparations for dev version.")
+ q(save = "no")
+}
+
+cli::cli_alert_info("Installing dependencies")
+devtools::install_dev_deps(".", upgrade = "never")
+devtools::install_cran("styler")
+
+cli::cli_alert_info("Removing badges from Readme.md")
+system2("sed", c("-i", "'/^<!--- badges: start -->$/,/^<!--- badges: stop -->$/d'", "README.md"))
Review Comment:
It looks like update-checksums-R also calls sed assuming it's GNU sed not BSD so whatever change is made here should be made there too.
##########
r/bootstrap.R:
##########
@@ -0,0 +1,84 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+git_sc <- system2("git", c("rev-parse", "--is-inside-work-tree"), stdout = FALSE, stderr = FALSE)
+if (git_sc != 0) {
+ cli::cli_alert_warning("Not in a git repository, skipping bootstrap.R")
+ q(save = "no")
+}
+
+pkg_version <- package_version(desc::desc_get("Version"))
+is_release <- is.na(pkg_version[1, 4]) || pkg_version[1, 4] < "100"
+# for testing and CI
+force_bootstrap <- identical(tolower(Sys.getenv("ARROW_FORCE_BOOTSTRAP", default = "false")), "true")
Review Comment:
What is the `default` arg here? Did you mean `unset`?
##########
r/bootstrap.R:
##########
@@ -0,0 +1,84 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+git_sc <- system2("git", c("rev-parse", "--is-inside-work-tree"), stdout = FALSE, stderr = FALSE)
+if (git_sc != 0) {
+ cli::cli_alert_warning("Not in a git repository, skipping bootstrap.R")
+ q(save = "no")
+}
+
+pkg_version <- package_version(desc::desc_get("Version"))
+is_release <- is.na(pkg_version[1, 4]) || pkg_version[1, 4] < "100"
+# for testing and CI
+force_bootstrap <- identical(tolower(Sys.getenv("ARROW_FORCE_BOOTSTRAP", default = "false")), "true")
+
+get_checksum_version <- function() {
+ url_exists <- function(url) {
+ headers <- curlGetHeaders(url)
+ attr(headers, "status") == 200
+ }
+ # Run update-checksums.R manually if the checksums already exists
+ # but have an unexpected version.
+ if (dir.exists("tools/checksums") || force_bootstrap) {
+ cli::cli_alert_success("Found existing checksums, skipping download.")
+ return(NA)
+ }
+
+ # We currently don't support prebuild binaries with a differing version.
+ jfrog_url <- "https://apache.jfrog.io/artifactory/arrow/r/%s/libarrow"
+ matching_url <- url_exists(sprintf(jfrog_url, pkg_version[1, 1:3]))
+ if (matching_url) {
+ version <- pkg_version[1, 1:3]
+ } else {
+ stop("No matching checksums found, can't proceed.")
Review Comment:
Could this be `cli::cli_abort` to match how we stop on line 69?
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #39148:
URL: https://github.com/apache/arrow/pull/39148#discussion_r1424161582
##########
r/bootstrap.R:
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+if (!dir.exists('../cpp')) {
+ cli::cli_alert_warning("Arrow C++ sources not found, skipping bootstrap.")
+ q(save = "no")
+}
+
+pkg_version <- package_version(desc::desc_get("Version"))
+is_release <- is.na(pkg_version[1, 4]) || pkg_version[1, 4] < "100"
+# for testing and CI
+force_bootstrap <- identical(tolower(Sys.getenv("ARROW_FORCE_BOOTSTRAP", unset = "false")), "true")
+
+get_checksum_version <- function() {
+ url_exists <- function(url) {
+ headers <- curlGetHeaders(url)
+ attr(headers, "status") == 200
+ }
+ # Run update-checksums.R manually if the checksums already exists
+ # but have an unexpected version.
+ if (dir.exists("tools/checksums") || force_bootstrap) {
+ cli::cli_alert_success("Found existing checksums, skipping download.")
+ return(NA)
+ }
+
+ # We currently don't support prebuild binaries with a differing version.
+ jfrog_url <- "https://apache.jfrog.io/artifactory/arrow/r/%s/libarrow"
+ matching_url <- url_exists(sprintf(jfrog_url, pkg_version[1, 1:3]))
+ if (matching_url) {
+ version <- pkg_version[1, 1:3]
+ } else {
+ cli::cli_abort("No matching checksums found, can't proceed.")
+ }
+ version
+}
+
+cli::cli_alert_info("Vendoring C++ sources")
+system2("make", c("sync-cpp"))
+
+if (!is_release && !force_bootstrap) {
+ cli::cli_alert_warning("Skipping releases preparations for dev version.")
+ q(save = "no")
+}
+
+cli::cli_alert_info("Installing dependencies")
+devtools::install_dev_deps(".", upgrade = "never")
+devtools::install_cran("styler")
+
+cli::cli_alert_info("Removing badges from Readme.md")
+system2("sed", c("-i", "-e", "'/^<!--- badges: start -->$/,/^<!--- badges: stop -->$/d'", "README.md"))
Review Comment:
Ah sorry, I thought this step was automatic in one of the version bumping scripts we had already (but that's not the case)
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on code in PR #39148:
URL: https://github.com/apache/arrow/pull/39148#discussion_r1423176438
##########
r/bootstrap.R:
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+if (!dir.exists('../cpp')) {
+ cli::cli_alert_warning("Arrow C++ sources not found, skipping bootstrap.")
+ q(save = "no")
Review Comment:
But that would produce an error if you have a tarball of just /r (potentially with sources already vendored) and the file really isn't there to be `source`d so I don't think that's a valid concern (unlike the `install-arrow.R` case where it also had a standalone functionality).
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on code in PR #39148:
URL: https://github.com/apache/arrow/pull/39148#discussion_r1423177991
##########
r/bootstrap.R:
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+if (!dir.exists('../cpp')) {
+ cli::cli_alert_warning("Arrow C++ sources not found, skipping bootstrap.")
+ q(save = "no")
+}
+
+pkg_version <- package_version(desc::desc_get("Version"))
+is_release <- is.na(pkg_version[1, 4]) || pkg_version[1, 4] < "100"
+# for testing and CI
+force_bootstrap <- identical(tolower(Sys.getenv("ARROW_FORCE_BOOTSTRAP", unset = "false")), "true")
+
+get_checksum_version <- function() {
+ url_exists <- function(url) {
+ headers <- curlGetHeaders(url)
+ attr(headers, "status") == 200
+ }
+ # Run update-checksums.R manually if the checksums already exists
+ # but have an unexpected version.
+ if (dir.exists("tools/checksums") || force_bootstrap) {
+ cli::cli_alert_success("Found existing checksums, skipping download.")
+ return(NA)
+ }
+
+ # We currently don't support prebuild binaries with a differing version.
+ jfrog_url <- "https://apache.jfrog.io/artifactory/arrow/r/%s/libarrow"
+ matching_url <- url_exists(sprintf(jfrog_url, pkg_version[1, 1:3]))
+ if (matching_url) {
+ version <- pkg_version[1, 1:3]
+ } else {
+ cli::cli_abort("No matching checksums found, can't proceed.")
+ }
+ version
+}
+
+cli::cli_alert_info("Vendoring C++ sources")
+system2("make", c("sync-cpp"))
+
+if (!is_release && !force_bootstrap) {
+ cli::cli_alert_warning("Skipping releases preparations for dev version.")
+ q(save = "no")
+}
+
+cli::cli_alert_info("Installing dependencies")
+devtools::install_dev_deps(".", upgrade = "never")
+devtools::install_cran("styler")
+
+cli::cli_alert_info("Removing badges from Readme.md")
+system2("sed", c("-i", "-e", "'/^<!--- badges: start -->$/,/^<!--- badges: stop -->$/d'", "README.md"))
+
+cli::cli_alert_info("Running urlchecker")
+url_res <- urlchecker::url_check()
+print(url_res)
+
+if (nrow(url_res) > 0 && !force_bootstrap) {
+ cli::cli_abort("Broken URLs found, can't proceed.")
+}
+
+VERSION <- get_checksum_version()
+if (!is.na(VERSION)) {
+ cli::cli_alert_info("Updating checksums for libarrow version: {VERSION}")
+ source("tools/update-checksums.R", local = TRUE)
+}
+
+cli::cli_alert_info("Building documentation")
+system2("make", c("doc"))
Review Comment:
it also runs some data gen script in addition to roxygen so probably not? But I would have to take a look. If we move all of this prep functionality into a separate file just for use in releases we don't really need to do it in pure R .
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #39148:
URL: https://github.com/apache/arrow/pull/39148#issuecomment-2014209627
After merging your PR, Conbench analyzed the 7 benchmarking runs that have been run so far on merge-commit dc20d9539ece9db704a616094aed9069ee7b89a7.
There were 2 benchmark results indicating a performance regression:
- Commit Run on `ursa-i9-9960x` at [2024-03-21 23:47:20Z](https://conbench.ursa.dev/compare/runs/39640e316974468786ab5ecd8b1932df...fec69fa003f54caaadb88a20cffdb616/)
- [`dataframe-to-table` (R) with dataset=type_integers, language=R](https://conbench.ursa.dev/compare/benchmarks/065fcb3c01ae756080002c45f0d8dbb7...065fcd47f4e27e9b80007150f0f06710)
- [`dataframe-to-table` (R) with dataset=type_floats, language=R](https://conbench.ursa.dev/compare/benchmarks/065fcb3c910b7ec18000f550b7764f08...065fcd48cd0371a58000859f179419a5)
The [full Conbench report](https://github.com/apache/arrow/runs/22962350732) has more details.
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #39148:
URL: https://github.com/apache/arrow/pull/39148#discussion_r1534124676
##########
r/bootstrap.R:
##########
@@ -0,0 +1,77 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+
+
+# exclude_patterns are regular expressions so use ^ to match top-level
+# directories only. Otherwise 'dir' will match 'src/dir' and 'dir/file'.
+rsync <- function(src_dir, dest_dir, exclude_patterns) {
+ all_files <- list.files(src_dir, recursive = TRUE)
+
+ # Filter out excluded patterns
+ for (pattern in exclude_patterns) {
+ all_files <- all_files[!grepl(pattern, all_files)]
+ }
+
+ files_to_vendor_src <- file.path(src_dir, all_files)
+ files_to_vendor_dst <- file.path(dest_dir, all_files)
+
+ # Recreate the directory structure
+ dst_dirs <- unique(dirname(files_to_vendor_dst))
+ for (dst_dir in dst_dirs) {
+ if (!dir.exists(dst_dir)) {
+ dir.create(dst_dir, recursive = TRUE)
+ }
+ }
+
+ # Copy the files
+ if (all(file.copy(files_to_vendor_src, files_to_vendor_dst, overwrite = TRUE))) {
+ cat("All files successfully copied to tools/cpp\n")
+ } else {
+ stop("Failed to vendor all files")
+ }
+}
+
+
+if (dir.exists("../cpp")) {
+ rsync("../cpp",
+ "tools/cpp",
+ exclude_patterns = c(
+ "^apidoc",
+ "^build$",
+ "^build-support/boost-",
+ "^examples",
+ "^submodules",
+ "^src/gandiva",
+ "^src/jni",
+ "^src/skyhook",
+ "_test\\.cc"
+ )
+ )
+
+ # Note: files in tools are available at build time, but not at run time. The thirdparty
+ # cmake expects .env, NOTICE.txt, and LICENSE.txt to be available one level up from cpp/
+ # we must rename .env to dotenv and then replace references to it in cpp/CMakeLists.txt
+ file.copy(from = c("../NOTICE.txt", to = "../LICENSE.txt"), "tools/", overwrite = TRUE)
+ file.copy("../.env", "tools/dotenv", overwrite = TRUE)
+
+ cmake_lists <- readLines("tools/cpp/CMakeLists.txt")
+ cmake_lists <- gsub("\\.env", "dotenv", cmake_lists)
Review Comment:
Is this any safer as `"\\b\\.env\\b"`?
##########
r/bootstrap.R:
##########
@@ -0,0 +1,77 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+
+
+# exclude_patterns are regular expressions so use ^ to match top-level
+# directories only. Otherwise 'dir' will match 'src/dir' and 'dir/file'.
+rsync <- function(src_dir, dest_dir, exclude_patterns) {
+ all_files <- list.files(src_dir, recursive = TRUE)
+
Review Comment:
Do we want an option to clean the `dest_dir` to prevent files that were removed from persisting into a tarball? Or maybe every time?
##########
r/bootstrap.R:
##########
@@ -0,0 +1,77 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+
+
+# exclude_patterns are regular expressions so use ^ to match top-level
+# directories only. Otherwise 'dir' will match 'src/dir' and 'dir/file'.
+rsync <- function(src_dir, dest_dir, exclude_patterns) {
+ all_files <- list.files(src_dir, recursive = TRUE)
+
+ # Filter out excluded patterns
+ for (pattern in exclude_patterns) {
+ all_files <- all_files[!grepl(pattern, all_files)]
+ }
+
+ files_to_vendor_src <- file.path(src_dir, all_files)
+ files_to_vendor_dst <- file.path(dest_dir, all_files)
+
+ # Recreate the directory structure
+ dst_dirs <- unique(dirname(files_to_vendor_dst))
+ for (dst_dir in dst_dirs) {
+ if (!dir.exists(dst_dir)) {
+ dir.create(dst_dir, recursive = TRUE)
+ }
+ }
+
+ # Copy the files
+ if (all(file.copy(files_to_vendor_src, files_to_vendor_dst, overwrite = TRUE))) {
+ cat("All files successfully copied to tools/cpp\n")
+ } else {
+ stop("Failed to vendor all files")
+ }
+}
+
+
+if (dir.exists("../cpp")) {
+ rsync("../cpp",
+ "tools/cpp",
+ exclude_patterns = c(
+ "^apidoc",
+ "^build$",
Review Comment:
```suggestion
"^build/",
```
(Or else my build directory is copied into the tarball with many warnings ensuing!)
##########
r/bootstrap.R:
##########
@@ -0,0 +1,77 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+
+
+# exclude_patterns are regular expressions so use ^ to match top-level
+# directories only. Otherwise 'dir' will match 'src/dir' and 'dir/file'.
+rsync <- function(src_dir, dest_dir, exclude_patterns) {
+ all_files <- list.files(src_dir, recursive = TRUE)
+
+ # Filter out excluded patterns
+ for (pattern in exclude_patterns) {
+ all_files <- all_files[!grepl(pattern, all_files)]
+ }
+
+ files_to_vendor_src <- file.path(src_dir, all_files)
+ files_to_vendor_dst <- file.path(dest_dir, all_files)
+
+ # Recreate the directory structure
+ dst_dirs <- unique(dirname(files_to_vendor_dst))
+ for (dst_dir in dst_dirs) {
+ if (!dir.exists(dst_dir)) {
+ dir.create(dst_dir, recursive = TRUE)
+ }
+ }
+
+ # Copy the files
+ if (all(file.copy(files_to_vendor_src, files_to_vendor_dst, overwrite = TRUE))) {
+ cat("All files successfully copied to tools/cpp\n")
+ } else {
+ stop("Failed to vendor all files")
+ }
+}
+
+
+if (dir.exists("../cpp")) {
+ rsync("../cpp",
+ "tools/cpp",
+ exclude_patterns = c(
+ "^apidoc",
+ "^build$",
+ "^build-support/boost-",
+ "^examples",
+ "^submodules",
+ "^src/gandiva",
+ "^src/jni",
+ "^src/skyhook",
+ "_test\\.cc"
+ )
+ )
+
+ # Note: files in tools are available at build time, but not at run time. The thirdparty
+ # cmake expects .env, NOTICE.txt, and LICENSE.txt to be available one level up from cpp/
+ # we must rename .env to dotenv and then replace references to it in cpp/CMakeLists.txt
+ file.copy(from = c("../NOTICE.txt", to = "../LICENSE.txt"), "tools/", overwrite = TRUE)
+ file.copy("../.env", "tools/dotenv", overwrite = TRUE)
Review Comment:
I am a little confused as to how these two statements are connected. Presumably the file has to be renamed because R CMD check complains about hidden files (which might be nice to put 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
Re: [PR] GH-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on PR #39148:
URL: https://github.com/apache/arrow/pull/39148#issuecomment-1848022416
@github-actions crossbow submit r-binary-packages
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on PR #39148:
URL: https://github.com/apache/arrow/pull/39148#issuecomment-1848073744
I have removed `devtools::build` from r-binary-packages again, installing the dependencies just takes to long. It also makes more sense to add a specific 'prepare release' job, if we create more automation (e.g. cherry picking things) we can test that there as well.
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #39148:
URL: https://github.com/apache/arrow/pull/39148#issuecomment-1847605737
:warning: GitHub issue #39147 **has been automatically assigned in GitHub** to PR creator.
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on code in PR #39148:
URL: https://github.com/apache/arrow/pull/39148#discussion_r1534319998
##########
r/bootstrap.R:
##########
@@ -0,0 +1,77 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+
+
+# exclude_patterns are regular expressions so use ^ to match top-level
+# directories only. Otherwise 'dir' will match 'src/dir' and 'dir/file'.
+rsync <- function(src_dir, dest_dir, exclude_patterns) {
+ all_files <- list.files(src_dir, recursive = TRUE)
+
Review Comment:
hm, that would only really be an issue for the actual release and you'd do `make clean` before + run the doc changes etc. as well. We could just unlink dest dir before recreating the structure if that doesn't produce an error if the dir doesn't exits?
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on code in PR #39148:
URL: https://github.com/apache/arrow/pull/39148#discussion_r1423182640
##########
r/bootstrap.R:
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+if (!dir.exists('../cpp')) {
+ cli::cli_alert_warning("Arrow C++ sources not found, skipping bootstrap.")
+ q(save = "no")
+}
+
+pkg_version <- package_version(desc::desc_get("Version"))
+is_release <- is.na(pkg_version[1, 4]) || pkg_version[1, 4] < "100"
+# for testing and CI
+force_bootstrap <- identical(tolower(Sys.getenv("ARROW_FORCE_BOOTSTRAP", unset = "false")), "true")
+
+get_checksum_version <- function() {
+ url_exists <- function(url) {
+ headers <- curlGetHeaders(url)
+ attr(headers, "status") == 200
+ }
+ # Run update-checksums.R manually if the checksums already exists
+ # but have an unexpected version.
+ if (dir.exists("tools/checksums") || force_bootstrap) {
+ cli::cli_alert_success("Found existing checksums, skipping download.")
+ return(NA)
+ }
+
+ # We currently don't support prebuild binaries with a differing version.
+ jfrog_url <- "https://apache.jfrog.io/artifactory/arrow/r/%s/libarrow"
+ matching_url <- url_exists(sprintf(jfrog_url, pkg_version[1, 1:3]))
+ if (matching_url) {
+ version <- pkg_version[1, 1:3]
+ } else {
+ cli::cli_abort("No matching checksums found, can't proceed.")
+ }
+ version
+}
+
+cli::cli_alert_info("Vendoring C++ sources")
+system2("make", c("sync-cpp"))
+
+if (!is_release && !force_bootstrap) {
+ cli::cli_alert_warning("Skipping releases preparations for dev version.")
+ q(save = "no")
+}
+
+cli::cli_alert_info("Installing dependencies")
+devtools::install_dev_deps(".", upgrade = "never")
+devtools::install_cran("styler")
+
+cli::cli_alert_info("Removing badges from Readme.md")
+system2("sed", c("-i", "-e", "'/^<!--- badges: start -->$/,/^<!--- badges: stop -->$/d'", "README.md"))
Review Comment:
No the first make build/devtools::build() happens after the cran branch is created of the rc branch. (and these changes would be rebased onto the final release branch as well)
But maybe it is time to integrate the cran branch into the normal process within 'apache/arrow' (aided by these new automations) instead of haiving it different contributor forks (potentially getting lost...)
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on code in PR #39148:
URL: https://github.com/apache/arrow/pull/39148#discussion_r1421215028
##########
r/bootstrap.R:
##########
@@ -0,0 +1,84 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+git_sc <- system2("git", c("rev-parse", "--is-inside-work-tree"), stdout = FALSE, stderr = FALSE)
+if (git_sc != 0) {
+ cli::cli_alert_warning("Not in a git repository, skipping bootstrap.R")
+ q(save = "no")
+}
+
+pkg_version <- package_version(desc::desc_get("Version"))
+is_release <- is.na(pkg_version[1, 4]) || pkg_version[1, 4] < "100"
+# for testing and CI
+force_bootstrap <- identical(tolower(Sys.getenv("ARROW_FORCE_BOOTSTRAP", default = "false")), "true")
Review Comment:
:facepalm: yes.
```suggestion
force_bootstrap <- identical(tolower(Sys.getenv("ARROW_FORCE_BOOTSTRAP", unset = "false")), "true")
```
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on PR #39148:
URL: https://github.com/apache/arrow/pull/39148#issuecomment-1848197948
`devtools::install_github('assignUser/arrow@bootstrap.r')` with `LIBARROW_BINARY=false` set correctly vendors cpp and builds from source now:
```
Running bootstrap.R...
ℹ Vendoring C++ sources
cp ../NOTICE.txt inst/NOTICE.txt
rsync --archive --delete --exclude 'apidoc' --exclude 'build' --exclude 'build-support/boost_*' --exclude 'examples' --exclude 'src/gandiva' --exclude 'src/jni' --exclude 'src/skyhook' --exclude 'submodules' --exclude '**/*_test.cc' ../cpp tools/
cp -p ../.env tools/dotenv
cp -p ../NOTICE.txt tools/
cp -p ../LICENSE.txt tools/
sed -i"" -e "s/\.env/dotenv/g" tools/cpp/CMakeLists.txt
! Skipping releases preparations for dev version.
── R CMD build ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
✔ checking for file ‘/tmp/Rtmp2g1C7O/remotes1d33ded19da00/assignUser-arrow-25f733d/r/DESCRIPTION’ ...
─ preparing ‘arrow’:
✔ checking DESCRIPTION meta-information ...
─ cleaning src
─ running ‘cleanup’
─ checking for LF line-endings in source and make files and shell scripts
─ checking for empty or unneeded directories
─ building ‘arrow_14.0.1.9000.tar.gz’
Installing package into '/home/jwj/R/x86_64-pc-linux-gnu-library/4.3'
(as 'lib' is unspecified)
* installing *source* package 'arrow' ...
** using staged installation
*** pkg-config found.
*** Latest available nightly for 14.0.1.9000: 14.0.1.100000226
*** Found local C++ source: 'tools/cpp'
*** Building libarrow from source
```
I'll look at converting the rsync invocation to R next week
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser merged PR #39148:
URL: https://github.com/apache/arrow/pull/39148
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "jonkeane (via GitHub)" <gi...@apache.org>.
jonkeane commented on PR #39148:
URL: https://github.com/apache/arrow/pull/39148#issuecomment-2015056351
Sorry this is post merge, I just saw it now while tracking something else down.
I see that there was already some discussion about how this is duplicative of the sync-cpp step in the make file and it seemed everyone agreed that having this in two places wasn't ideal: but is there a ticket to clean that up? I don't think I see anything linked here, but I might have missed something
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #39148:
URL: https://github.com/apache/arrow/pull/39148#discussion_r1424174402
##########
r/bootstrap.R:
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+if (!dir.exists('../cpp')) {
+ cli::cli_alert_warning("Arrow C++ sources not found, skipping bootstrap.")
+ q(save = "no")
Review Comment:
pkgbuild doesn't error for anything that happens in bootstrap.R...`configure` would be the correct place to ensure the tarball was built properly. (However, since we have build-ignored bootstrap.R, it will also never run when `../cpp` doesn't exist).
I do think that `q()` is anti-social here...the logical way most contributors would contribute to and interactively debug this is in RStudio and by `source()`ing it. The current script will close the RStudio window and nix all data in the user session without any indication as to why, which I think we should avoid unless absolutely necessary.
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "amoeba (via GitHub)" <gi...@apache.org>.
amoeba commented on code in PR #39148:
URL: https://github.com/apache/arrow/pull/39148#discussion_r1421218784
##########
r/bootstrap.R:
##########
@@ -0,0 +1,84 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+git_sc <- system2("git", c("rev-parse", "--is-inside-work-tree"), stdout = FALSE, stderr = FALSE)
+if (git_sc != 0) {
+ cli::cli_alert_warning("Not in a git repository, skipping bootstrap.R")
+ q(save = "no")
+}
+
+pkg_version <- package_version(desc::desc_get("Version"))
+is_release <- is.na(pkg_version[1, 4]) || pkg_version[1, 4] < "100"
+# for testing and CI
+force_bootstrap <- identical(tolower(Sys.getenv("ARROW_FORCE_BOOTSTRAP", default = "false")), "true")
+
+get_checksum_version <- function() {
+ url_exists <- function(url) {
+ headers <- curlGetHeaders(url)
+ attr(headers, "status") == 200
+ }
+ # Run update-checksums.R manually if the checksums already exists
+ # but have an unexpected version.
+ if (dir.exists("tools/checksums") || force_bootstrap) {
+ cli::cli_alert_success("Found existing checksums, skipping download.")
+ return(NA)
+ }
+
+ # We currently don't support prebuild binaries with a differing version.
+ jfrog_url <- "https://apache.jfrog.io/artifactory/arrow/r/%s/libarrow"
+ matching_url <- url_exists(sprintf(jfrog_url, pkg_version[1, 1:3]))
+ if (matching_url) {
+ version <- pkg_version[1, 1:3]
+ } else {
+ stop("No matching checksums found, can't proceed.")
+ }
+ version
+}
+
+if (!is_release) {
+ cli::cli_alert_warning("Skipping releases preparations for dev version.")
+ q(save = "no")
+}
+
+cli::cli_alert_info("Installing dependencies")
+devtools::install_dev_deps(".", upgrade = "never")
+devtools::install_cran("styler")
+
+cli::cli_alert_info("Removing badges from Readme.md")
+system2("sed", c("-i", "'/^<!--- badges: start -->$/,/^<!--- badges: stop -->$/d'", "README.md"))
Review Comment:
Ah, nice.
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on PR #39148:
URL: https://github.com/apache/arrow/pull/39148#issuecomment-2019216171
@jonkeane there is now #40787 ;)
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on code in PR #39148:
URL: https://github.com/apache/arrow/pull/39148#discussion_r1534325851
##########
r/bootstrap.R:
##########
@@ -0,0 +1,77 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+
+
+# exclude_patterns are regular expressions so use ^ to match top-level
+# directories only. Otherwise 'dir' will match 'src/dir' and 'dir/file'.
+rsync <- function(src_dir, dest_dir, exclude_patterns) {
+ all_files <- list.files(src_dir, recursive = TRUE)
+
+ # Filter out excluded patterns
+ for (pattern in exclude_patterns) {
+ all_files <- all_files[!grepl(pattern, all_files)]
+ }
+
+ files_to_vendor_src <- file.path(src_dir, all_files)
+ files_to_vendor_dst <- file.path(dest_dir, all_files)
+
+ # Recreate the directory structure
+ dst_dirs <- unique(dirname(files_to_vendor_dst))
+ for (dst_dir in dst_dirs) {
+ if (!dir.exists(dst_dir)) {
+ dir.create(dst_dir, recursive = TRUE)
+ }
+ }
+
+ # Copy the files
+ if (all(file.copy(files_to_vendor_src, files_to_vendor_dst, overwrite = TRUE))) {
+ cat("All files successfully copied to tools/cpp\n")
+ } else {
+ stop("Failed to vendor all files")
+ }
+}
+
+
+if (dir.exists("../cpp")) {
+ rsync("../cpp",
+ "tools/cpp",
+ exclude_patterns = c(
+ "^apidoc",
+ "^build$",
+ "^build-support/boost-",
+ "^examples",
+ "^submodules",
+ "^src/gandiva",
+ "^src/jni",
+ "^src/skyhook",
+ "_test\\.cc"
+ )
+ )
+
+ # Note: files in tools are available at build time, but not at run time. The thirdparty
+ # cmake expects .env, NOTICE.txt, and LICENSE.txt to be available one level up from cpp/
+ # we must rename .env to dotenv and then replace references to it in cpp/CMakeLists.txt
+ file.copy(from = c("../NOTICE.txt", to = "../LICENSE.txt"), "tools/", overwrite = TRUE)
+ file.copy("../.env", "tools/dotenv", overwrite = TRUE)
Review Comment:
The comment is taken from the Makefile, I think you are right with R CMD check
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on code in PR #39148:
URL: https://github.com/apache/arrow/pull/39148#discussion_r1533231821
##########
r/bootstrap.R:
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+if (!dir.exists('../cpp')) {
+ cli::cli_alert_warning("Arrow C++ sources not found, skipping bootstrap.")
+ q(save = "no")
Review Comment:
That's actually not true, when I use `devtools::local_install()` with cli_abort it errors (because '.' is copied to a tmp dir that doesn't have cpp)
```
Enter one or more numbers, or an empty line to skip updates: 3
Running bootstrap.R...
Error:
! Arrow C++ sources not found, skipping bootstrap.
Backtrace:
▆
1. └─cli::cli_abort("Arrow C++ sources not found, skipping bootstrap.")
2. └─rlang::abort(...)
Execution halted
Error: Failed to install 'arrow' from local:
! System command 'Rscript' failed
```
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #39148:
URL: https://github.com/apache/arrow/pull/39148#discussion_r1423020585
##########
r/bootstrap.R:
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+if (!dir.exists('../cpp')) {
+ cli::cli_alert_warning("Arrow C++ sources not found, skipping bootstrap.")
+ q(save = "no")
Review Comment:
I think using `q()` here might lead to surprising results if somebody tries to `source()` this file. (I know we do this in nixlibs.R, but in that file we care a lot about the return code and I don't think that matters here).
##########
r/bootstrap.R:
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+if (!dir.exists('../cpp')) {
+ cli::cli_alert_warning("Arrow C++ sources not found, skipping bootstrap.")
+ q(save = "no")
+}
+
+pkg_version <- package_version(desc::desc_get("Version"))
+is_release <- is.na(pkg_version[1, 4]) || pkg_version[1, 4] < "100"
+# for testing and CI
+force_bootstrap <- identical(tolower(Sys.getenv("ARROW_FORCE_BOOTSTRAP", unset = "false")), "true")
+
+get_checksum_version <- function() {
+ url_exists <- function(url) {
+ headers <- curlGetHeaders(url)
+ attr(headers, "status") == 200
+ }
+ # Run update-checksums.R manually if the checksums already exists
+ # but have an unexpected version.
+ if (dir.exists("tools/checksums") || force_bootstrap) {
+ cli::cli_alert_success("Found existing checksums, skipping download.")
+ return(NA)
+ }
+
+ # We currently don't support prebuild binaries with a differing version.
+ jfrog_url <- "https://apache.jfrog.io/artifactory/arrow/r/%s/libarrow"
+ matching_url <- url_exists(sprintf(jfrog_url, pkg_version[1, 1:3]))
+ if (matching_url) {
+ version <- pkg_version[1, 1:3]
+ } else {
+ cli::cli_abort("No matching checksums found, can't proceed.")
+ }
+ version
+}
+
+cli::cli_alert_info("Vendoring C++ sources")
+system2("make", c("sync-cpp"))
Review Comment:
I think this will need to avoid `system()` to be useful: on Windows there's no guarantee that RTools is on PATH and I don't think there's any guarantee that `rsync` is installed anywhere. That said, `rsync` is probably faster.
The recursive copy written in R I use is here: https://github.com/apache/arrow-adbc/blob/main/r/adbcsnowflake/bootstrap.R#L18-L64
##########
r/bootstrap.R:
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+if (!dir.exists('../cpp')) {
+ cli::cli_alert_warning("Arrow C++ sources not found, skipping bootstrap.")
+ q(save = "no")
Review Comment:
Maybe `cli_abort()`?
##########
r/bootstrap.R:
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+if (!dir.exists('../cpp')) {
+ cli::cli_alert_warning("Arrow C++ sources not found, skipping bootstrap.")
+ q(save = "no")
+}
+
+pkg_version <- package_version(desc::desc_get("Version"))
+is_release <- is.na(pkg_version[1, 4]) || pkg_version[1, 4] < "100"
+# for testing and CI
+force_bootstrap <- identical(tolower(Sys.getenv("ARROW_FORCE_BOOTSTRAP", unset = "false")), "true")
+
+get_checksum_version <- function() {
+ url_exists <- function(url) {
+ headers <- curlGetHeaders(url)
+ attr(headers, "status") == 200
+ }
+ # Run update-checksums.R manually if the checksums already exists
+ # but have an unexpected version.
+ if (dir.exists("tools/checksums") || force_bootstrap) {
+ cli::cli_alert_success("Found existing checksums, skipping download.")
+ return(NA)
+ }
+
+ # We currently don't support prebuild binaries with a differing version.
+ jfrog_url <- "https://apache.jfrog.io/artifactory/arrow/r/%s/libarrow"
+ matching_url <- url_exists(sprintf(jfrog_url, pkg_version[1, 1:3]))
+ if (matching_url) {
+ version <- pkg_version[1, 1:3]
+ } else {
+ cli::cli_abort("No matching checksums found, can't proceed.")
+ }
+ version
+}
+
+cli::cli_alert_info("Vendoring C++ sources")
+system2("make", c("sync-cpp"))
+
+if (!is_release && !force_bootstrap) {
+ cli::cli_alert_warning("Skipping releases preparations for dev version.")
+ q(save = "no")
+}
+
+cli::cli_alert_info("Installing dependencies")
+devtools::install_dev_deps(".", upgrade = "never")
+devtools::install_cran("styler")
+
+cli::cli_alert_info("Removing badges from Readme.md")
+system2("sed", c("-i", "-e", "'/^<!--- badges: start -->$/,/^<!--- badges: stop -->$/d'", "README.md"))
+
+cli::cli_alert_info("Running urlchecker")
+url_res <- urlchecker::url_check()
+print(url_res)
+
+if (nrow(url_res) > 0 && !force_bootstrap) {
+ cli::cli_abort("Broken URLs found, can't proceed.")
+}
+
+VERSION <- get_checksum_version()
+if (!is.na(VERSION)) {
+ cli::cli_alert_info("Updating checksums for libarrow version: {VERSION}")
+ source("tools/update-checksums.R", local = TRUE)
+}
+
+cli::cli_alert_info("Building documentation")
+system2("make", c("doc"))
Review Comment:
Can this be `devtools::document()`?
##########
r/bootstrap.R:
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+if (!dir.exists('../cpp')) {
+ cli::cli_alert_warning("Arrow C++ sources not found, skipping bootstrap.")
+ q(save = "no")
+}
+
+pkg_version <- package_version(desc::desc_get("Version"))
+is_release <- is.na(pkg_version[1, 4]) || pkg_version[1, 4] < "100"
+# for testing and CI
+force_bootstrap <- identical(tolower(Sys.getenv("ARROW_FORCE_BOOTSTRAP", unset = "false")), "true")
+
+get_checksum_version <- function() {
+ url_exists <- function(url) {
+ headers <- curlGetHeaders(url)
+ attr(headers, "status") == 200
+ }
+ # Run update-checksums.R manually if the checksums already exists
+ # but have an unexpected version.
+ if (dir.exists("tools/checksums") || force_bootstrap) {
+ cli::cli_alert_success("Found existing checksums, skipping download.")
+ return(NA)
+ }
+
+ # We currently don't support prebuild binaries with a differing version.
+ jfrog_url <- "https://apache.jfrog.io/artifactory/arrow/r/%s/libarrow"
+ matching_url <- url_exists(sprintf(jfrog_url, pkg_version[1, 1:3]))
+ if (matching_url) {
+ version <- pkg_version[1, 1:3]
+ } else {
+ cli::cli_abort("No matching checksums found, can't proceed.")
+ }
+ version
+}
+
+cli::cli_alert_info("Vendoring C++ sources")
+system2("make", c("sync-cpp"))
+
+if (!is_release && !force_bootstrap) {
+ cli::cli_alert_warning("Skipping releases preparations for dev version.")
+ q(save = "no")
Review Comment:
Again, I think we want `stop()` or `cli_abort()`
##########
r/bootstrap.R:
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+if (!dir.exists('../cpp')) {
+ cli::cli_alert_warning("Arrow C++ sources not found, skipping bootstrap.")
+ q(save = "no")
+}
+
+pkg_version <- package_version(desc::desc_get("Version"))
+is_release <- is.na(pkg_version[1, 4]) || pkg_version[1, 4] < "100"
+# for testing and CI
+force_bootstrap <- identical(tolower(Sys.getenv("ARROW_FORCE_BOOTSTRAP", unset = "false")), "true")
+
+get_checksum_version <- function() {
+ url_exists <- function(url) {
+ headers <- curlGetHeaders(url)
+ attr(headers, "status") == 200
+ }
+ # Run update-checksums.R manually if the checksums already exists
+ # but have an unexpected version.
+ if (dir.exists("tools/checksums") || force_bootstrap) {
+ cli::cli_alert_success("Found existing checksums, skipping download.")
+ return(NA)
+ }
+
+ # We currently don't support prebuild binaries with a differing version.
+ jfrog_url <- "https://apache.jfrog.io/artifactory/arrow/r/%s/libarrow"
+ matching_url <- url_exists(sprintf(jfrog_url, pkg_version[1, 1:3]))
+ if (matching_url) {
+ version <- pkg_version[1, 1:3]
+ } else {
+ cli::cli_abort("No matching checksums found, can't proceed.")
+ }
+ version
+}
+
+cli::cli_alert_info("Vendoring C++ sources")
+system2("make", c("sync-cpp"))
+
+if (!is_release && !force_bootstrap) {
+ cli::cli_alert_warning("Skipping releases preparations for dev version.")
+ q(save = "no")
+}
+
+cli::cli_alert_info("Installing dependencies")
+devtools::install_dev_deps(".", upgrade = "never")
+devtools::install_cran("styler")
+
+cli::cli_alert_info("Removing badges from Readme.md")
+system2("sed", c("-i", "-e", "'/^<!--- badges: start -->$/,/^<!--- badges: stop -->$/d'", "README.md"))
Review Comment:
This is done prior to creating a release branch by the version bumping script, no?
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on PR #39148:
URL: https://github.com/apache/arrow/pull/39148#issuecomment-1850978389
> I do wonder if having both the makefile and the Rified release steps are both useful or if it's a little confusing to have them both
Moving everything outside of `sync-cpp` into a separate release script allows us to keep everything else in the make file and keep only that part duplicated. We can of course also transition to using the bootstrap.R version exclusivly after giving it some time to prove itself?
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on PR #39148:
URL: https://github.com/apache/arrow/pull/39148#issuecomment-1852276579
> We can of course also transition to using the bootstrap.R version exclusivly after giving it some time to prove itself?
I think that's a good idea...continue to use `R CMD build` and slowly transition to `devtools::build()` in CI and then for actual releases.
My high-level comment of having `bash` calling `R` calling `bash` and/or `R` remains...I think it's find to have `bash`/Makefile OR R automate some of the release things, but those should all be in the same place (and avoid too many back-and-forth indirections between the two).
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on PR #39148:
URL: https://github.com/apache/arrow/pull/39148#issuecomment-1848610895
> Ah yes that sounds useful, so we want to change it to always vendor the cpp source
Yes! bootstrap.R is the *only* file where this can happen that's guaranteed to run during `pkgbuild::build()`. Moving that logic there is the only way to ensure that installation from developer/package install tools that use `pkgbuild::build()` will get the same tarball as the one we would release to CRAN.
> only do the other release stuff on release version/opt-in.
I would say that automating all those steps is a good thing, but bootstrap.R is not the file to do it. These steps aren't things that you want to fail during package installation via `pak::pak()`, for example...you really just want to do them on a release branch and then commit the result.
> why install a release version from gh instead of CRAN?
This mostly matters before we have released it to CRAN (allows, for example, that I or somebody could quickly install the about-to-be-released tarball to ensure that it fixed some problem).
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on code in PR #39148:
URL: https://github.com/apache/arrow/pull/39148#discussion_r1534345136
##########
r/bootstrap.R:
##########
@@ -0,0 +1,77 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+
+
+# exclude_patterns are regular expressions so use ^ to match top-level
+# directories only. Otherwise 'dir' will match 'src/dir' and 'dir/file'.
+rsync <- function(src_dir, dest_dir, exclude_patterns) {
+ all_files <- list.files(src_dir, recursive = TRUE)
+
+ # Filter out excluded patterns
+ for (pattern in exclude_patterns) {
+ all_files <- all_files[!grepl(pattern, all_files)]
+ }
+
+ files_to_vendor_src <- file.path(src_dir, all_files)
+ files_to_vendor_dst <- file.path(dest_dir, all_files)
+
+ # Recreate the directory structure
+ dst_dirs <- unique(dirname(files_to_vendor_dst))
+ for (dst_dir in dst_dirs) {
+ if (!dir.exists(dst_dir)) {
+ dir.create(dst_dir, recursive = TRUE)
+ }
+ }
+
+ # Copy the files
+ if (all(file.copy(files_to_vendor_src, files_to_vendor_dst, overwrite = TRUE))) {
+ cat("All files successfully copied to tools/cpp\n")
+ } else {
+ stop("Failed to vendor all files")
+ }
+}
+
+
+if (dir.exists("../cpp")) {
+ rsync("../cpp",
+ "tools/cpp",
+ exclude_patterns = c(
+ "^apidoc",
+ "^build$",
+ "^build-support/boost-",
+ "^examples",
+ "^submodules",
+ "^src/gandiva",
+ "^src/jni",
+ "^src/skyhook",
+ "_test\\.cc"
+ )
+ )
+
+ # Note: files in tools are available at build time, but not at run time. The thirdparty
+ # cmake expects .env, NOTICE.txt, and LICENSE.txt to be available one level up from cpp/
+ # we must rename .env to dotenv and then replace references to it in cpp/CMakeLists.txt
+ file.copy(from = c("../NOTICE.txt", to = "../LICENSE.txt"), "tools/", overwrite = TRUE)
+ file.copy("../.env", "tools/dotenv", overwrite = TRUE)
Review Comment:
```
N checking for hidden files and directories ...
Found the following hidden files and directories:
tools/.env
These were most likely included in error. See section ‘Package
structure’ in the ‘Writing R Extensions’ manual.
```
yep
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on code in PR #39148:
URL: https://github.com/apache/arrow/pull/39148#discussion_r1421215167
##########
r/bootstrap.R:
##########
@@ -0,0 +1,84 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+git_sc <- system2("git", c("rev-parse", "--is-inside-work-tree"), stdout = FALSE, stderr = FALSE)
+if (git_sc != 0) {
+ cli::cli_alert_warning("Not in a git repository, skipping bootstrap.R")
+ q(save = "no")
+}
+
+pkg_version <- package_version(desc::desc_get("Version"))
+is_release <- is.na(pkg_version[1, 4]) || pkg_version[1, 4] < "100"
+# for testing and CI
+force_bootstrap <- identical(tolower(Sys.getenv("ARROW_FORCE_BOOTSTRAP", default = "false")), "true")
+
+get_checksum_version <- function() {
+ url_exists <- function(url) {
+ headers <- curlGetHeaders(url)
+ attr(headers, "status") == 200
+ }
+ # Run update-checksums.R manually if the checksums already exists
+ # but have an unexpected version.
+ if (dir.exists("tools/checksums") || force_bootstrap) {
+ cli::cli_alert_success("Found existing checksums, skipping download.")
+ return(NA)
+ }
+
+ # We currently don't support prebuild binaries with a differing version.
+ jfrog_url <- "https://apache.jfrog.io/artifactory/arrow/r/%s/libarrow"
+ matching_url <- url_exists(sprintf(jfrog_url, pkg_version[1, 1:3]))
+ if (matching_url) {
+ version <- pkg_version[1, 1:3]
+ } else {
+ stop("No matching checksums found, can't proceed.")
Review Comment:
```suggestion
cli::cli_abort("No matching checksums found, can't proceed.")
```
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #39148:
URL: https://github.com/apache/arrow/pull/39148#issuecomment-1848024381
Revision: b9c839cdca2e8516a530dbb8d7e8c40ac2be34c1
Submitted crossbow builds: [ursacomputing/crossbow @ actions-c75677e771](https://github.com/ursacomputing/crossbow/branches/all?query=actions-c75677e771)
|Task|Status|
|----|------|
|r-binary-packages|[](https://github.com/ursacomputing/crossbow/actions/runs/7147938293/job/19468279734)|
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on code in PR #39148:
URL: https://github.com/apache/arrow/pull/39148#discussion_r1421216266
##########
r/bootstrap.R:
##########
@@ -0,0 +1,84 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+git_sc <- system2("git", c("rev-parse", "--is-inside-work-tree"), stdout = FALSE, stderr = FALSE)
+if (git_sc != 0) {
+ cli::cli_alert_warning("Not in a git repository, skipping bootstrap.R")
+ q(save = "no")
+}
+
+pkg_version <- package_version(desc::desc_get("Version"))
+is_release <- is.na(pkg_version[1, 4]) || pkg_version[1, 4] < "100"
+# for testing and CI
+force_bootstrap <- identical(tolower(Sys.getenv("ARROW_FORCE_BOOTSTRAP", default = "false")), "true")
+
+get_checksum_version <- function() {
+ url_exists <- function(url) {
+ headers <- curlGetHeaders(url)
+ attr(headers, "status") == 200
+ }
+ # Run update-checksums.R manually if the checksums already exists
+ # but have an unexpected version.
+ if (dir.exists("tools/checksums") || force_bootstrap) {
+ cli::cli_alert_success("Found existing checksums, skipping download.")
+ return(NA)
+ }
+
+ # We currently don't support prebuild binaries with a differing version.
+ jfrog_url <- "https://apache.jfrog.io/artifactory/arrow/r/%s/libarrow"
+ matching_url <- url_exists(sprintf(jfrog_url, pkg_version[1, 1:3]))
+ if (matching_url) {
+ version <- pkg_version[1, 1:3]
+ } else {
+ stop("No matching checksums found, can't proceed.")
+ }
+ version
+}
+
+if (!is_release) {
+ cli::cli_alert_warning("Skipping releases preparations for dev version.")
+ q(save = "no")
+}
+
+cli::cli_alert_info("Installing dependencies")
+devtools::install_dev_deps(".", upgrade = "never")
+devtools::install_cran("styler")
+
+cli::cli_alert_info("Removing badges from Readme.md")
+system2("sed", c("-i", "'/^<!--- badges: start -->$/,/^<!--- badges: stop -->$/d'", "README.md"))
Review Comment:
for gnu sed `-e` is optional so it's no problem to add it :+1:
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on code in PR #39148:
URL: https://github.com/apache/arrow/pull/39148#discussion_r1421215328
##########
r/bootstrap.R:
##########
@@ -0,0 +1,84 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+git_sc <- system2("git", c("rev-parse", "--is-inside-work-tree"), stdout = FALSE, stderr = FALSE)
+if (git_sc != 0) {
+ cli::cli_alert_warning("Not in a git repository, skipping bootstrap.R")
+ q(save = "no")
+}
+
+pkg_version <- package_version(desc::desc_get("Version"))
+is_release <- is.na(pkg_version[1, 4]) || pkg_version[1, 4] < "100"
+# for testing and CI
+force_bootstrap <- identical(tolower(Sys.getenv("ARROW_FORCE_BOOTSTRAP", default = "false")), "true")
+
+get_checksum_version <- function() {
+ url_exists <- function(url) {
+ headers <- curlGetHeaders(url)
+ attr(headers, "status") == 200
+ }
+ # Run update-checksums.R manually if the checksums already exists
+ # but have an unexpected version.
+ if (dir.exists("tools/checksums") || force_bootstrap) {
+ cli::cli_alert_success("Found existing checksums, skipping download.")
+ return(NA)
+ }
+
+ # We currently don't support prebuild binaries with a differing version.
+ jfrog_url <- "https://apache.jfrog.io/artifactory/arrow/r/%s/libarrow"
+ matching_url <- url_exists(sprintf(jfrog_url, pkg_version[1, 1:3]))
+ if (matching_url) {
+ version <- pkg_version[1, 1:3]
+ } else {
+ stop("No matching checksums found, can't proceed.")
+ }
+ version
+}
+
+if (!is_release) {
+ cli::cli_alert_warning("Skipping releases preparations for dev version.")
+ q(save = "no")
+}
+
+cli::cli_alert_info("Installing dependencies")
+devtools::install_dev_deps(".", upgrade = "never")
+devtools::install_cran("styler")
+
+cli::cli_alert_info("Removing badges from Readme.md")
+system2("sed", c("-i", "'/^<!--- badges: start -->$/,/^<!--- badges: stop -->$/d'", "README.md"))
Review Comment:
the only change is the `-e` flag right?
--
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-39147: [R] Add Bootstrap.r [arrow]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #39148:
URL: https://github.com/apache/arrow/pull/39148#discussion_r1424178761
##########
r/bootstrap.R:
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+if (!dir.exists('../cpp')) {
+ cli::cli_alert_warning("Arrow C++ sources not found, skipping bootstrap.")
+ q(save = "no")
+}
+
+pkg_version <- package_version(desc::desc_get("Version"))
+is_release <- is.na(pkg_version[1, 4]) || pkg_version[1, 4] < "100"
+# for testing and CI
+force_bootstrap <- identical(tolower(Sys.getenv("ARROW_FORCE_BOOTSTRAP", unset = "false")), "true")
+
+get_checksum_version <- function() {
+ url_exists <- function(url) {
+ headers <- curlGetHeaders(url)
+ attr(headers, "status") == 200
+ }
+ # Run update-checksums.R manually if the checksums already exists
+ # but have an unexpected version.
+ if (dir.exists("tools/checksums") || force_bootstrap) {
+ cli::cli_alert_success("Found existing checksums, skipping download.")
+ return(NA)
+ }
+
+ # We currently don't support prebuild binaries with a differing version.
+ jfrog_url <- "https://apache.jfrog.io/artifactory/arrow/r/%s/libarrow"
+ matching_url <- url_exists(sprintf(jfrog_url, pkg_version[1, 1:3]))
+ if (matching_url) {
+ version <- pkg_version[1, 1:3]
+ } else {
+ cli::cli_abort("No matching checksums found, can't proceed.")
+ }
+ version
+}
+
+cli::cli_alert_info("Vendoring C++ sources")
+system2("make", c("sync-cpp"))
+
+if (!is_release && !force_bootstrap) {
+ cli::cli_alert_warning("Skipping releases preparations for dev version.")
+ q(save = "no")
+}
+
+cli::cli_alert_info("Installing dependencies")
+devtools::install_dev_deps(".", upgrade = "never")
+devtools::install_cran("styler")
+
+cli::cli_alert_info("Removing badges from Readme.md")
+system2("sed", c("-i", "-e", "'/^<!--- badges: start -->$/,/^<!--- badges: stop -->$/d'", "README.md"))
+
+cli::cli_alert_info("Running urlchecker")
+url_res <- urlchecker::url_check()
+print(url_res)
+
+if (nrow(url_res) > 0 && !force_bootstrap) {
+ cli::cli_abort("Broken URLs found, can't proceed.")
+}
+
+VERSION <- get_checksum_version()
+if (!is.na(VERSION)) {
+ cli::cli_alert_info("Updating checksums for libarrow version: {VERSION}")
+ source("tools/update-checksums.R", local = TRUE)
+}
+
+cli::cli_alert_info("Building documentation")
+system2("make", c("doc"))
Review Comment:
I think my general point here is that it's fine for any of this to be in `bash` OR in `R`, but the situation where we have bash calling R calling bash calling R (i.e., `Rscript tools/release_stuff.R` calling `make doc` calling `R -e 'devtools::document()'`) is not a workflow we should be adding or encouraging.
--
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