Added some simple introspection functions.#1170
Conversation
e002d17 to
0b15461
Compare
|
Apologies, but I'll need someone with a Mac to do the final fixing of the Makefile for this. I have no remote login permission for our work macs (as they stupidly all require you to be sat infront of it, duh!). For reference, line 337 of Makefile is currently: := is a GNU extension. The POSIX syntax is ::= (https://www.gnu.org/software/make/manual/html_node/Flavors.html#Flavors) which is what I started with, but that doesn't work either on Macs. Maybe this just isn't possible on whatever make system Apple provides? |
It's because the system make installed by Apple is based on GNU Make 3.81 — for their usual anti-GPLv3-jihad reasons — and that pre-dates the invention of the POSIX form of There is precedent for this sort of thing with plugin.o in config.mk.in so you could follow what that does. However in Makefile I'd probably suggest generating it to a C source file that you then This would be trivial if configure is run, so this somewhat requires a decision on whether to finally remove non-configure builds… |
|
Without expressing an opinion on whether all this is desirable: hts_os.c/.h are for papering over platform-specific deficiencies in wacky ways, so are the wrong place for this. (Essentially no third party code that doesn't care about being portable to weird platforms should ever need to You should probably just commit the trivial |
|
I don't think the |
|
Oops yes I forgot about my need to hack hts_lrand48. I was going to put that in a separate commit! It'll have no ABI issues IMO as it's only changing on broken systems where it's already broken anyway. (Edit: and how many years has it been with no one even noticing it? Clearly this is never exercised which gives us freedom to simply fix it.) I did look at how plugin.o was defined, and indeed copied that syntax from there. The difference though is the requirement for := instead of = to avoid a recursively defined variable. I can't include I did also think where best to put this. Initially I was going to put them in hts.c, but then saw the statement at the top that it is "format-neutral I/O, indexing, and iterator API functions" which this introspection work clearly is not. No other file matched the bill bar the OS specific stuff, which arguably this is - it's build specific anyway. IMO it's a good home. I'd agree that maybe we should put the function declarations somewhere else though. |
A lot would be trivial if we could cull the split-brain personality of "make or no-make". I've been arguing this for years so +1 from me. :-) |
4250945 to
753a505
Compare
|
Removed the lrand fix, squashed and rebased. NB this won't compile until after #1171 is merged, after which we can rebase off develop again. |
These permit testing of explicit features, e.g.
if (htslib_test_feature(HTS_FEATURE_PLUGINS)) {
...
}
or capturing the entire feature bit-field via htslib_features().
(This function may be redundant so perhaps is a candidate for
culling?).
This also permits querying compilation details: CC, CFLAGS, LDFLAGS
and CPPFLAGS.
Finally there is a htslib_feature_string() function which is mainly
for verbose feature printing rather than programmatic parsing, but it
conveniently also permits us to get features from a library simply by
running "strings" on the binary.
Note this doesn't actually do any testing so it's not in the Makefile.
It's hard to know quite what testing it could do given by design the
output would differ based on how the user built it. We could maybe
call it to make sure it doesn't crash, but that's not likely to be
something we need regression testing on.
753a505 to
0804ee5
Compare
Some of it needs to be there anyway for the scheme and plugin APIs.
15deadf to
43644dc
Compare
43644dc to
52fbca3
Compare
|
I had a number of further comments about the proposed API here. Also the PR had not been revised to take advantage of #1187 which was applied so that one of the Makefile hacks in this could be simplified. I realise that I should have made those comments over the New Year break. Still, it would be appreciated if some warning could be given when long-untouched PRs are about to be merged. |
These permit testing of explicit features, e.g.
or capturing the entire feature bit-field via htslib_features().
(This function may be redundant so perhaps is a candidate for culling?).
This also permits querying compilation details: CC, CFLAGS, LDFLAGS and CPPFLAGS.
Finally there is a htslib_feature_string() function which is mainly for verbose feature printing rather than programmatic parsing, but it conveniently also permits us to get features from a library simply by running "strings" on the binary.
If accepted, I can work on an equivalent samtools command to report its own data along with the htslib it links against. How should we access this?
samtools versionmaybe for more detail than the minimal version data in the usage statement?