samtools / htslib

C library for high-throughput sequencing data formats
Other
812 stars 445 forks source link

hfile_libcurl plugin uses private htslib APIs #623

Open jmarshall opened 7 years ago

jmarshall commented 7 years ago

Since PR #600, hfile_libcurl uses the JSON tokeniser, which is private HTSlib functionality declared in _htsinternal.h. This is a problem when hfile_libcurl is built as a plugin.

Plugins should not use private HTSlib functions (apart from those in _hfileinternal.h for hfile plugins, obviously) as they are not tightly bound to particular htslib versions — this is part of the point of being a plugin!

As alluded to in the 39ca08917a1ed4b94ed6c2aae4a2038f26edb193 commit message, the JSON tokeniser should be finalised as a public API before it is used in plugins.

jkbonfield commented 7 years ago

Are we making a rod for our own back by having curl as a plugin? Agreed we should finalise the json tokeniser, but not being able to use it in our own code seems like beating yourself around the head with a bat.

Maybe I'm missing the reason for having curl as a plugin.

jmarshall commented 7 years ago

Part of the reason for inventing plugins was so that exotic dependencies would be isolated. Seeing as everybody still tends to link statically against libhts.a.

So in this case: so that every program in the world didn't have to suddenly start linking with -lcurl.

daviesrob commented 7 years ago

The EBI have recently made their reference server TLS only. If CRAM users want to download references they now have to use libcurl.

While I don't like adding dependencies, it's likely we will need to make this one compulsory. It's the only way we can support TLS, and the world is moving in that direction.