Skip to content

cmake: use static dependencies when building static libgit2#4356

Merged
ethomson merged 1 commit into
libgit2:masterfrom
pks-t:pks/static-clar
Oct 9, 2017
Merged

cmake: use static dependencies when building static libgit2#4356
ethomson merged 1 commit into
libgit2:masterfrom
pks-t:pks/static-clar

Conversation

@pks-t

@pks-t pks-t commented Sep 22, 2017

Copy link
Copy Markdown
Member

CMake allows us to build a static library by simply setting the variable
BUILD_SHARED_LIBS to OFF. While this causes us to create a static
libgit2.a archive, it will not automatically cause CMake to only locate
static archives when searching for dependencies. This does no harm in
case of building our libgit2.a, as we do not want to include all
required dependencies in the resulting archive anyway. Instead, we ask
users of a static libgit2.a to link against the required set of static
archives themselves, typically aided by the libgit2.pc file.

Where it does cause harm, though, is when we build the libgit2_clar test
suite. CMake has happily populated our LIBGIT2_LIBS variable with shared
libraries, and so linking the final libgit2_clar test does not do the
right thing. It will simply ignore those shared libraries, we end up
with a test suite with undefined symbols.

To fix the issue, we can instruct CMake to only locate libraries with a
certain suffix. As static libraries are typically identifiable by their
".a" suffix on Unix-based systems, we can instruct CMake to only locate
libraries with this suffix to restrict it from finding any shared
libraries. This fixes building a static libgit2_clar test suite.

Note that this ignores the problem on Windows. The problem here is that
we cannot even distinguish static and dynamic libraries by only
inspecting their suffix. So we just ignore the problem on Windows, for
now.

CMake allows us to build a static library by simply setting the variable
`BUILD_SHARED_LIBS` to `OFF`. While this causes us to create a static
libgit2.a archive, it will not automatically cause CMake to only locate
static archives when searching for dependencies. This does no harm in
case of building our libgit2.a, as we do not want to include all
required dependencies in the resulting archive anyway. Instead, we ask
users of a static libgit2.a to link against the required set of static
archives themselves, typically aided by the libgit2.pc file.

Where it does cause harm, though, is when we build the libgit2_clar test
suite. CMake has happily populated our LIBGIT2_LIBS variable with shared
libraries, and so linking the final libgit2_clar test does not do the
right thing. It will simply ignore those shared libraries, we end up
with a test suite with undefined symbols.

To fix the issue, we can instruct CMake to only locate libraries with a
certain suffix. As static libraries are typically identifiable by their
".a" suffix on Unix-based systems, we can instruct CMake to only locate
libraries with this suffix to restrict it from finding any shared
libraries. This fixes building a static libgit2_clar test suite.

Note that this ignores the problem on Windows. The problem here is that
we cannot even distinguish static and dynamic libraries by only
inspecting their suffix. So we just ignore the problem on Windows, for
now.
@pks-t

pks-t commented Sep 22, 2017

Copy link
Copy Markdown
Member Author

By now I feel like I've opened a can of worms by ever touching our CMake build instructions. I'm surprised by how many issues we actually had which were previously undiscovered.

@ethomson

ethomson commented Oct 7, 2017

Copy link
Copy Markdown
Member

The problem here is that we cannot even distinguish static and dynamic libraries by only inspecting their suffix.

You should be able to: static libraries are .LIB on Windows, while dynamic libraries are .DLL.

(IIRC, there's a subtlety here. I think that there's a compiler/linker option that sets it up to build a .LIB stub library that actually handles the dynamic loading itself - ie, it does the morale equivalent of a dlopen. But strictly speaking it is still a static binary, and we can probably ignore that case anyway since we don't do that.)

@pks-t

pks-t commented Oct 9, 2017

Copy link
Copy Markdown
Member Author

On Windows you can have ".lib" files for both static and dynamic libraries and you are not able to easily distinguish them. See for example https://cmake.org/Bug/view.php?id=1643 (not the best reference, I had better ones but cannot find them anymore).

@ethomson

ethomson commented Oct 9, 2017

Copy link
Copy Markdown
Member

I think that's an "import library" - that's the stub .LIB that I was talking about. I may be wrong, but I don't think that a .LIB can actually export symbols so that you couldn't create a .LIB that could be dynamically opened...

Anyway - we're getting off on a tangent. 😛

Why don't we go ahead and merge this. I think that it would also be valuable on Windows, but it's not a pressing need for anybody at the moment and even if it's as easy as adding a .LIB suffix to a list, it will definitely require testing.

@ethomson ethomson merged commit 8ac8c78 into libgit2:master Oct 9, 2017
@pks-t pks-t deleted the pks/static-clar branch November 11, 2017 20:34
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.

2 participants