ArthurSonzogni / FTXUI

:computer: C++ Functional Terminal User Interface. :heart:
MIT License
6.79k stars 409 forks source link

Added -fPIC compile option on UNIX #913

Closed pbosetti closed 1 month ago

pbosetti commented 1 month ago

Without this, a shared object containing FTXUI code won't work. This is only needed on Unix.

ArthurSonzogni commented 1 month ago

Hello Paolo, Thanks for this PR.

I suspect the problem is on your side, not FTXUI. I would like to learn more about the case you are encountering. I heard CMake automatically adds that flag to all shared libraries. Is this instruction really needed?

If we want to go this route, the "cmake" way is:

set_property(TARGET ${library} PROPERTY POSITION_INDEPENDENT_CODE ON)

this might gives "smarter" results depending on how cmake handle this.

pbosetti commented 1 month ago

Hello Arthur, I am testing (and getting an error) on a clean Ubuntu 24.04 install, while I got no problem on MacOS or Windows. The use case is admittedly not so common: I am integrating FTXUI in a plugin (a shared object) for a command line program. FTXUI is vendorized and linked to the plugin as a static library. The plugin code is compiled as PIC, but apparently the FTXUI static lib is not, so there's the error (which honestly surprises me as well: I was trusting cmake to take care of this), As said, this is probably a very limited and rare use case (which only shows up on Linux), but adding a -fPIC won't be harmful for other use cases, wouldn't it? Of course your cmake line is more elegant, although being it only an issue on Linux that's why I've used a more brute approach. What do you think?

ArthurSonzogni commented 1 month ago

Thanks!

As long as it is not harmful for performance, I am fine adding it with the CMake property to keep compatible with every compilers. Could you add a comment explaing why we are adding this? Something along the lines of:

# CMake does automatically add -fPIC when linking a shared library, but it does
# not add it when linking a static library. This is a problem when the static
# library is later linked into a shared library. Doing it helps some users.
set_property(TARGET ${library} PROPERTY POSITION_INDEPENDENT_CODE ON)
pbosetti commented 1 month ago

Done! Thx, P