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

Link flif to libflif.so for release builds, and dflif to libflif_dec.so #416

Merged
merged 1 commit into from
May 13, 2017

Conversation

Traneptora
Copy link
Contributor

For release builds (make flif, make decoder), the CLI will be dynamically linked to the library. I also removed -fwhole-program from the optimizations because flif and dflif are no longer the "whole program" in one go.

@jonsneyers
Copy link
Member

It's going to be more complicated than just changing the Makefile – the CLI should actually use the library API instead of calling the various functions directly, otherwise the actual code is still in the flif executable and the library linking is not actually doing anything. So that's going to require some changes in flif.cpp...

@Traneptora
Copy link
Contributor Author

But if that's true, why does it work?

@Traneptora
Copy link
Contributor Author

Because compiling it as above does actually work and allow flif to run with no problems, despite not having the libflif.so code inside flif

@jonsneyers jonsneyers merged commit 98e68a8 into FLIF-hub:master May 13, 2017
@jonsneyers
Copy link
Member

Nice, I didn't realize that it was going to work just like that to reduce the size of the flif binary. Still, it would be better to use the actual library API, so it uses a well-defined API which in the future we can keep consistent, allowing e.g. drop-in replacements in case someone would make an alternative encoder or a faster decoder.

@fherzog2
Copy link
Contributor

It works because gcc is exporting all the internal C++ symbols for libflif by default. On Windows this change wouldn't be so simple, because there only the proper API is exported from the DLL. I agree that the executable should use the library API. That would make testing easier as well (no separate tests for executable and library needed).

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.

3 participants