Update compile options to enable static linking to work#380
Update compile options to enable static linking to work#380davidchisnall merged 4 commits intognustep:masterfrom
Conversation
davidchisnall
left a comment
There was a problem hiding this comment.
Please can you also add static to at least some of the test matrix so that we don't break it in the future?
| if (IS_DIRECTORY /usr/local/include) | ||
| include_directories("/usr/local/include") | ||
| endif() |
There was a problem hiding this comment.
I'm not sure why this is needed, we should be fetching the robin map if it isn't installed, or finding it otherwise. I don't like accidentally depending on ambient things if we can avoid it.
There was a problem hiding this comment.
yes, it puzzled me, because even though it includes the header, you dont need to link the robin library to use the result. will take a look at this.
will also look at the tests, [ I assumed that they would run automatically if I built it static ]
There was a problem hiding this comment.
[ oh, the code is all in the header.... ]
There was a problem hiding this comment.
For the tests, if you build the static library, it now compiles a static version of each of the non-legacy tests, and statically links the whole thing. Those get run alongside the shared library tests, and they all pass.
[ I am not great at CMake, so I have to admit I used Claude for the actual changes to make this happen. I understand what it has done, and it seems reasonable to me, but you may want to eyeball that last commit a bit more closely. ]
There was a problem hiding this comment.
Yes, CMake is a bit weird, ‘link library’ really means ‘import everything this target exposes’, which includes adding include paths it exports.
This makes the static options the same as the dynamic, which was causing the linking to fail. As this requires robin-map, and this is installed under /usr/local on BSD and similar systems, then we also add /usr/local/include to the header search path if it exists .
This rmeoves the clunky insretion of /usr/local/include, which was my prevuous fix. It nwo works the same way for static as it does for dynamic.
When we are bulding the static library, then a second version of each of the non-legacy tests is generated, linked against it. These are run together with the shared library versions.
…table The previous commit created tests which linked the static library, but were stil themselves dynamic executables. To properly test that we can use the static library in a statically linked executable we want the tests themselevs to be linekd this way. Hence this.
This makes the static options the same as the dynamic, which was causing the linking to fail. As this requires robin-map, and this is installed under /usr/local on BSD and similar systems, then we also add /usr/local/include to the header search path if it exists.
All the tests pass, and this works perfectly with my test code too.