pitrou commented on a change in pull request #11714: URL: https://github.com/apache/arrow/pull/11714#discussion_r756197117
########## File path: ci/scripts/r_windows_build.sh ########## @@ -87,7 +87,7 @@ if [ -d mingw64/lib/ ]; then # These may be from https://dl.bintray.com/rtools/backports/ cp $MSYS_LIB_DIR/mingw64/lib/lib{thrift,snappy}.a $DST_DIR/${RWINLIB_LIB_DIR}/x64 # These are from https://dl.bintray.com/rtools/mingw{32,64}/ - cp $MSYS_LIB_DIR/mingw64/lib/lib{zstd,lz4,crypto,utf8proc,re2,aws*}.a $DST_DIR/lib/x64 + cp $MSYS_LIB_DIR/mingw64/lib/lib{zstd,lz4,brotlicommon,brotlidec,brotlienc,crypto,utf8proc,re2,aws*}.a $DST_DIR/lib/x64 Review comment: Perhaps simply `brotli*` here and below? ########## File path: r/configure.win ########## @@ -52,7 +53,7 @@ function configure_release() { # NOTE: If you make changes to the libraries below, you should also change # ci/scripts/r_windows_build.sh and ci/scripts/PKGBUILD PKG_CFLAGS="-I${RWINLIB}/include -DARROW_STATIC -DPARQUET_STATIC -DARROW_DS_STATIC -DARROW_R_WITH_ARROW -DARROW_R_WITH_PARQUET -DARROW_R_WITH_DATASET -DARROW_R_WITH_JSON" - PKG_LIBS="-L${RWINLIB}/lib"'$(subst gcc,,$(COMPILED_BY))$(R_ARCH) '"-L${RWINLIB}/lib"'$(R_ARCH)$(CRT) '"-lparquet -larrow_dataset -larrow -larrow_bundled_dependencies -lutf8proc -lthrift -lsnappy -lz -lzstd -llz4 -lole32 ${MIMALLOC_LIBS} ${OPENSSL_LIBS}" + PKG_LIBS="-L${RWINLIB}/lib"'$(subst gcc,,$(COMPILED_BY))$(R_ARCH) '"-L${RWINLIB}/lib"'$(R_ARCH)$(CRT) '"-lparquet -larrow_dataset -larrow -larrow_bundled_dependencies -lutf8proc -lthrift -lsnappy -lz -lzstd -llz4 ${BROTLI_LIBS} -lole32 ${MIMALLOC_LIBS} ${OPENSSL_LIBS}" Review comment: While we are at it, can this easily (and the `PKG_CFLAGS` variable above) be wrapped over multiple lines for better readability? ########## File path: r/tests/testthat/test-compressed.R ########## @@ -27,6 +27,7 @@ if (identical(Sys.getenv("APPVEYOR"), "True")) { test_that("Compression codecs are included in the Windows build", { Review comment: We don't have any R builds on AppVeyor, do we? The condition should be that the test is running on Windows instead. -- 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]
