build: Make libunivalue a non-Libtool convenience library#24972
build: Make libunivalue a non-Libtool convenience library#24972hebasto wants to merge 1 commit intobitcoin:masterfrom
libunivalue a non-Libtool convenience library#24972Conversation
To follow #24322 approach, this PR should contain the alternative diff mentioned in the OP: --- a/src/Makefile.univalue.include
+++ b/src/Makefile.univalue.include
@@ -4,3 +4,6 @@ LIBUNIVALUE = libunivalue.la
noinst_LTLIBRARIES += $(LIBUNIVALUE)
libunivalue_la_SOURCES = $(UNIVALUE_LIB_SOURCES_INT) $(UNIVALUE_DIST_HEADERS_INT) $(UNIVALUE_LIB_HEADERS_INT) $(UNIVALUE_TEST_FILES_INT)
libunivalue_la_CPPFLAGS = $(AM_CPPFLAGS) -I$(srcdir)/$(UNIVALUE_INCLUDE_DIR_INT)
+if TARGET_WINDOWS
+libunivalue_la_LDFLAGS = -static
+endif |
ryanofsky
left a comment
ryanofsky
left a comment
There was a problem hiding this comment.
Code review ACK c91fc29 for the current fix. The -static fix in #24972 (comment) also seems ok if there is a comment explaining why -static is needed on windows.
I'm not sure how these fixes relate to the original issue. If univalue test binaries depend on pthread functions, shouldn't they just be built with -lpthread? Having the test binaries inherit the library's dependencies by linking the library statically instead of dynamically seems like in an indirect workaround, if it is how this fix works.
re: #24972 (comment)
After #22646 the
libunivaluelibrary been built not using its own build system. In such a context it will never be a shared library. Therefore, no need to use Libtool, and thelibunivaluelibrary could be a non-Libtool convenience library.
I think this isn't strictly true. Even if the library is only built as a static library, not a dynamic library, you may still need to build it with libtool to apply PIC flags if that static library is a dependency of a dynamic library. libbitcoinconsensus is the only dynamic library we have though, so probably this isn't a concern.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
🐙 This pull request conflicts with the target branch and needs rebase. Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft". |
|
Concept NACK. Prefer the libtool change from #25008. |
There hasn't been much activity lately and the patch still needs rebase. What is the status here?
|
|
Closing this as it has not had any activity in a while. If you are interested in continuing work on this, please leave a comment so that it can be reopened. |
6 participants
Although #22646 was a great change, it broke Windows builds with
DEBUG=1:Please note that the
--without-libsconfiguration option, therefore, this bug differs from #19772.Unfortunately, this is another evidence of the tough relationship between Libtool and cross-compiler for MinGW-w64. Other workarounds see here and here.
This PR avoids using Libtool for the
libunivaluelibrary at all. After #22646 thelibunivaluelibrary been built not using its own build system. In such a context it will never be a shared library. Therefore, no need to use Libtool, and thelibunivaluelibrary could be a non-Libtool convenience library.Btw, I strongly believe such an approach could also solve #19772.
FWIW, while working on this PR I found a more concise fix: