You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "hiroyuki-sato (via GitHub)" <gi...@apache.org> on 2025/06/03 00:03:31 UTC
[PR] GH-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
hiroyuki-sato opened a new pull request, #46682:
URL: https://github.com/apache/arrow/pull/46682
### Rationale for this change
This PR fixes the bug introduced in #46527.
### What changes are included in this PR?
If the environment variable `INSTALL_ARGS` doesn't set,
the R command will execute,
`CMD INSTALL arrow*tar.gz`.
otherwise if `INSTALL_ARGS="--with-keep.source --no-multiarch"`
`CMD INSTALL --with-keep.source --no-multiarch arrow*tar.gz`
### Are these changes tested?
Yes.
### Are there any user-facing changes?
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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #46682:
URL: https://github.com/apache/arrow/pull/46682#issuecomment-2933746023
Revision: c8a2a1cc6558b7c4b68ebb9fe605cdec12a9f79a
Submitted crossbow builds: [ursacomputing/crossbow @ actions-0d4aee9e28](https://github.com/ursacomputing/crossbow/branches/all?query=actions-0d4aee9e28)
|Task|Status|
|----|------|
|preview-docs|[](https://github.com/ursacomputing/crossbow/actions/runs/15410385301/job/43360967971)|
--
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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "hiroyuki-sato (via GitHub)" <gi...@apache.org>.
hiroyuki-sato commented on code in PR #46682:
URL: https://github.com/apache/arrow/pull/46682#discussion_r2123780086
##########
ci/scripts/r_build.sh:
##########
@@ -24,6 +24,12 @@ build_dir=${2}
: "${BUILD_DOCS_R:=OFF}"
+if [ -z "${INSTALL_ARGS}" ] ; then
+ R_INSTALL_ARGS=()
+else
+ read -r -a R_INSTALL_ARGS <<< "$INSTALL_ARGS"
+fi
Review Comment:
Thank you for your reply. Changed to `for` syntax.
I just wanted to avoid unnecessary modification accidentally.
I'm now quoting many variables for fixing SC2086.
I'll compare shellcheck carefully for avoid unnecessary quote.
`r_buld.sh` already passed shellcheck. So, I may don't need care.
--
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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "hiroyuki-sato (via GitHub)" <gi...@apache.org>.
hiroyuki-sato commented on PR #46682:
URL: https://github.com/apache/arrow/pull/46682#issuecomment-2933741213
@github-actions crossbow submit preview-docs
--
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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #46682:
URL: https://github.com/apache/arrow/pull/46682#issuecomment-2932943519
:warning: GitHub issue #46673 **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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "hiroyuki-sato (via GitHub)" <gi...@apache.org>.
hiroyuki-sato commented on code in PR #46682:
URL: https://github.com/apache/arrow/pull/46682#discussion_r2122466717
##########
ci/scripts/r_build.sh:
##########
@@ -24,6 +24,12 @@ build_dir=${2}
: "${BUILD_DOCS_R:=OFF}"
+if [ -z "${INSTALL_ARGS}" ] ; then
+ R_INSTALL_ARGS=()
+else
+ read -r -a R_INSTALL_ARGS <<< "$INSTALL_ARGS"
+fi
Review Comment:
`for arg in ${INSTALL_ARGS};` violates `SC2153`.
```
for arg in ${INSTALL_ARGS}; do
^-------------^ SC2153 (info): Possible misspelling: INSTALL_ARGS may not be assigned. Did you mean R_INSTALL_ARGS?
For more information:
https://www.shellcheck.net/wiki/SC2153 -- Possible misspelling: INSTALL_ARG...
```
--
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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "hiroyuki-sato (via GitHub)" <gi...@apache.org>.
hiroyuki-sato commented on code in PR #46682:
URL: https://github.com/apache/arrow/pull/46682#discussion_r2123091098
##########
ci/scripts/r_build.sh:
##########
@@ -24,6 +24,12 @@ build_dir=${2}
: "${BUILD_DOCS_R:=OFF}"
+if [ -z "${INSTALL_ARGS}" ] ; then
+ R_INSTALL_ARGS=()
+else
+ read -r -a R_INSTALL_ARGS <<< "$INSTALL_ARGS"
+fi
Review Comment:
Thanks! I don't know hot to do it with `for` and `+=` with SC2086 safe. So, I added the comment.
--
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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "hiroyuki-sato (via GitHub)" <gi...@apache.org>.
hiroyuki-sato commented on PR #46682:
URL: https://github.com/apache/arrow/pull/46682#issuecomment-2934071916
Thanks! Updated PR description.
--
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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #46682:
URL: https://github.com/apache/arrow/pull/46682#issuecomment-2933845113
Could you update "What changes are included in this PR?" in the PR description?
The current content should be for "Rationale for this change" not "What changes are included in this PR?".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #46682:
URL: https://github.com/apache/arrow/pull/46682#issuecomment-2933041983
```
Unable to match any tasks for `debian-docs`
The Archery job run can be found at: https://github.com/apache/arrow/actions/runs/15406293591
```
--
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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "hiroyuki-sato (via GitHub)" <gi...@apache.org>.
hiroyuki-sato commented on code in PR #46682:
URL: https://github.com/apache/arrow/pull/46682#discussion_r2122466717
##########
ci/scripts/r_build.sh:
##########
@@ -24,6 +24,12 @@ build_dir=${2}
: "${BUILD_DOCS_R:=OFF}"
+if [ -z "${INSTALL_ARGS}" ] ; then
+ R_INSTALL_ARGS=()
+else
+ read -r -a R_INSTALL_ARGS <<< "$INSTALL_ARGS"
+fi
Review Comment:
`for arg in ${INSTALL_ARGS};` violates `SC2153`.
```
for arg in ${INSTALL_ARGS}; do
^-------------^ SC2153 (info): Possible misspelling: INSTALL_ARGS may not be assigned. Did you mean R_INSTALL_ARGS?
For more information:
https://www.shellcheck.net/wiki/SC2153 -- Possible misspelling: INSTALL_ARG...
```
`for arg in "${INSTALL_ARGS}"` also violates `SC2066`.
```
for arg in "${INSTALL_ARGS}"; do
^---------------^ SC2066 (error): Since you double quoted this, it will not word split, and the loop will only run once.
^-------------^ SC2153 (info): Possible misspelling: INSTALL_ARGS may not be assigned. Did you mean R_INSTALL_ARGS?
```
--
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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #46682:
URL: https://github.com/apache/arrow/pull/46682#discussion_r2123707635
##########
ci/scripts/r_build.sh:
##########
@@ -24,6 +24,12 @@ build_dir=${2}
: "${BUILD_DOCS_R:=OFF}"
+if [ -z "${INSTALL_ARGS}" ] ; then
+ R_INSTALL_ARGS=()
+else
+ read -r -a R_INSTALL_ARGS <<< "$INSTALL_ARGS"
+fi
Review Comment:
> This version doesn't violate shellcheck on my machine.
> But, I want to quote `${INSTALL_ARGS}` part.
> It seems violates SC2086(need quoting).
> (I don't know why this syntax passed shellcheck, by the way)
> However It doesn't work well.
If ShellCheck doesn't report this usage as an error, there is not a problem. Why do you want to quote this case too?
BTW, we don't need `: "${INSTALL_ARGS:-}"`. We can do it in `for`: `for arg in ${INSTALL_ARGS:-}; do`
If we want to care glob here, we can disable glob for `INSTALL_ARGS="--X * --Y"` by `set -f`:
```bash
R_INSTALL_ARGS=()
set -f
for arg in ${INSTALL_ARGS:-}; do
R_INSTALL_ARGS+=("${arg}")
done
set +f
```
But it's also unfamiliar syntax. If we want to use `set -f` too, there is no advantage than `read`.
--
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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "hiroyuki-sato (via GitHub)" <gi...@apache.org>.
hiroyuki-sato commented on PR #46682:
URL: https://github.com/apache/arrow/pull/46682#issuecomment-2935373552
@github-actions crossbow submit preview-docs
--
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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #46682:
URL: https://github.com/apache/arrow/pull/46682#issuecomment-2933049278
Revision: 224c19b2f940210e0a40d505783f7accca2533be
Submitted crossbow builds: [ursacomputing/crossbow @ actions-29b57edaf2](https://github.com/ursacomputing/crossbow/branches/all?query=actions-29b57edaf2)
|Task|Status|
|----|------|
|preview-docs|[](https://github.com/ursacomputing/crossbow/actions/runs/15406362558/job/43349749527)|
--
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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "hiroyuki-sato (via GitHub)" <gi...@apache.org>.
hiroyuki-sato commented on PR #46682:
URL: https://github.com/apache/arrow/pull/46682#issuecomment-2933046119
@github-actions crossbow submit preview-docs
--
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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #46682:
URL: https://github.com/apache/arrow/pull/46682#discussion_r2123120654
##########
ci/scripts/r_build.sh:
##########
@@ -24,6 +24,12 @@ build_dir=${2}
: "${BUILD_DOCS_R:=OFF}"
+if [ -z "${INSTALL_ARGS}" ] ; then
+ R_INSTALL_ARGS=()
+else
+ read -r -a R_INSTALL_ARGS <<< "$INSTALL_ARGS"
+fi
Review Comment:
SC2086? https://www.shellcheck.net/wiki/SC2083 ?
Could you share the full lint error?
--
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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #46682:
URL: https://github.com/apache/arrow/pull/46682#issuecomment-2935386473
Revision: fe6b479739c4706f0df90d34dcd5fcef08db9f72
Submitted crossbow builds: [ursacomputing/crossbow @ actions-699fc50655](https://github.com/ursacomputing/crossbow/branches/all?query=actions-699fc50655)
|Task|Status|
|----|------|
|preview-docs|[](https://github.com/ursacomputing/crossbow/actions/runs/15419331372/job/43389912841)|
--
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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "hiroyuki-sato (via GitHub)" <gi...@apache.org>.
hiroyuki-sato commented on PR #46682:
URL: https://github.com/apache/arrow/pull/46682#issuecomment-2933040768
@github-actions crossbow submit debian-docs
--
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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "hiroyuki-sato (via GitHub)" <gi...@apache.org>.
hiroyuki-sato commented on code in PR #46682:
URL: https://github.com/apache/arrow/pull/46682#discussion_r2122870646
##########
ci/scripts/r_build.sh:
##########
@@ -24,6 +24,12 @@ build_dir=${2}
: "${BUILD_DOCS_R:=OFF}"
+if [ -z "${INSTALL_ARGS}" ] ; then
+ R_INSTALL_ARGS=()
+else
+ read -r -a R_INSTALL_ARGS <<< "$INSTALL_ARGS"
+fi
Review Comment:
Thanks! Thanks! I did simplified `read -r -a R_INSTALL_ARGS <<< "${INSTALL_ARGS:-}"`.
--
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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "hiroyuki-sato (via GitHub)" <gi...@apache.org>.
hiroyuki-sato commented on code in PR #46682:
URL: https://github.com/apache/arrow/pull/46682#discussion_r2122466717
##########
ci/scripts/r_build.sh:
##########
@@ -24,6 +24,12 @@ build_dir=${2}
: "${BUILD_DOCS_R:=OFF}"
+if [ -z "${INSTALL_ARGS}" ] ; then
+ R_INSTALL_ARGS=()
+else
+ read -r -a R_INSTALL_ARGS <<< "$INSTALL_ARGS"
+fi
Review Comment:
`for arg in ${INSTALL_ARGS};` violates `SC2153`.
```
for arg in ${INSTALL_ARGS}; do
^-------------^ SC2153 (info): Possible misspelling: INSTALL_ARGS may not be assigned. Did you mean R_INSTALL_ARGS?
For more information:
https://www.shellcheck.net/wiki/SC2153 -- Possible misspelling: INSTALL_ARG...
```
Do we disable SC2153 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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #46682:
URL: https://github.com/apache/arrow/pull/46682#discussion_r2122684697
##########
ci/scripts/r_build.sh:
##########
@@ -24,6 +24,12 @@ build_dir=${2}
: "${BUILD_DOCS_R:=OFF}"
+if [ -z "${INSTALL_ARGS}" ] ; then
+ R_INSTALL_ARGS=()
+else
+ read -r -a R_INSTALL_ARGS <<< "$INSTALL_ARGS"
+fi
Review Comment:
Can we use `${INSTALL_ARGS:-}`?
--
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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #46682:
URL: https://github.com/apache/arrow/pull/46682#discussion_r2123120654
##########
ci/scripts/r_build.sh:
##########
@@ -24,6 +24,12 @@ build_dir=${2}
: "${BUILD_DOCS_R:=OFF}"
+if [ -z "${INSTALL_ARGS}" ] ; then
+ R_INSTALL_ARGS=()
+else
+ read -r -a R_INSTALL_ARGS <<< "$INSTALL_ARGS"
+fi
Review Comment:
SC2086? https://www.shellcheck.net/wiki/SC2086 ?
Could you share the full lint error?
--
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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "hiroyuki-sato (via GitHub)" <gi...@apache.org>.
hiroyuki-sato commented on code in PR #46682:
URL: https://github.com/apache/arrow/pull/46682#discussion_r2123247616
##########
ci/scripts/r_build.sh:
##########
@@ -24,6 +24,12 @@ build_dir=${2}
: "${BUILD_DOCS_R:=OFF}"
+if [ -z "${INSTALL_ARGS}" ] ; then
+ R_INSTALL_ARGS=()
+else
+ read -r -a R_INSTALL_ARGS <<< "$INSTALL_ARGS"
+fi
Review Comment:
Thank you for your comment.
Suggested change violates SC2153.
```bash
#!/bin/bash
R_INSTALL_ARGS=()
for arg in ${INSTALL_ARGS}; do
R_INSTALL_ARGS+=("${arg}")
done
```
```
for arg in ${INSTALL_ARGS}; do
^-------------^ SC2153 (info): Possible misspelling: INSTALL_ARGS may not be assigned. Did you mean R_INSTALL_ARGS?
```
SC2153 can fix like this.
```bash
: "${INSTALL_ARGS:-}"
R_INSTALL_ARGS=()
for arg in ${INSTALL_ARGS}; do
R_INSTALL_ARGS+=("${arg}")
done
```
This version doesn't violate shellcheck on my machine.
But, I want to quote `${INSTALL_ARGS}` part.
It seems violates SC2086(need quoting).
(I don't know why this syntax passed shellcheck, by the way)
However It doesn't work well.
So, If we use this plan, I think the comment may help.
```bash
: "${INSTALL_ARGS:-}"
R_INSTALL_ARGS=()
# Do not Quote ${INSTALL_ARGS} like "${INSTALL_ARGS}"
for arg in ${INSTALL_ARGS}; do
R_INSTALL_ARGS+=("${arg}")
done
```
I don't know how to fix `${INSTALL_ARGS}` part SC2086 safe.
SC2086 meant about the original code.
Before #46663, The original message was
```
In ci/scripts/r_build.sh line 45:
${R_BIN} CMD INSTALL ${INSTALL_ARGS} arrow*.tar.gz
^------^ SC2086 (info): Double quote to prevent globbing and word splitting.
^-------------^ SC2086 (info): Double quote to prevent globbing and word splitting.
```
`read -r -a R_INSTALL_ARGS <<< "$INSTALL_ARGS"` has unfamiliar syntax, but it is SC2086 safe.
I support your decision. Should we change `for` syntax?
--
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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #46682:
URL: https://github.com/apache/arrow/pull/46682#discussion_r2122936969
##########
ci/scripts/r_build.sh:
##########
@@ -24,6 +24,12 @@ build_dir=${2}
: "${BUILD_DOCS_R:=OFF}"
+if [ -z "${INSTALL_ARGS}" ] ; then
+ R_INSTALL_ARGS=()
+else
+ read -r -a R_INSTALL_ARGS <<< "$INSTALL_ARGS"
+fi
Review Comment:
If you want to use `read -r -a <<<`, we must add a comment why this words. Because it's not a straightforward way for us. If we use straitforword `for` + `+=`, we don't need a comment.
--
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-46673: [CI][R][Docs] Set empty INSTALL_ARGS options as default [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #46682:
URL: https://github.com/apache/arrow/pull/46682#discussion_r2122435556
##########
ci/scripts/r_build.sh:
##########
@@ -24,6 +24,12 @@ build_dir=${2}
: "${BUILD_DOCS_R:=OFF}"
+if [ -z "${INSTALL_ARGS}" ] ; then
Review Comment:
```suggestion
if [ -z "${INSTALL_ARGS}" ]; then
```
##########
ci/scripts/r_build.sh:
##########
@@ -24,6 +24,12 @@ build_dir=${2}
: "${BUILD_DOCS_R:=OFF}"
+if [ -z "${INSTALL_ARGS}" ] ; then
+ R_INSTALL_ARGS=()
+else
+ read -r -a R_INSTALL_ARGS <<< "$INSTALL_ARGS"
+fi
Review Comment:
Can we simplify this?
```suggestion
R_INSTALL_ARGS=()
for arg in ${INSTALL_ARGS}; do
R_INSTALL_ARGS+=("${arg}")
done
```
--
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-46673: [CI][R][Docs] Accept empty INSTALL_ARGS again [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #46682:
URL: https://github.com/apache/arrow/pull/46682#issuecomment-2939927771
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 35e8136622672b7677a5dbb6056e8c10d28115d2.
There were 69 benchmark results with an error:
- Commit Run on `amd64-m5-4xlarge-linux` at [2025-06-03 23:17:46Z](https://conbench.ursa.dev/compare/runs/df8e408251ba4778b7b71e362700bf89...7c78c35678614f2f866ec23600849cba/)
- [`tpch` (R) with engine=arrow, format=parquet, language=R, memory_map=False, query_id=TPCH-04, scale_factor=10](https://conbench.ursa.dev/benchmark-results/0683fb968f7b747780009d15b12c0a91)
- [`tpch` (R) with engine=arrow, format=native, language=R, memory_map=False, query_id=TPCH-12, scale_factor=1](https://conbench.ursa.dev/benchmark-results/0683fd3eafc97b7380004b25df7956ba)
- and 67 more (see the report linked below)
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/43464461427) has more details. It also includes information about 44 possible false positives for unstable benchmarks that are known to sometimes produce them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46673: [CI][R][Docs] Accept empty INSTALL_ARGS again [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou merged PR #46682:
URL: https://github.com/apache/arrow/pull/46682
--
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