nealrichardson commented on code in PR #44989:
URL: https://github.com/apache/arrow/pull/44989#discussion_r1896942141
##########
ci/scripts/PKGBUILD:
##########
@@ -119,7 +117,9 @@ build() {
-DCMAKE_BUILD_TYPE="release" \
-DCMAKE_INSTALL_PREFIX=${MINGW_PREFIX} \
-DCMAKE_UNITY_BUILD=OFF \
- -DCMAKE_VERBOSE_MAKEFILE=ON
+ -DCMAKE_VERBOSE_MAKEFILE=ON \
+ -DAWSSDK_SOURCE=BUNDLED \
Review Comment:
Move `-DAWSSDK_SOURCE=BUNDLED \` up so we stay sorted?
##########
r/configure.win:
##########
@@ -61,10 +61,8 @@ function configure_binaries() {
OPENSSL_LIBS="-lcrypto -lcrypt32"
MIMALLOC_LIBS="-lbcrypt -lpsapi"
BROTLI_LIBS="-lbrotlienc -lbrotlidec -lbrotlicommon" # Common goes last
since dec and enc depend on it
- AWS_LIBS="-laws-cpp-sdk-config -laws-cpp-sdk-transfer
-laws-cpp-sdk-identity-management \
- -laws-cpp-sdk-cognito-identity -laws-cpp-sdk-sts -laws-cpp-sdk-s3 \
- -laws-cpp-sdk-core -laws-c-event-stream -laws-checksums
-laws-c-common \
- -luserenv -lversion -lws2_32 -lbcrypt -lwininet -lwinhttp"
+ # AWS specific libs for Windows are bundled
Review Comment:
```suggestion
# We build aws-sdk-cpp bundled now, so the AWS libs are included in
arrow_bundled_dependencies
# but we also need to include these Windows system libraries
```
##########
ci/scripts/PKGBUILD:
##########
@@ -119,7 +117,9 @@ build() {
-DCMAKE_BUILD_TYPE="release" \
-DCMAKE_INSTALL_PREFIX=${MINGW_PREFIX} \
-DCMAKE_UNITY_BUILD=OFF \
- -DCMAKE_VERBOSE_MAKEFILE=ON
+ -DCMAKE_VERBOSE_MAKEFILE=ON \
+ -DAWSSDK_SOURCE=BUNDLED \
+ -DARROW_USE_CCACHE=OFF
Review Comment:
Is this needed or just part of debugging? If it's important, maybe leave a
comment explaining why?
##########
ci/scripts/PKGBUILD:
##########
@@ -83,7 +81,7 @@ build() {
# segfaults in tests
MSYS2_ARG_CONV_EXCL="-DCMAKE_INSTALL_PREFIX=" \
Review Comment:
This `"-DCMAKE_INSTALL_PREFIX="` looks odd up here--why would it go before
the call to cmake itself? And we also have
`-DCMAKE_INSTALL_PREFIX=${MINGW_PREFIX}` below. (I know y'all didn't touch it
in this PR, just observing.)
##########
ci/scripts/PKGBUILD:
##########
@@ -83,7 +81,7 @@ build() {
# segfaults in tests
MSYS2_ARG_CONV_EXCL="-DCMAKE_INSTALL_PREFIX=" \
- ${MINGW_PREFIX}/bin/cmake.exe \
+ "${PROGRAMFILES}\CMake\bin\cmake.exe" \
Review Comment:
I feel like this deserves a comment explaining where this comes from, and/or
passing in path-to-cmake via env var or something so that it's explicitly set.
##########
ci/scripts/r_windows_build.sh:
##########
@@ -25,11 +25,21 @@ export ARROW_HOME="$(cd "${ARROW_HOME}" && pwd)"
# Uncomment L38-41 if you're testing a new rtools dependency that hasn't yet
sync'd to CRAN
# curl
https://raw.githubusercontent.com/r-windows/rtools-packages/master/pacman.conf
> /etc/pacman.conf
+# cp /etc/pacman.conf /etc/pacman.conf.bak
+# curl
https://raw.githubusercontent.com/r-windows/rtools-packages/master/pacman.conf
> /etc/pacman.conf
+# cat /etc/pacman.conf
# curl -OSsl
"http://repo.msys2.org/msys/x86_64/msys2-keyring-r21.b39fb11-1-any.pkg.tar.xz";
# pacman -U --noconfirm msys2-keyring-r21.b39fb11-1-any.pkg.tar.xz && rm
msys2-keyring-r21.b39fb11-1-any.pkg.tar.xz
# pacman --noconfirm -Scc
+# pacman --noconfirm -Syy
+# pacman --noconfirm -S ${MINGW_PACKAGE_PREFIX}-cmake
+
+#Try reverting to the original pacman.conf
+# cp /etc/pacman.conf.bak /etc/pacman.conf
+# cat /etc/pacman.conf
Review Comment:
As discussed, this comment is no longer valid because the rtools pacman
setup is no longer maintained.
```suggestion
```
--
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]
