-
Notifications
You must be signed in to change notification settings - Fork 126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable GCS, S3, and libdeflate support for bcftools #1019
Conversation
…configure before compiling
bcftools/1.20/Dockerfile
Outdated
liblzma-dev \ | ||
libcurl4-gnutls-dev \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these actually need to be -dev versions with headers in the runtime container? Do plugins somehow compile/link against them at runtime or can they just be liblzma libcurl4-gnutls
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about compilation & linking, but I think the correct syntax is liblzma5
for Ubuntu Jammy: https://packages.ubuntu.com/search?suite=jammy§ion=all&arch=any&keywords=liblzma&searchon=names
and libcurl4
: https://packages.ubuntu.com/search?suite=jammy§ion=all&arch=any&keywords=libcurl4&searchon=names
I believe when the samtools/bcftools/htslib dockerfiles were written we were following these instructions: https://github.com/samtools/samtools/blob/972c1889942a4f07d8f62e93330f723da919c271/INSTALL#L220
Could you please mark this PR as draft? The dockerfile doesn't build successfully yet (according to the GH Actions log) and I think it will require some edits prior to review from our team. We would love to have additional tests for these features built into the dockerfile, preferably in the And my last thought - it may be good to also update the samtools and htslib dockerfiles as well as I imagine they are also missing these features (I have not checked though, don't quote me). Can be done as part of this PR or separately. |
…d publicly in GCS.
…dependencies at runtime. Update htslib to configure before building and link against libdeflate.
@pettyalex Thank you for raising this issue and making a pull request. GCS/S3 and libdeflate support are important features that we missed while building version 1.20. So, I will request a few changes from you:
Any further tests, recommendations, and feedback will be appreciated.
|
Thank you for the feedback! About Libcurl3 is ABI compatible with libcurl4, so the name of the compiled library has not been incremented. That means that libcurl3-gnutls is the correct runtime library for libcurl4-gnutls-dev, and if you look in the libcurl4-gnutls-dev package it indeed contains libcurl3-gnutls |
@pettyalex Thank you very much for the changes. This looks great! I need one minor change as you see in the checklist. You will need to add Before:
After:
|
@pettyalex Thank you for your contribution! |
Enable AWS S3, GCS, and libdeflate support for bcftools by running ./configure before compiling
This fixes #1018
If you want to merge this, I don't see a way to mark another build number for an already published package, but I'd be glad to update that if it exists.
I'd also be glad to add tests that test reading from AWS S3 or GCS storage directly to validate that these features are working.
Pull Request (PR) checklist:
docker build --tag samtools:1.15test --target test docker-builds/samtools/1.15
)spades/3.12.0/Dockerfile
)shigatyper/2.0.1/test.sh
)spades/3.12.0/README.md
)