htslib
htslib copied to clipboard
samtools
Prefix exported symbols
Hi,
In Debian we recently had the problem, that a tool had a function called json clashing with a value from an enum defined in hts.h. Already, most functions are prefixed with hts_, please consider adding that to other symbols, too. Especially, since json, sam, etc. are very generic names which may well be used in other contexts.
Best, Fabian
json is on the way out, but not immediately as it will change HTSlib's API to do so. We'll get rid of it next time we have a more major API/ABI breaking release.
C++ has enum class to avoid these kinds of problems. But I am looking forward to the prefixed symbols in the next version.
HTSlib is a C library so it doesn't really matter what C++ has :smile:
For some reason I thought enumeration constants were in a separate name space (so did not need much prefixing — unlike HTSlib's external functions) and this was not picked up during review when format detection was added in 2014. Bugger. Most of htsExactFormat and htsCompression are less than ideal…
I am updating PR #721 at present and am planning to add new entries as bzip2_compression, fastq_format, etc rather than plain bzip2 and fastq.
Does this seem like a sufficient and good convention? Will there ever be a good time to change the existing constants?
Adding more characters definitely helps to prevent a collision.
Will there ever be a good time to change the existing constants?
I fear the answer to that will always be no.
Would it be better to use an hts prefix, resulting in hts_fmt_sam, hts_compress_gzip etc.?
Will there ever be a good time to change the existing constants?
While it doesn't solve the immediate problem, we could add new symbols and deprecate the old ones (as was done for json). We could also possibly remove json as it's been deprecated for a while now...
Would it be better to use an hts prefix, resulting in hts_fmt_sam, hts_compress_gzip etc.?
As usual, it's a tradeoff. It depends how people feel about the hideous ugliness, etc…
Release 1.10 is a major step forward in terms of symbol tables. All internal functions are no longer exported from the library.
However this is mainly a FYI as the primary point of this issue hasn't been resolved. We heavily changed the ABI in this release but kept API breaking things to a minimum hence "json" is still in the htsExactFormat enum.
We can't really change this without having an impact on code that uses these enums, so I think it will have to wait until we're doing broader API sweeping changes (which could be years away).
