Skip to content
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.

Sign up for GitHub

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

Jump to bottom

formerly caught bgzf_idx_amend_last symbol #1722

Closed
emollier opened this issue Dec 15, 2023 · 3 comments
Closed

formerly caught bgzf_idx_amend_last symbol #1722

emollier opened this issue Dec 15, 2023 · 3 comments
Assignees
@daviesrob

Comments

@emollier
Copy link
Contributor

Greetings,

I'm trying to bump the htslib suite to version 1.19 in Debian sid, and symbols tracking caught the drop of the bgzf_idx_amend_last function. As far as I understand, this function was not supposed to be part of the ABI in the first place, given its declaration in hts_internal.h. Besides, running the test suites of the many reverse dependencies, without rebuilding them first, does not raise any ill effects (except perhaps cyvcf2, which seems to have a test item failing to write VCF4.1 files properly, but as far as I can see, it is unrelated to the missing symbol). So the function does not look to have made it to the ABI by accident to consumers of the htslib.

Just checking, may I go ahead with the missing symbol without soversion bump, or was the drop actually unintended?

Have a nice day, :)
Étienne.
@emollier
Copy link
Contributor Author

Nevermind the parenthesis about cyvcf2 test failure, there is work in progress on their side to get the code compatible with htslib 1.19; see brentp/cyvcf2#290.

@daviesrob
Copy link
Member

Yes, it was dropped deliberately in PR #1672, although it looks like I forgot to remove the prototype from hts_internal.h. It's not part or the ABI, and should not have been exported in the .so file (at least after we tidied up the exports in #946 for release 1.10).

Since PR #1560, htslib has symbol versioning via htslib.map, which should make tracking ABI changes a bit easier.

@emollier
Copy link
Contributor Author

Hi Rob,
Thank you for the clarification, that makes sense to me.
Have a nice day, :)
Étienne.

@daviesrob daviesrob self-assigned this Jan 2, 2024
jmarshall added a commit to jmarshall/htslib that referenced this issue Jul 16, 2024
@jmarshall
Remove last traces of previously deleted bgzf_idx_amend_last()
96af274 
As noted in samtools#1722, this function was removed in PR samtools#1672.
@jmarshall jmarshall mentioned this issue Jul 16, 2024
Merged
daviesrob pushed a commit that referenced this issue Jul 17, 2024
@jmarshall
Miscellaneous minor build infrastructure and code fixes (PR #1807)
db2b449 
* Ignore and clean test/*/FAIL* for six subdirectories

These files can appear in base_mods, fastq, mpileup, and sam_filter
as well as faidx and tabix.

* Fix comment header to use `@CO\t` as per other comment headers

* Remove extraneous inclusion and add missing dependency

* Remove last traces of previously deleted bgzf_idx_amend_last()

As noted in #1722, this function was removed in PR #1672.

* Use isspace_c() et al in annot-tsv.c

* Minor corrections to system headers

Plain getopt() is declared in <unistd.h>; strcasecmp() et al are only
portably declared in <strings.h>.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@daviesrob daviesrob

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@daviesrob @emollier