Skip to contentSkip to conversation

package/git: Fix static compilation

PATCH New Remi Pommarel 2 participants 8 messages Nov 2, 2015 – Nov 4, 2015

lore
Share & tools
lore
#0 lore raw

v3 cover package/git: Fix static compilation

Hi,

This patch series fixes git package static compilation. This fixes missing library dependency coming from static library undefined symbols as well as backport of submitted and accepted git patches.

Thanks
Remi

Remi Pommarel (3):
package/git: Add needed library for static compilation package/git: Add git patch for static compilation package/git: Used curl-config program for target

 .../0001-Makefile-link-libcurl-before-zlib.patch   | 62 +++++++++++++++
 ...kefile-make-curl-config-path-configurable.patch | 88 ++++++++++++++++++++++
 ...configure.ac-detect-ssl-need-with-libcurl.patch | 46 +++++++++++
 package/git/git.mk                                 |  8 +-
 4 files changed, 202 insertions(+), 2 deletions(-)
 create mode 100644 package/git/0001-Makefile-link-libcurl-before-zlib.patch
 create mode 100644 package/git/0002-Makefile-make-curl-config-path-configurable.patch
 create mode 100644 package/git/0003-configure.ac-detect-ssl-need-with-libcurl.patch
Signature · 1 line
2.0.1
#1 lore raw

PATCH v3 1/3 package/git: Add needed library for static compilation

Add missing ssl and crypto libraries in LIBS variable used by configure.

A statically compiled libcurl need to be linked with some libraries to get all its undefined symbols resolved. Indeed when libcurl supports openssl it needs -lssl to satisfy all its dependencies, the same goes for crypto symbols.

Configure uses LIBS variable to get additional library when it compile applilcations to probe for features.

---
 package/git/git.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
package/git/git.mk +1 -1
index 2bd6a01..2c404b4 100644
@@ -14,7 +14,7 @@ GIT_DEPENDENCIES = zlib host-gettext
 ifeq ($(BR2_PACKAGE_OPENSSL),y)
 GIT_DEPENDENCIES += openssl
 GIT_CONF_OPTS += --with-openssl
-GIT_CONF_ENV_LIBS += $(if $(BR2_STATIC_LIBS),-lz)
+GIT_CONF_ENV_LIBS += $(if $(BR2_STATIC_LIBS),-lssl -lcrypto -lz)
 else
 GIT_CONF_OPTS += --without-openssl
 endif
Signature · 1 line
2.0.1
#2 lore raw

PATCH v3 1/3 package/git: Add needed library for static compilation

Dear Remi Pommarel,

Quoted 5 lines · Remi Pommarel

Add missing ssl and crypto libraries in LIBS variable used by configure.

A statically compiled libcurl need to be linked with some libraries to get all its undefined symbols resolved. Indeed when libcurl supports openssl it needs -lssl to satisfy all its dependencies, the same goes for crypto symbols.

It isn't clear what is the relation here between libcurl and openssl. Can you detail?

package/git/git.mk GIT_DEPENDENCIES = zlib host-gettext · line 20

Quoted 13 lines
package/git/git.mk +1 -1
index 2bd6a01..2c404b4 100644
@@ -14,7 +14,7 @@ GIT_DEPENDENCIES = zlib host-gettext
 ifeq ($(BR2_PACKAGE_OPENSSL),y)
 GIT_DEPENDENCIES += openssl
 GIT_CONF_OPTS += --with-openssl
-GIT_CONF_ENV_LIBS += $(if $(BR2_STATIC_LIBS),-lz)
+GIT_CONF_ENV_LIBS += $(if $(BR2_STATIC_LIBS),-lssl -lcrypto -lz)
 else
 GIT_CONF_OPTS += --without-openssl
 endif

What is this fixing exactly? I've tried building Git statically, then Git+OpenSSL statically, then Git+OpenSSL+libcurl statically, and all these configurations. Can you rework your patch to clearly indicate in the commit log which configuration is currently failing and how it is failing (relevant error messages) ?

The following configuration:

BR2_arm=y
BR2_STATIC_LIBS=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-arm-full-static-2015.08-647-gc356fb2.tar.bz2" BR2_TOOLCHAIN_EXTERNAL_GCC_4_9=y
BR2_TOOLCHAIN_EXTERNAL_HEADERS_3_14=y
BR2_TOOLCHAIN_EXTERNAL_LOCALE=y
# BR2_TOOLCHAIN_EXTERNAL_HAS_THREADS_DEBUG is not set
BR2_TOOLCHAIN_EXTERNAL_INET_RPC=y
BR2_TOOLCHAIN_EXTERNAL_CXX=y
BR2_INIT_NONE=y
BR2_SYSTEM_BIN_SH_NONE=y
# BR2_PACKAGE_BUSYBOX is not set
BR2_PACKAGE_GIT=y
BR2_PACKAGE_OPENSSL=y
BR2_PACKAGE_LIBCURL=y
# BR2_TARGET_ROOTFS_TAR is not set

Builds just fine here.

Thanks,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#3 lore raw

PATCH v3 1/3 package/git: Add needed library for static compilation

Hello Remi,

Quoted 5 lines · Thomas Petazzoni

What is this fixing exactly? I've tried building Git statically, then Git+OpenSSL statically, then Git+OpenSSL+libcurl statically, and all these configurations. Can you rework your patch to clearly indicate in the commit log which configuration is currently failing and how it is failing (relevant error messages) ?

Ok, now that I looked to your patch 3/3, I understand a bit more the problem. In my testing, it in fact doesn't detect curl anymore, because it can't link against libcurl due to missing libraries.

Your patch 3/3 adds support for specifying a curl-config program, which is good. But the entire purpose of using curl-config is to get the appropriate compiler and linker flags, and in our case, curl-config is properly saying that we should link against -lssl -lcrypto:

$ ./output/staging/usr/bin/curl-config --libs
-L/home/thomas/projets/buildroot/output/host/usr/arm-buildroot-linux-uclibcgnueabi/sysroot/usr/lib -lcurl -lssl -lcrypto -lssl -lz -lz -lcrypto -lz -lz

So we definitely shouldn't have to do the hacks that you do in PATCH 1/3. Except that you have to do it because Git configure.ac is not doing the right thing.

Since you've already contributed several fixes for this to the Git people, can you work with upstream to resolve the problem ? Either by calling the curl-config program from configure.ac, or by using pkg-config (but Git developers may not like having to use pkg-config).

Also, your patch organization is weird: your patch 2/3 is adding patches 0001 and 0003, while your patch 3/3 is adding patch 0002. This is pretty odd.

Thanks,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#4 lore raw

PATCH v3 1/3 package/git: Add needed library for static compilation

Hi,

Quoted 10 lines · Thomas Petazzoni

Hello Remi,

Quoted 5 lines · Thomas Petazzoni

What is this fixing exactly? I've tried building Git statically, then Git+OpenSSL statically, then Git+OpenSSL+libcurl statically, and all these configurations. Can you rework your patch to clearly indicate in the commit log which configuration is currently failing and how it is failing (relevant error messages) ?

Quoted 3 lines

Ok, now that I looked to your patch 3/3, I understand a bit more the problem. In my testing, it in fact doesn't detect curl anymore, because it can't link against libcurl due to missing libraries.

Exactly

Quoted 9 lines

Your patch 3/3 adds support for specifying a curl-config program, which is good. But the entire purpose of using curl-config is to get the appropriate compiler and linker flags, and in our case, curl-config is properly saying that we should link against -lssl -lcrypto:

$ ./output/staging/usr/bin/curl-config --libs
-L/home/thomas/projets/buildroot/output/host/usr/arm-buildroot-linux-uclibcgnueabi/sysroot/usr/lib -lcurl -lssl -lcrypto -lssl -lz -lz -lcrypto -lz -lz
Quoted 8 lines

So we definitely shouldn't have to do the hacks that you do in PATCH 1/3. Except that you have to do it because Git configure.ac is not doing the right thing.

Since you've already contributed several fixes for this to the Git people, can you work with upstream to resolve the problem ? Either by calling the curl-config program from configure.ac, or by using pkg-config (but Git developers may not like having to use pkg-config).

In fact I was using this trick to workaround configure, because it was already used (cf. GIT_CONF_ENV_LIBS with -lz and -liconv), thus avoiding me to dive into git's configure.ac.

Agreed that a patch that fixes configure.ac would be a better solution, but as it does not exist yet I though the workaround would be a good temporary fix.

Quoted 4 lines

Also, your patch organization is weird: your patch 2/3 is adding patches 0001 and 0003, while your patch 3/3 is adding patch 0002. This is pretty odd.

Yes it is odd, I can re-number the patches if they apply correctly.

Thanks

Signature · 1 line
Remi
#5 lore raw

PATCH v3 1/3 package/git: Add needed library for static compilation

In reply to Thomas Petazzoni
Quoted 12 lines · Thomas Petazzoni

Dear Remi Pommarel,

Quoted 5 lines · Remi Pommarel

Add missing ssl and crypto libraries in LIBS variable used by configure.

A statically compiled libcurl need to be linked with some libraries to get all its undefined symbols resolved. Indeed when libcurl supports openssl it needs -lssl to satisfy all its dependencies, the same goes for crypto symbols.

It isn't clear what is the relation here between libcurl and openssl. Can you detail?

Libcurl uses openssl (when compiled with ssl support) functions to support protocol such as HTTPS. So libcurl will have many undefined symbols from openssl.

Quoted 42 lines

package/git/git.mk GIT_DEPENDENCIES = zlib host-gettext · line 20

Quoted 13 lines
package/git/git.mk +1 -1
index 2bd6a01..2c404b4 100644
@@ -14,7 +14,7 @@ GIT_DEPENDENCIES = zlib host-gettext
 ifeq ($(BR2_PACKAGE_OPENSSL),y)
 GIT_DEPENDENCIES += openssl
 GIT_CONF_OPTS += --with-openssl
-GIT_CONF_ENV_LIBS += $(if $(BR2_STATIC_LIBS),-lz)
+GIT_CONF_ENV_LIBS += $(if $(BR2_STATIC_LIBS),-lssl -lcrypto -lz)
 else
 GIT_CONF_OPTS += --without-openssl
 endif

What is this fixing exactly? I've tried building Git statically, then Git+OpenSSL statically, then Git+OpenSSL+libcurl statically, and all these configurations. Can you rework your patch to clearly indicate in the commit log which configuration is currently failing and how it is failing (relevant error messages) ?

The following configuration:

BR2_arm=y
BR2_STATIC_LIBS=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-arm-full-static-2015.08-647-gc356fb2.tar.bz2" BR2_TOOLCHAIN_EXTERNAL_GCC_4_9=y
BR2_TOOLCHAIN_EXTERNAL_HEADERS_3_14=y
BR2_TOOLCHAIN_EXTERNAL_LOCALE=y
# BR2_TOOLCHAIN_EXTERNAL_HAS_THREADS_DEBUG is not set
BR2_TOOLCHAIN_EXTERNAL_INET_RPC=y
BR2_TOOLCHAIN_EXTERNAL_CXX=y
BR2_INIT_NONE=y
BR2_SYSTEM_BIN_SH_NONE=y
# BR2_PACKAGE_BUSYBOX is not set
BR2_PACKAGE_GIT=y
BR2_PACKAGE_OPENSSL=y
BR2_PACKAGE_LIBCURL=y
# BR2_TARGET_ROOTFS_TAR is not set

Quoted 1 line

Builds just fine here.

It builds fine, but git-http-{fetch,push} will not be built as they should be. What happens is that configure fail to detect libcurl because its simple test program will not compile due to missing ssl library.

Thanks,

Signature · 1 line
Remi
#6 lore raw

PATCH v3 2/3 package/git: Add git patch for static compilation

In reply to Remi Pommarel

Use submitted git patches that fix static compilation:
- The first patch fix library link order
- The second patch adds -lssl when needed with libcurl

These patches fix missing library in the Makefile for git compilation. configure still need additional library to be passed to LIBS variable to compile test programs for feature detection.

---
 .../0001-Makefile-link-libcurl-before-zlib.patch   | 62 ++++++++++++++++++++++
 ...configure.ac-detect-ssl-need-with-libcurl.patch | 46 ++++++++++++++++
 package/git/git.mk                                 |  2 +
 3 files changed, 110 insertions(+)
 create mode 100644 package/git/0001-Makefile-link-libcurl-before-zlib.patch
 create mode 100644 package/git/0003-configure.ac-detect-ssl-need-with-libcurl.patch
package/git/0001-Makefile-link-libcurl-before-zlib.patch new file +62 -0
new file mode 100644
index 0000000..8b6786e
@@ -0,0 +1,62 @@
+From 9eaa78b0b037964da85d6a15f0128639f2118a53 Mon Sep 17 00:00:00 2001
+From: Remi Pommarel <[email protected]>
+Date: Wed, 21 Oct 2015 19:01:13 +0200
+Subject: [PATCH 1/3] Makefile: link libcurl before zlib
+
+For static linking especially library order while linking is important. For
+example, libcurl wants symbols from zlib when building http-push, http-fetch
+and remote-curl. So for these programs libcurl has to be linked before zlib.
+
+Signed-off-by: Remi Pommarel <[email protected]>
+Reviewed-by: Jonathan Nieder <[email protected]>
+Signed-off-by: Junio C Hamano <[email protected]>
+---
+ Makefile | 9 +++++----
+ 1 file changed, 5 insertions(+), 4 deletions(-)
+
+diff --git a/Makefile b/Makefile
+index ce0cfe2..9787385 100644
+--- a/Makefile
++++ b/Makefile
+@@ -1029,7 +1029,7 @@ ifdef HAVE_ALLOCA_H
+ endif
+ 
+ IMAP_SEND_BUILDDEPS =
+-IMAP_SEND_LDFLAGS = $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
++IMAP_SEND_LDFLAGS =
+ 
+ ifdef NO_CURL
+ 	BASIC_CFLAGS += -DNO_CURL
+@@ -1086,6 +1086,7 @@ else
+ 		endif
+ 	endif
+ endif
++IMAP_SEND_LDFLAGS += $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
+ 
+ ifdef ZLIB_PATH
+ 	BASIC_CFLAGS += -I$(ZLIB_PATH)/include
+@@ -1971,10 +1972,10 @@ git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)
+ 
+ git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)
+ 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
+-		$(LIBS) $(CURL_LIBCURL)
++		$(CURL_LIBCURL) $(LIBS)
+ git-http-push$X: http.o http-push.o GIT-LDFLAGS $(GITLIBS)
+ 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
+-		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
++		$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS)
+ 
+ git-remote-testsvn$X: remote-testsvn.o GIT-LDFLAGS $(GITLIBS) $(VCSSVN_LIB)
+ 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) \
+@@ -1988,7 +1989,7 @@ $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY)
+ 
+ $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS)
+ 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
+-		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
++		$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS)
+ 
+ $(LIB_FILE): $(LIB_OBJS)
+ 	$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $^
+-- 
+2.0.1
+
package/git/0003-configure.ac-detect-ssl-need-with-libcurl.patch new file +46 -0

46 lines not on this page · raw message

package/git/git.mk +2 -0
index 2c404b4..79cc31c 100644
@@ -9,6 +9,8 @@ GIT_SOURCE = git-$(GIT_VERSION).tar.xz
 GIT_SITE = https://www.kernel.org/pub/software/scm/git
 GIT_LICENSE = GPLv2 LGPLv2.1+
 GIT_LICENSE_FILES = COPYING LGPL-2.1
+# configure.ac gets patched, so autoreconf is necessary
+GIT_AUTORECONF = YES
 GIT_DEPENDENCIES = zlib host-gettext
 
 ifeq ($(BR2_PACKAGE_OPENSSL),y)
Signature · 1 line
2.0.1
#7 lore raw

PATCH v3 3/3 package/git: Used curl-config program for target

In reply to Remi Pommarel

Fix cross compilation by using curl-config script for target instead of the one from host.

---
 ...kefile-make-curl-config-path-configurable.patch | 88 ++++++++++++++++++++++
 package/git/git.mk                                 |  4 +-
 2 files changed, 91 insertions(+), 1 deletion(-)
 create mode 100644 package/git/0002-Makefile-make-curl-config-path-configurable.patch
package/git/0002-Makefile-make-curl-config-path-configurable.patch new file +88 -0
new file mode 100644
index 0000000..b281b99
@@ -0,0 +1,88 @@
+From f89158760d5f02ba59f644799abd921e6be22f13 Mon Sep 17 00:00:00 2001
+From: Remi Pommarel <[email protected]>
+Date: Wed, 21 Oct 2015 19:10:46 +0200
+Subject: [PATCH 2/3] Makefile: make curl-config path configurable
+
+There are situations, e.g. during cross compilation, where curl-config
+program is not present in the PATH.
+
+Make the makefile use a configurable curl-config program passed through
+CURL_CONFIG variable which can be set through config.mak.
+
+Also make this variable tunable through use of autoconf/configure. Configure
+will set CURL_CONFIG variable in config.mak.autogen to whatever value has been
+passed to ac_cv_prog_CURL_CONFIG.
+
+Signed-off-by: Remi Pommarel <[email protected]>
+Reviewed-by: Jonathan Nieder <[email protected]>
+Signed-off-by: Junio C Hamano <[email protected]>
+---
+ Makefile     |  8 ++++++--
+ configure.ac | 13 +++++++++++++
+ 2 files changed, 19 insertions(+), 2 deletions(-)
+
+diff --git a/Makefile b/Makefile
+index 9787385..64d17d0 100644
+--- a/Makefile
++++ b/Makefile
+@@ -39,6 +39,9 @@ all::
+ # Define CURLDIR=/foo/bar if your curl header and library files are in
+ # /foo/bar/include and /foo/bar/lib directories.
+ #
++# Define CURL_CONFIG to curl's configuration program that prints information
++# about the library (e.g., its version number).  The default is 'curl-config'.
++#
+ # Define NO_EXPAT if you do not have expat installed.  git-http-push is
+ # not built, and you cannot push using http:// and https:// transports (dumb).
+ #
+@@ -425,6 +428,7 @@ TCL_PATH = tclsh
+ TCLTK_PATH = wish
+ XGETTEXT = xgettext
+ MSGFMT = msgfmt
++CURL_CONFIG = curl-config
+ PTHREAD_LIBS = -lpthread
+ PTHREAD_CFLAGS =
+ GCOV = gcov
+@@ -1059,13 +1063,13 @@ else
+ 	REMOTE_CURL_NAMES = $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_ALIASES)
+ 	PROGRAM_OBJS += http-fetch.o
+ 	PROGRAMS += $(REMOTE_CURL_NAMES)
+-	curl_check := $(shell (echo 070908; curl-config --vernum | sed -e '/^70[BC]/s/^/0/') 2>/dev/null | sort -r | sed -ne 2p)
++	curl_check := $(shell (echo 070908; $(CURL_CONFIG) --vernum | sed -e '/^70[BC]/s/^/0/') 2>/dev/null | sort -r | sed -ne 2p)
+ 	ifeq "$(curl_check)" "070908"
+ 		ifndef NO_EXPAT
+ 			PROGRAM_OBJS += http-push.o
+ 		endif
+ 	endif
+-	curl_check := $(shell (echo 072200; curl-config --vernum | sed -e '/^70[BC]/s/^/0/') 2>/dev/null | sort -r | sed -ne 2p)
++	curl_check := $(shell (echo 072200; $(CURL_CONFIG) --vernum | sed -e '/^70[BC]/s/^/0/') 2>/dev/null | sort -r | sed -ne 2p)
+ 	ifeq "$(curl_check)" "072200"
+ 		USE_CURL_FOR_IMAP_SEND = YesPlease
+ 	endif
+diff --git a/configure.ac b/configure.ac
+index 14012fa..01b07ad 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -525,6 +525,19 @@ GIT_UNSTASH_FLAGS($CURLDIR)
+ 
+ GIT_CONF_SUBST([NO_CURL])
+ 
++if test -z "$NO_CURL"; then
++
++AC_CHECK_PROG([CURL_CONFIG], [curl-config],
++[curl-config],
++[no])
++
++if test $CURL_CONFIG != no; then
++    GIT_CONF_SUBST([CURL_CONFIG])
++fi
++
++fi
++
++
+ #
+ # Define NO_EXPAT if you do not have expat installed.  git-http-push is
+ # not built, and you cannot push using http:// and https:// transports.
+-- 
+2.0.1
+
package/git/git.mk +3 -1
index 79cc31c..7cfc0d4 100644
@@ -31,6 +31,8 @@ endif
 ifeq ($(BR2_PACKAGE_CURL),y)
 GIT_DEPENDENCIES += libcurl
 GIT_CONF_OPTS += --with-curl
+GIT_CONF_ENV +=	\
+	ac_cv_prog_curl_config=$(STAGING_DIR)/usr/bin/$(LIBCURL_CONFIG_SCRIPTS)
 else
 GIT_CONF_OPTS += --without-curl
 endif
@@ -59,7 +61,7 @@ endif
 
 # assume yes for these tests, configure will bail out otherwise
 # saying error: cannot run test program while cross compiling
-GIT_CONF_ENV = \
+GIT_CONF_ENV += \
 	ac_cv_fread_reads_directories=yes \
 	ac_cv_snprintf_returns_bogus=yes LIBS='$(GIT_CONF_ENV_LIBS)'
 
Signature · 1 line
2.0.1