GH-37945: [R] Update developer documentation #38220
Conversation
|
|
github-actions
bot
added
Component: R
awaiting committer review
r/vignettes/developers/setup.Rmd
Outdated
| You'll need to create a `libarrow` directory inside the R package directory and | ||
| unzip the zip file containing the compiled libarrow binary files into it. |
There was a problem hiding this comment.
Doesn't this happen automatically now? I thought that's where we were going when we removed the brew stuff?
There was a problem hiding this comment.
It's not quite there yet and I'm not sure we have the capacity to get it there before 14.0.0.
There was a problem hiding this comment.
If we don't think we will be ready with the nightly grabbing, we should roll back the removal of using brew for this purpose, IMO. We can still use the ASF hosted binaries for release, but like I've said before: if we are worried about committee bandwidth we shouldn't do something that shrinks the possible pool of contributors.
Can you remind me of the ticket for doing the nightly grabbing? I still haven't been able to find it.
There was a problem hiding this comment.
Ah, we can use ARROW_DOWNLOADED_BINARIES on mac and linux and RWINLIB_LOCAL on windows to point to the downloaded zip. The script will then unzip into r/libarrow.
With the other changes taking a while to validate on ci I might be able to finish the latestnightly stuff before the release, otherwise shortly after (which would also be fine imo as it only applies to a dev setup)
There was a problem hiding this comment.
Do you have a ticket already? I would be happy to write one if not. Perhaps we could work on this in parallel to get it over the line quicker. But I don't want to step on something that already exists / is already being worked
There was a problem hiding this comment.
I think this existing one is applicable: #27839
I have the logic done, I just want to refactor *libs.R a bit as they are overlapping more and more and I don't want to keep copying stuff across ^^ I should be able to get it done today... I will open a PR with my changes regardless of how far I am later today so you can test/review/add to it :)
There was a problem hiding this comment.
#38080 is a bit of a kludge, but perhaps worth revisiting as a stopgap if the server-side fix is hard. I'm also not opposed to putting the brew clauses back in (and I'm happy to do a local check). @jonkeane I would be so excited if you took on any of that (but let me know and I can put it on the TODO for tomorrow).
There was a problem hiding this comment.
In any case I'll wait until the best practice we can come up with prior to the 14.0.0 release and document that here.
There was a problem hiding this comment.
Here is the PR with the latest nightly search + some bonus refactoring ^^)#38236
There was a problem hiding this comment.
I've updated the PR to document the intended behaviour after #38236.
github-actions
bot
added
awaiting changes
assignUser
changed the title
|
|
paleolimbot
force-pushed
the
r-dev-docs
branch
from
fb72b12
to
c751317
Compare
github-actions
bot
added
awaiting change review
|
@github-actions crossbow submit test-r-devdocs |
|
Revision: 824bcde Submitted crossbow builds: ursacomputing/crossbow @ actions-858c104cea
|
github-actions
bot
added
awaiting changes
Co-authored-by: Jacob Wujciak-Jens <[email protected]>
github-actions
bot
added
awaiting change review
r/vignettes/developers/setup.Rmd
Outdated
| and do a "clean" rebuild. You can do this from a terminal with | ||
| `R CMD INSTALL . --preclean` or from RStudio using the "Clean and Install" | ||
| option from "Build" tab. |
There was a problem hiding this comment.
There's also a clean target in the Makefile — though I can appreciate wanting to give folks a single way to do this.
github-actions
bot
added
awaiting changes
github-actions
bot
added
awaiting change review
|
If nobody objects, I'll merge this after my kids go to bed (~5 hours), or early tomorrow morning if they require cuddling all night (10% chance). |
### Rationale for this change Several PRs over the last few months have update the build system to be more friendly for developers. During this process it has also come to light that we haven't supported the Windows development setup documented here since R 4.1 (released in spring 2021). I had to remove Windows from the test-r-devdocs job because the approach used there was not compatible with the `setup-r@ v2` action, and the job was failing with the `@ v1` action. ### What changes are included in this PR? - Updated the sections on using pre-built static libraries and bundled builds - Removed the Windows section regarding the bundled build. This section would need rewriting to support the last two minor releases of R but in the meantime I think it is mostly confusing. ### Are these changes tested? They are documentation changes. They are also slightly optimisitc: we can fix problems with the developer setup incrementally between releases, but it's more difficult to update our documentation. This PR documents the intended behaviour after #38236 . ### Are there any user-facing changes? No. * Closes: #37945 Lead-authored-by: Dewey Dunnington <[email protected]> Co-authored-by: Dewey Dunnington <[email protected]> Co-authored-by: Jacob Wujciak-Jens <[email protected]> Signed-off-by: Dewey Dunnington <[email protected]>
|
After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 0574535. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 14 possible false positives for unstable benchmarks that are known to sometimes produce them. |
### Rationale for this change Several PRs over the last few months have update the build system to be more friendly for developers. During this process it has also come to light that we haven't supported the Windows development setup documented here since R 4.1 (released in spring 2021). I had to remove Windows from the test-r-devdocs job because the approach used there was not compatible with the `setup-r@ v2` action, and the job was failing with the `@ v1` action. ### What changes are included in this PR? - Updated the sections on using pre-built static libraries and bundled builds - Removed the Windows section regarding the bundled build. This section would need rewriting to support the last two minor releases of R but in the meantime I think it is mostly confusing. ### Are these changes tested? They are documentation changes. They are also slightly optimisitc: we can fix problems with the developer setup incrementally between releases, but it's more difficult to update our documentation. This PR documents the intended behaviour after apache#38236 . ### Are there any user-facing changes? No. * Closes: apache#37945 Lead-authored-by: Dewey Dunnington <[email protected]> Co-authored-by: Dewey Dunnington <[email protected]> Co-authored-by: Jacob Wujciak-Jens <[email protected]> Signed-off-by: Dewey Dunnington <[email protected]>
### Rationale for this change Several PRs over the last few months have update the build system to be more friendly for developers. During this process it has also come to light that we haven't supported the Windows development setup documented here since R 4.1 (released in spring 2021). I had to remove Windows from the test-r-devdocs job because the approach used there was not compatible with the `setup-r@ v2` action, and the job was failing with the `@ v1` action. ### What changes are included in this PR? - Updated the sections on using pre-built static libraries and bundled builds - Removed the Windows section regarding the bundled build. This section would need rewriting to support the last two minor releases of R but in the meantime I think it is mostly confusing. ### Are these changes tested? They are documentation changes. They are also slightly optimisitc: we can fix problems with the developer setup incrementally between releases, but it's more difficult to update our documentation. This PR documents the intended behaviour after apache#38236 . ### Are there any user-facing changes? No. * Closes: apache#37945 Lead-authored-by: Dewey Dunnington <[email protected]> Co-authored-by: Dewey Dunnington <[email protected]> Co-authored-by: Jacob Wujciak-Jens <[email protected]> Signed-off-by: Dewey Dunnington <[email protected]>
### Rationale for this change Several PRs over the last few months have update the build system to be more friendly for developers. During this process it has also come to light that we haven't supported the Windows development setup documented here since R 4.1 (released in spring 2021). I had to remove Windows from the test-r-devdocs job because the approach used there was not compatible with the `setup-r@ v2` action, and the job was failing with the `@ v1` action. ### What changes are included in this PR? - Updated the sections on using pre-built static libraries and bundled builds - Removed the Windows section regarding the bundled build. This section would need rewriting to support the last two minor releases of R but in the meantime I think it is mostly confusing. ### Are these changes tested? They are documentation changes. They are also slightly optimisitc: we can fix problems with the developer setup incrementally between releases, but it's more difficult to update our documentation. This PR documents the intended behaviour after apache#38236 . ### Are there any user-facing changes? No. * Closes: apache#37945 Lead-authored-by: Dewey Dunnington <[email protected]> Co-authored-by: Dewey Dunnington <[email protected]> Co-authored-by: Jacob Wujciak-Jens <[email protected]> Signed-off-by: Dewey Dunnington <[email protected]>
Rationale for this change
Several PRs over the last few months have update the build system to be more friendly for developers. During this process it has also come to light that we haven't supported the Windows development setup documented here since R 4.1 (released in spring 2021). I had to remove Windows from the test-r-devdocs job because the approach used there was not compatible with the
setup-r@v2action, and the job was failing with the@v1action.What changes are included in this PR?
Are these changes tested?
They are documentation changes. They are also slightly optimisitc: we can fix problems with the developer setup incrementally between releases, but it's more difficult to update our documentation. This PR documents the intended behaviour after #38236 .
Are there any user-facing changes?
No.