nealrichardson commented on code in PR #37684: URL: https://github.com/apache/arrow/pull/37684#discussion_r1340536025
########## r/tools/nixlibs-allowlist.txt: ########## @@ -2,3 +2,4 @@ ubuntu centos redhat rhel +darwin Review Comment: > I understand the concerns and would agree with @nealrichardson that contacting CRAN and getting official approval is the best (if risky) way forward. With the bundled builds it should only be a matter of setting the feature flags to enable s3 and gcs for macos, everything else is covered by the default settings iirc. > Correct; do you want to do that here or in a followup? This would also require some documentation changes to explain the different behavior between macOS and linux. Or would we only want to make this change if CRAN rejects the binary downloading? > With regards to discussions with CRAN: another positive to this new system in addition to the f/reliability aspects is the improved archive-ability: currently without autobrew available the src package can not be build on mac as we do not ship the package with the local formulas we use in the nightly builds to build from apache/arrow@main. With these changes that will be possible. > Technically this is solvable with the current build system--we do for our nightly builds. > For linux an argument pro pre-compiled binary would be build/check times as arrow currently takes ~20x the checktime that dplyr takes and that is already on the longer side. So that would save cran a lot of resources and we could collaborate with them in someway to make sure one of the checks builds from source if they want that (that could just be setting `LIBARROW_BINARY=false`). > I guess this is up to @thisisnic as the maintainer, but if it were me, I'd mention the possibility of doing Linux binary downloading but not necessarily couple it to the macOS change. There are what, 3, 4 macOS environments we have to build on? So many more bespoke machines to deal with for Linux, and while I think we've done all of the relevant checking to download a binary that will work, I wouldn't rule out that BDR has an evil machine somewhere that would break it, and I wouldn't want to have to fix that under time pressure. FWIW I don't think we'd need to tell them to set an env var to test the full source build. We already don't use the prebuilt binaries with `libc++` (https://github.com/apache/arrow/pull/37684/files/6a040aa4c55702999945b8e87c4bf572b2f7ec81#diff-935746c34b16289a07b0d9bf7642dbd268b18059b6187f7cdec7c464be47a3deR249) so that job would test the full source build. I'd stress this in the communication with CRAN: we don't blindly download binaries everywhere, only on systems that are compatible with the libraries we've built; for the rest, we build from source. > For windows we are currently already hosting the binaries but are still using rwinlib primarily with our binaries as a fallback, we will need to switch that around for this release as well. I do want to extend the windows part of the build system to match linux and macos so that we can do source builds within the regular R package mechanisms but that won't be ready for the next release but could be mentioned if CRAN asks about windows. +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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
