[Buildroot] [PATCH] package/android-tools: fix static build failure due to missing -latomic

[Buildroot] [PATCH] package/android-tools: fix static build failure due to missing -latomic

[Giulio Benetti]

During static linking build fails because -lcrypto doesn't inherit its
dependency like -latomic. So need to fix Makefile before building.

Add check in package recipe for TOOLCHAIN_HAS_LIBATOMIC. If toolchain
has libatomic use sed to append -latomic at the end of libs list in
adbd.mk makefile.

Fixes:
http://autobuild.buildroot.net/results/d75/d75a6cf99f58fb5f42abaf9d54cde28224bc44fb/

Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 package/android-tools/android-tools.mk | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/package/android-tools/android-tools.mk b/package/android-tools/android-tools.mk
index 6f6ca7729b..cd06afa2c2 100644
--- a/package/android-tools/android-tools.mk
+++ b/package/android-tools/android-tools.mk
@@ -12,6 +12,16 @@ HOST_ANDROID_TOOLS_EXTRA_DOWNLOADS = $(ANDROID_TOOLS_EXTRA_DOWNLOADS)
 ANDROID_TOOLS_LICENSE = Apache-2.0
 ANDROID_TOOLS_LICENSE_FILES = debian/copyright
 
+# Uses __atomic_fetch_add_4. In adbd.mk Makefile there is no hunk to the end of
+# linking command. So need to add -latomic to LIBS using sed to provide
+# -latomic at the correct place in the linking command.
+ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
+define ANDROID_TOOLS_FIX_MAKEFILE
+	$(SED) 's/-lz -lcrypt/-lz -lcrypt -latomic/' $(@D)/debian/makefiles/adbd.mk
+endef
+ANDROID_TOOLS_POST_PATCH_HOOKS = ANDROID_TOOLS_FIX_MAKEFILE
+endif
+
 # Extract the Debian tarball inside the sources
 define ANDROID_TOOLS_DEBIAN_EXTRACT
 	$(call suitable-extractor,$(notdir $(ANDROID_TOOLS_EXTRA_DOWNLOADS))) \
-- 
2.17.1

[Buildroot] [PATCH] package/android-tools: fix static build failure due to missing -latomic

[Thomas Petazzoni]

Hello Giulio,

On Thu,  4 Apr 2019 17:39:04 +0200
Giulio Benetti <giulio.benetti@micronovasrl.com> wrote:

> +# Uses __atomic_fetch_add_4. In adbd.mk Makefile there is no hunk to the end of
> +# linking command. So need to add -latomic to LIBS using sed to provide
> +# -latomic at the correct place in the linking command.
> +ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
> +define ANDROID_TOOLS_FIX_MAKEFILE
> +	$(SED) 's/-lz -lcrypt/-lz -lcrypt -latomic/' $(@D)/debian/makefiles/adbd.mk
> +endef
> +ANDROID_TOOLS_POST_PATCH_HOOKS = ANDROID_TOOLS_FIX_MAKEFILE
> +endif

Instead of this, please do regular patch on adbd.mk that does this:

-LIBS+= -lc -lpthread -lz -lcrypto -lcrypt `pkg-config --libs glib-2.0 gio-2.0`
+LIBS+= -lc -lpthread -lz `pkg-config --libs libcrypto` -lcrypt `pkg-config --libs glib-2.0 gio-2.0`

and that's it.

Also, adding host-pkgconf to ANDROID_TOOLS_DEPENDENCIES when adbd is
selected would be necessary (but it should already be needed anyway, as
pkg-config is already used to detect glib).

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] package/android-tools: fix static build failure due to missing -latomic

[Giulio Benetti]

Hello Thomas,

Il 05/04/2019 00:32, Giulio Benetti ha scritto:
> Hello
> 
> Il 04/04/2019 22:45, Thomas Petazzoni ha scritto:
>> Hello Giulio,
>>
>> On Thu,  4 Apr 2019 17:39:04 +0200
>> Giulio Benetti <giulio.benetti@micronovasrl.com> wrote:
>>
>>> +# Uses __atomic_fetch_add_4. In adbd.mk Makefile there is no hunk to the end of
>>> +# linking command. So need to add -latomic to LIBS using sed to provide
>>> +# -latomic at the correct place in the linking command.
>>> +ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
>>> +define ANDROID_TOOLS_FIX_MAKEFILE
>>> +	$(SED) 's/-lz -lcrypt/-lz -lcrypt -latomic/' $(@D)/debian/makefiles/adbd.mk
>>> +endef
>>> +ANDROID_TOOLS_POST_PATCH_HOOKS = ANDROID_TOOLS_FIX_MAKEFILE
>>> +endif
>>
>> Instead of this, please do regular patch on adbd.mk that does this:
>>
>> -LIBS+= -lc -lpthread -lz -lcrypto -lcrypt `pkg-config --libs glib-2.0 gio-2.0`

In current version there is not glib-2.0 or gio-2.0, do I add them 
anyway as follows?

>> +LIBS+= -lc -lpthread -lz `pkg-config --libs libcrypto` -lcrypt `pkg-config --libs glib-2.0 gio-2.0`
>>
>> and that's it.
>>
>> Also, adding host-pkgconf to ANDROID_TOOLS_DEPENDENCIES when adbd is
>> selected would be necessary (but it should already be needed anyway, as
>> pkg-config is already used to detect glib).

Sure, I've checked it's needed otherwise my distro's pkgconfig will be 
used(checked it to understand how it works).

Thanks
-- 
Giulio Benetti
CTO

MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale ? 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642

[Buildroot] [PATCH] package/android-tools: fix static build failure due to missing -latomic

[Thomas Petazzoni]

Hello Giulio,

On Sun, 7 Apr 2019 23:55:38 +0200
Giulio Benetti <giulio.benetti@micronovasrl.com> wrote:

> >> -LIBS+= -lc -lpthread -lz -lcrypto -lcrypt `pkg-config --libs glib-2.0 gio-2.0`  
> 
> In current version there is not glib-2.0 or gio-2.0, do I add them 
> anyway as follows?

Wow, I got confused about this. pkg-config is used for glib-2.0 and
gio-2.0 in the upstream android-tools code, but we remove that in our
0002-Fix-adbd-for-non-Ubuntu-systems.patch patch.

So of course, don't re-add those pkg-config calls for glib-2.0 and gio-2.0.

However, use pkg-config for OpenSSL. This will also allow you to drop
patch 0005-fix-static-link-zlib.patch, which was needed for the same
reason: pkg-config is not used to find OpenSSL.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] package/android-tools: fix static build failure due to missing -latomic

[Giulio Benetti]

Il 08/04/2019 09:18, Thomas Petazzoni ha scritto:
> Hello Giulio,
> 
> On Sun, 7 Apr 2019 23:55:38 +0200
> Giulio Benetti <giulio.benetti@micronovasrl.com> wrote:
> 
>>>> -LIBS+= -lc -lpthread -lz -lcrypto -lcrypt `pkg-config --libs glib-2.0 gio-2.0`
>>
>> In current version there is not glib-2.0 or gio-2.0, do I add them
>> anyway as follows?
> 
> Wow, I got confused about this. pkg-config is used for glib-2.0 and
> gio-2.0 in the upstream android-tools code, but we remove that in our
> 0002-Fix-adbd-for-non-Ubuntu-systems.patch patch.
> 
> So of course, don't re-add those pkg-config calls for glib-2.0 and gio-2.0.

Ok

> However, use pkg-config for OpenSSL. This will also allow you to drop
> patch 0005-fix-static-link-zlib.patch, which was needed for the same
> reason: pkg-config is not used to find OpenSSL.

Oh yes.
In this case, can I substitute "0005-fix-static-link-zlib.patch" with 
the new one I'm going to create? I mean "0005-...patch" ?

Thanks

-- 
Giulio Benetti
CTO

MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale ? 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642

[Buildroot] [PATCH] package/android-tools: fix static build failure due to missing -latomic

[Thomas Petazzoni]

On Mon, 8 Apr 2019 09:43:53 +0200
Giulio Benetti <giulio.benetti@micronovasrl.com> wrote:

> In this case, can I substitute "0005-fix-static-link-zlib.patch" with 
> the new one I'm going to create? I mean "0005-...patch" ?

Yes, sure.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] package/android-tools: fix static linking failure due to lack of deps

[Giulio Benetti]

When static linking some dependency library can be missing(i.e. -latomic
for -lcrypto) on linking libraries list. This is because when static
linking libraries dependencies are not transparently linked into binary.

To avoid moving libraries before/after one another or add new ones
that are not needed at all in some linking case use `pkg-config --libs
LIBRARY` where LIBRARY is the library we "probe" for its existence and
dependency.
So:
- Remove 0005-fix-static-link-zlib.patch where -lcrypto and -lz were
swapped
- Replace it with 0005-Use-pkgconf-to-get-libs-deps.patch where -lcrypto
has been substituted with `pkg-config --libs libcrypto`
- Add host-pkgconf to ANDROID_TOOLS_DEPENDENCIES

Fixes:
http://autobuild.buildroot.net/results/d3d/d3d6679cfc8afe4467368bd3d31483172c1032de/

Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 .../0005-Use-pkgconf-to-get-libs-deps.patch   | 35 ++++++++++++++++++
 .../0005-fix-static-link-zlib.patch           | 36 -------------------
 package/android-tools/android-tools.mk        |  1 +
 3 files changed, 36 insertions(+), 36 deletions(-)
 create mode 100644 package/android-tools/0005-Use-pkgconf-to-get-libs-deps.patch
 delete mode 100644 package/android-tools/0005-fix-static-link-zlib.patch

diff --git a/package/android-tools/0005-Use-pkgconf-to-get-libs-deps.patch b/package/android-tools/0005-Use-pkgconf-to-get-libs-deps.patch
new file mode 100644
index 0000000000..63ea1fb6a4
--- /dev/null
+++ b/package/android-tools/0005-Use-pkgconf-to-get-libs-deps.patch
@@ -0,0 +1,35 @@
+makefiles: use pkgconf to get libs deps
+
+LIBS lists library dependencies without taking into account static linking
+that need ordered listing and more libraries listed since differently from
+shared linking dependency is not transparent(i.e. -lcrypto could need
+-latomic etc.).
+
+Replace -lcrypto with `pkg-config --libs libcrypto` command to be sure all
+needed libraries are listed during linking.
+
+Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
+---
+diff -urpN android-tools-4.2.2+git20130218.orig/debian/makefiles/adbd.mk android-tools-4.2.2+git20130218/debian/makefiles/adbd.mk
+--- android-tools-4.2.2+git20130218.orig/debian/makefiles/adbd.mk	2019-04-08 16:05:02.967710428 +0200
++++ android-tools-4.2.2+git20130218/debian/makefiles/adbd.mk	2019-04-08 16:30:42.463084426 +0200
+@@ -44,7 +44,7 @@ CPPFLAGS+= -DADBD_NON_ANDROID
+ CPPFLAGS+= -I$(SRCDIR)/core/adbd
+ CPPFLAGS+= -I$(SRCDIR)/core/include
+
+-LIBS+= -lc -lpthread -lz -lcrypto -lcrypt
++LIBS+= -lc -lpthread -lz `pkg-config --libs libcrypto` -lcrypt
+
+ OBJS= $(patsubst %, %.o, $(basename $(SRCS)))
+
+diff -urpN android-tools-4.2.2+git20130218.orig/debian/makefiles/adb.mk android-tools-4.2.2+git20130218/debian/makefiles/adb.mk
+--- android-tools-4.2.2+git20130218.orig/debian/makefiles/adb.mk	2019-04-08 16:05:02.959701400 +0200
++++ android-tools-4.2.2+git20130218/debian/makefiles/adb.mk	2019-04-08 16:31:06.529426250 +0200
+@@ -41,7 +41,7 @@ CPPFLAGS+= -DHAVE_TERMIO_H
+ CPPFLAGS+= -I$(SRCDIR)/core/adb
+ CPPFLAGS+= -I$(SRCDIR)/core/include
+
+-LIBS+= -lc -lpthread -lz -lcrypto
++LIBS+= -lc -lpthread -lz `pkg-config --libs libcrypto`
+
+ OBJS= $(SRCS:.c=.o)
diff --git a/package/android-tools/0005-fix-static-link-zlib.patch b/package/android-tools/0005-fix-static-link-zlib.patch
deleted file mode 100644
index dff4df6e79..0000000000
--- a/package/android-tools/0005-fix-static-link-zlib.patch
+++ /dev/null
@@ -1,36 +0,0 @@
-Fix static linking of adb/adbd
-
-Both adb and adbd use OpenSSL, which indirectly uses zlib. Since
-adb/adbd also use zlib directly -lz is included in the linker flags,
-but not at the right position to ensure that static linking works: to
-make it possible for OpenSSL symbols to see zlib symbols, -lz must
-appear after -lcrypto.
-
-Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
-
-Index: b/debian/makefiles/adb.mk
-===================================================================
---- a/debian/makefiles/adb.mk
-+++ b/debian/makefiles/adb.mk
-@@ -41,7 +41,7 @@
- CPPFLAGS+= -I$(SRCDIR)/core/adb
- CPPFLAGS+= -I$(SRCDIR)/core/include
- 
--LIBS+= -lc -lpthread -lz -lcrypto
-+LIBS+= -lc -lpthread -lcrypto -lz
- 
- OBJS= $(SRCS:.c=.o)
- 
-Index: b/debian/makefiles/adbd.mk
-===================================================================
---- a/debian/makefiles/adbd.mk
-+++ b/debian/makefiles/adbd.mk
-@@ -44,7 +44,7 @@
- CPPFLAGS+= -I$(SRCDIR)/core/adbd
- CPPFLAGS+= -I$(SRCDIR)/core/include
- 
--LIBS+= -lc -lpthread -lz -lcrypto -lcrypt
-+LIBS+= -lc -lpthread -lcrypto -lz -lcrypt
- 
- OBJS= $(patsubst %, %.o, $(basename $(SRCS)))
- 
diff --git a/package/android-tools/android-tools.mk b/package/android-tools/android-tools.mk
index 6f6ca7729b..3a63139014 100644
--- a/package/android-tools/android-tools.mk
+++ b/package/android-tools/android-tools.mk
@@ -11,6 +11,7 @@ ANDROID_TOOLS_EXTRA_DOWNLOADS = android-tools_$(ANDROID_TOOLS_VERSION)-3ubuntu41
 HOST_ANDROID_TOOLS_EXTRA_DOWNLOADS = $(ANDROID_TOOLS_EXTRA_DOWNLOADS)
 ANDROID_TOOLS_LICENSE = Apache-2.0
 ANDROID_TOOLS_LICENSE_FILES = debian/copyright
+ANDROID_TOOLS_DEPENDENCIES = host-pkgconf
 
 # Extract the Debian tarball inside the sources
 define ANDROID_TOOLS_DEBIAN_EXTRACT
-- 
2.17.1

[Buildroot] [PATCH] package/android-tools: fix static linking failure due to lack of deps

[Thomas Petazzoni]

On Mon,  8 Apr 2019 17:43:11 +0200
Giulio Benetti <giulio.benetti@micronovasrl.com> wrote:

> When static linking some dependency library can be missing(i.e. -latomic
> for -lcrypto) on linking libraries list. This is because when static
> linking libraries dependencies are not transparently linked into binary.
> 
> To avoid moving libraries before/after one another or add new ones
> that are not needed at all in some linking case use `pkg-config --libs
> LIBRARY` where LIBRARY is the library we "probe" for its existence and
> dependency.
> So:
> - Remove 0005-fix-static-link-zlib.patch where -lcrypto and -lz were
> swapped
> - Replace it with 0005-Use-pkgconf-to-get-libs-deps.patch where -lcrypto
> has been substituted with `pkg-config --libs libcrypto`
> - Add host-pkgconf to ANDROID_TOOLS_DEPENDENCIES
> 
> Fixes:
> http://autobuild.buildroot.net/results/d3d/d3d6679cfc8afe4467368bd3d31483172c1032de/
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
>  .../0005-Use-pkgconf-to-get-libs-deps.patch   | 35 ++++++++++++++++++
>  .../0005-fix-static-link-zlib.patch           | 36 -------------------
>  package/android-tools/android-tools.mk        |  1 +
>  3 files changed, 36 insertions(+), 36 deletions(-)
>  create mode 100644 package/android-tools/0005-Use-pkgconf-to-get-libs-deps.patch
>  delete mode 100644 package/android-tools/0005-fix-static-link-zlib.patch

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] package/android-tools: fix static linking failure due to lack of deps

[Peter Korsgaard]

>>>>> "Giulio" == Giulio Benetti <giulio.benetti@micronovasrl.com> writes:

 > When static linking some dependency library can be missing(i.e. -latomic
 > for -lcrypto) on linking libraries list. This is because when static
 > linking libraries dependencies are not transparently linked into binary.

 > To avoid moving libraries before/after one another or add new ones
 > that are not needed at all in some linking case use `pkg-config --libs
 > LIBRARY` where LIBRARY is the library we "probe" for its existence and
 > dependency.
 > So:
 > - Remove 0005-fix-static-link-zlib.patch where -lcrypto and -lz were
 > swapped
 > - Replace it with 0005-Use-pkgconf-to-get-libs-deps.patch where -lcrypto
 > has been substituted with `pkg-config --libs libcrypto`
 > - Add host-pkgconf to ANDROID_TOOLS_DEPENDENCIES

 > Fixes:
 > http://autobuild.buildroot.net/results/d3d/d3d6679cfc8afe4467368bd3d31483172c1032de/

 > Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>

Committed to 2019.02.x, thanks.

-- 
Bye, Peter Korsgaard