[GitHub] [arrow] wesm commented on pull request #7556: ARROW-9188: [C++] Use Brotli shared libraries if they are available

2020-06-30 Thread GitBox


wesm commented on pull request #7556:
URL: https://github.com/apache/arrow/pull/7556#issuecomment-652058759


   If there's no further feedback I will merge this in the next 24h and I 
assume that any packaging issues will come up in nightlies as we push toward 
the next release.



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [arrow] wesm commented on pull request #7556: ARROW-9188: [C++] Use Brotli shared libraries if they are available

2020-06-30 Thread GitBox


wesm commented on pull request #7556:
URL: https://github.com/apache/arrow/pull/7556#issuecomment-651847404


   This looks good to me, @kszucs @kou @nealrichardson anything else you would 
want to 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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [arrow] wesm commented on pull request #7556: ARROW-9188: [C++] Use Brotli shared libraries if they are available

2020-06-29 Thread GitBox


wesm commented on pull request #7556:
URL: https://github.com/apache/arrow/pull/7556#issuecomment-651466752


   @github-actions crossbow submit -g linux -g wheel -g conda



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [arrow] wesm commented on pull request #7556: ARROW-9188: [C++] Use Brotli shared libraries if they are available

2020-06-29 Thread GitBox


wesm commented on pull request #7556:
URL: https://github.com/apache/arrow/pull/7556#issuecomment-651466442


   Actually, that's crazy. I'm taking the same approach as ZSTD and adding a 
CMake toggle between shared and static Brotli (with default being shared)



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [arrow] wesm commented on pull request #7556: ARROW-9188: [C++] Use Brotli shared libraries if they are available

2020-06-29 Thread GitBox


wesm commented on pull request #7556:
URL: https://github.com/apache/arrow/pull/7556#issuecomment-651464683


   Apparently the `-DBUILD_SHARED_LIBS=OFF` option for Brotli doesn't do 
anything. I'll add some code to scrub the shared libs from the manylinux images



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [arrow] wesm commented on pull request #7556: ARROW-9188: [C++] Use Brotli shared libraries if they are available

2020-06-29 Thread GitBox


wesm commented on pull request #7556:
URL: https://github.com/apache/arrow/pull/7556#issuecomment-651186700


   If you're installing Brotli in any of the packaging setups, there may be a 
scenario where there is both the shared AND static library -- in that case 
there would be an issue. We need to see why the manylinux* builds have 
`libbrotli*.so` in them -- if for some reason Brotli's build is insubordinate 
and churlish we might have to add a flag to do a hard switch between 
shared/static like we do with ZSTD 



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [arrow] wesm commented on pull request #7556: ARROW-9188: [C++] Use Brotli shared libraries if they are available

2020-06-27 Thread GitBox


wesm commented on pull request #7556:
URL: https://github.com/apache/arrow/pull/7556#issuecomment-650648681


   @nealrichardson I figure this might impact the R packages also



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [arrow] wesm commented on pull request #7556: ARROW-9188: [C++] Use Brotli shared libraries if they are available

2020-06-27 Thread GitBox


wesm commented on pull request #7556:
URL: https://github.com/apache/arrow/pull/7556#issuecomment-650633720


   It appears that the Brotli shared libraries are in the manylinux1 image even 
though `-DBUILD_SHARED_LIBS=OFF` 
   
   
https://github.com/apache/arrow/blob/master/python/manylinux1/scripts/build_brotli.sh#L29



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [arrow] wesm commented on pull request #7556: ARROW-9188: [C++] Use Brotli shared libraries if they are available

2020-06-27 Thread GitBox


wesm commented on pull request #7556:
URL: https://github.com/apache/arrow/pull/7556#issuecomment-650627218


   @github-actions crossbow submit -g linux -g wheel -g conda



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [arrow] wesm commented on pull request #7556: ARROW-9188: [C++] Use Brotli shared libraries if they are available

2020-06-27 Thread GitBox


wesm commented on pull request #7556:
URL: https://github.com/apache/arrow/pull/7556#issuecomment-650618145


   Thanks, will look into this. I'm guessing these changes will break some of 
the Python wheel builds so we may need a flag to indicate a preference of 
shared vs static



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org