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.

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

Fix clang builds under mingw. #1435

Merged
merged 1 commit into from
May 25, 2022
Merged

Conversation

jkbonfield
Copy link
Contributor

Under mingw clang requires dllexport to be applied to both function
declarations and function definitions. (Gcc is happy for the
definition only to have the dllexport attribute.)

Note this exports several "internal" functions, including some
apparently unused anywhere (hts_json_*). Perhaps they were once used
in htslib-plugins, but no more.

I haven't taken the decision to remove these though, but it's worth
considering for whenever we next do an ABI breaking change. Either
that or stop pretending that their internal only when clearly they are
not, and move their API to the external htslib/*.h files instead.
These appear to be somewhat in Limbo right now.

Fixes #1433

@jkbonfield
Copy link
Contributor Author

Note it fails the tests using gcc-12. I think these will be fixed by samtools/htscodecs#51, but haven't verified it. (Or alternatively use -O3 which is preferred anyway for htscodecs.)

@daviesrob daviesrob self-assigned this May 17, 2022
Under mingw clang requires dllexport to be applied to both function
declarations and function definitions.  (Gcc is happy for the
definition only to have the dllexport attribute.)

Note this exports several "internal" functions, including some
apparently unused anywhere (hts_json_*).   Perhaps they were once used
in htslib-plugins, but no more.

I haven't taken the decision to remove these though, but it's worth
considering for whenever we next do an ABI breaking change.  Either
that or stop pretending that their internal only when clearly they are
not, and move their API to the external htslib/*.h files instead.
These appear to be somewhat in Limbo right now.

Fixes samtools#1433
@daviesrob daviesrob merged commit 008eabd into samtools:develop May 25, 2022
jkbonfield added a commit to jkbonfield/htslib that referenced this pull request Aug 15, 2022
If we use HTSLIB_EXPORT once, it has to be used everywhere for that symbol.

Also see samtools#1435
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compiling with clang on Windows does not work
2 participants