| Message ID | 1445893962-13338-2-git-send-email-repk@triplefau.lt |
|---|---|
| State | Superseded |
| Headers | show |
Hi Remi, On Mon, Oct 26, 2015 at 10:12:40PM +0100, Remi Pommarel wrote: > Add missing ssl, zlib and gettext library for static compilation support. > > 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 gettext and zlib > symbols. > > Signed-off-by: Remi Pommarel <repk@triplefau.lt> > --- > package/git/git.mk | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/package/git/git.mk b/package/git/git.mk > index 2bd6a01..942f29f 100644 > --- a/package/git/git.mk > +++ b/package/git/git.mk > @@ -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) zlib is always selected. You should not remove it. Also, since you seem to fix the libcurl issue in later patches is this still needed at all? > else > GIT_CONF_OPTS += --without-openssl > endif > @@ -55,9 +55,20 @@ else > GIT_CONF_OPTS += --without-tcltk > endif > > +ifeq ($(BR2_PACKAGE_GETTEXT),y) > +GIT_DEPENDENCIES += gettext > +GIT_CONF_ENV += ac_cv_lib_c_gettext=no > +endif Are you sure this fixes a static build issue? If not, please make a separate patch for this part. > +ifeq ($(BR2_PACKAGE_ZLIB),y) > +GIT_DEPENDENCIES += zlib > +GIT_CONF_ENV_LIBS += -lz > +endif This part should not be needed. zlib is already in GIT_DEPENDENCIES (above), -lz should be in GIT_CONF_ENV_LIBS for static build, and BR2_PACKAGE_ZLIB is always selected. baruch
Hi Baruch, On Tue, Oct 27, 2015 at 07:32:06AM +0200, Baruch Siach wrote: > Hi Remi, > > On Mon, Oct 26, 2015 at 10:12:40PM +0100, Remi Pommarel wrote: > > Add missing ssl, zlib and gettext library for static compilation support. > > > > 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 gettext and zlib > > symbols. > > > > Signed-off-by: Remi Pommarel <repk@triplefau.lt> > > --- > > package/git/git.mk | 15 +++++++++++++-- > > 1 file changed, 13 insertions(+), 2 deletions(-) > > > > diff --git a/package/git/git.mk b/package/git/git.mk > > index 2bd6a01..942f29f 100644 > > --- a/package/git/git.mk > > +++ b/package/git/git.mk > > @@ -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) > > zlib is always selected. You should not remove it. Ok will do, thanks. > > Also, since you seem to fix the libcurl issue in later patches is this still > needed at all? > There are two places the additional libraries are needed. In the Makefile in order to compile git binaries, the patches fix the issue here. And in the configure when it compile test application to probe for features, unfortunatly configure still need LIBS varibale to be set. > > else > > GIT_CONF_OPTS += --without-openssl > > endif > > @@ -55,9 +55,20 @@ else > > GIT_CONF_OPTS += --without-tcltk > > endif > > > > +ifeq ($(BR2_PACKAGE_GETTEXT),y) > > +GIT_DEPENDENCIES += gettext > > +GIT_CONF_ENV += ac_cv_lib_c_gettext=no > > +endif > > Are you sure this fixes a static build issue? If not, please make a separate > patch for this part. > When statically compiled with gettext support libcurl need to be linked with libintl, using ac_cv_lib_c_gettext does the job. So for me its static linking related. > > +ifeq ($(BR2_PACKAGE_ZLIB),y) > > +GIT_DEPENDENCIES += zlib > > +GIT_CONF_ENV_LIBS += -lz > > +endif > > This part should not be needed. zlib is already in GIT_DEPENDENCIES (above), > -lz should be in GIT_CONF_ENV_LIBS for static build, and BR2_PACKAGE_ZLIB is > always selected. > Ok will remove this bit. Thanks
Hi Remi, On Wed, Oct 28, 2015 at 09:16:24AM +0100, Remi Pommarel wrote: > On Tue, Oct 27, 2015 at 07:32:06AM +0200, Baruch Siach wrote: > > On Mon, Oct 26, 2015 at 10:12:40PM +0100, Remi Pommarel wrote: > > > Add missing ssl, zlib and gettext library for static compilation support. > > > > > > 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 gettext and zlib > > > symbols. > > > > > > Signed-off-by: Remi Pommarel <repk@triplefau.lt> > > > --- > > > package/git/git.mk | 15 +++++++++++++-- > > > 1 file changed, 13 insertions(+), 2 deletions(-) > > > > > > diff --git a/package/git/git.mk b/package/git/git.mk > > > index 2bd6a01..942f29f 100644 > > > --- a/package/git/git.mk > > > +++ b/package/git/git.mk > > > @@ -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) > > > > zlib is always selected. You should not remove it. > > Ok will do, thanks. > > > > > Also, since you seem to fix the libcurl issue in later patches is this still > > needed at all? > > > > There are two places the additional libraries are needed. In the Makefile > in order to compile git binaries, the patches fix the issue here. And in the > configure when it compile test application to probe for features, > unfortunatly configure still need LIBS varibale to be set. Please add this information to the commit log. > > > +ifeq ($(BR2_PACKAGE_GETTEXT),y) > > > +GIT_DEPENDENCIES += gettext > > > +GIT_CONF_ENV += ac_cv_lib_c_gettext=no > > > +endif > > > > Are you sure this fixes a static build issue? If not, please make a separate > > patch for this part. > > When statically compiled with gettext support libcurl need to be linked > with libintl, using ac_cv_lib_c_gettext does the job. So for me its > static linking related. So this is only needed when libcurl is enabled? Please add this information as well to the commit log, and add a comment in the code. baruch
diff --git a/package/git/git.mk b/package/git/git.mk index 2bd6a01..942f29f 100644 --- a/package/git/git.mk +++ b/package/git/git.mk @@ -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) else GIT_CONF_OPTS += --without-openssl endif @@ -55,9 +55,20 @@ else GIT_CONF_OPTS += --without-tcltk endif +ifeq ($(BR2_PACKAGE_GETTEXT),y) +GIT_DEPENDENCIES += gettext +GIT_CONF_ENV += ac_cv_lib_c_gettext=no +endif + +ifeq ($(BR2_PACKAGE_ZLIB),y) +GIT_DEPENDENCIES += zlib +GIT_CONF_ENV_LIBS += -lz +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)'
Add missing ssl, zlib and gettext library for static compilation support. 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 gettext and zlib symbols. Signed-off-by: Remi Pommarel <repk@triplefau.lt> --- package/git/git.mk | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)