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

Multiple definition errors #13

Open
hannespetur opened this issue Jul 28, 2016 · 5 comments
Open

Multiple definition errors #13

hannespetur opened this issue Jul 28, 2016 · 5 comments
Assignees

Comments

@hannespetur
Copy link

hannespetur commented Jul 28, 2016

Hey there, I am working on a program which both depends on libStatGen and HTSLib. When linking to both libraries I get the following errors:

/.../statgen/libStatGen.a(knetfile.o): In function `kftp_connect':
knetfile.c:(.text+0x320): multiple definition of `kftp_connect'
/.../htslib/libhts.a(knetfile.o):/.../htslib/knetfile.c:271: first defined here
/.../statgen/libStatGen.a(knetfile.o): In function `kftp_reconnect':
knetfile.c:(.text+0x530): multiple definition of `kftp_reconnect'
/.../htslib/libhts.a(knetfile.o):/.../htslib/knetfile.c:282: first defined here
/.../statgen/libStatGen.a(knetfile.o): In function `kftp_parse_url':
knetfile.c:(.text+0x760): multiple definition of `kftp_parse_url'
/.../htslib/libhts.a(knetfile.o):/.../htslib/knetfile.c:294: first defined here
/.../statgen/libStatGen.a(knetfile.o): In function `kftp_connect_file':
knetfile.c:(.text+0x8c0): multiple definition of `kftp_connect_file'

... and so on.

This is of course because both libraries have the same function names. Would it be possible to make it so both libraries can be linked without errors? As a short-term solution I simply renamed all these functions in libStatGen by adding 2 at the end. I could make a pull request, but perhaps you have other solutions in mind.

Thank you for your contributions.

@jonathonl
Copy link
Contributor

The appropriate fix for this and other naming conflicts is to wrap all of libStatGen in a namespace. This would be a breaking change but could be set up to be enabled with a preprocessor definition.

@hannespetur
Copy link
Author

I support the idea of wrapping libStatGen in a namespace. It fixes conflicts and makes your code clearer to read, especially if you are using many different libraries. However, in my case the naming conflicts originate from a C file and C doesn't support namespace resolution.

@jonathonl jonathonl self-assigned this Jul 29, 2016
@zhanxw
Copy link
Contributor

zhanxw commented Jul 29, 2016

Just curious, is it possible to use libStatGen alone?
Or, what function you will need to use is in htslib but not in libStatGen?

@hannespetur
Copy link
Author

Not always, there are some differences. The API is quite different, and in my opinion much clearer in libStatGen. But it is also missing some functionality, for example I don't see any support for BCF or CRAM.

@mktrost
Copy link
Contributor

mktrost commented Aug 4, 2016

You are correct, libStatGen does not support BCF or CRAM. It is a goal to someday support them (probably by incorporating htslib to handle those - ideally wrapping to maintain the libStatGen API).
Those conflicts you are seeing are actually from files that originally came from samtools that are now in both libStatGen & htslib.
I haven't looked at htslib in a while (I only work part time now), but if there hasn't been much change between the samtools version and the htslib version, libStatGen could point to the htslib version of the files if the user is also compiling with htslib.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants