GNU bug report logs - #26522
gnu: Add adb

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 26522 in the body.
You can then email your comments to 26522 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Julien Lepiller <julien <at> lepiller.eu>
To: guix-patches <at> gnu.org
Subject: gnu: Add adb
Date: Sat, 15 Apr 2017 18:31:46 +0200

[Message part 1 (text/plain, inline)]

Hi guix,

here is an updated version of Marius' patch mixed with my own. It
updates our patches to the latest 7_1_2.rc6, adapts to some changes
since last version, and fixes the build of adb.

[0001-gnu-Add-adb.patch (text/x-patch, attachment)]

Message #8 received at 26522 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Julien Lepiller <julien <at> lepiller.eu>, 26522 <at> debbugs.gnu.org
Subject: Re: bug#26522: gnu: Add adb
Date: Sat, 15 Apr 2017 20:02:52 +0200

[Message part 1 (text/plain, inline)]

Julien Lepiller <julien <at> lepiller.eu> writes:

> Hi guix,
>
> here is an updated version of Marius' patch mixed with my own. It
> updates our patches to the latest 7_1_2.rc6, adapts to some changes
> since last version, and fixes the build of adb.

Wahoo! :)

I was *just* thinking yesterday that I should fix up this package ASAP.
Thanks a lot for beating me to it!

[...]

> +(define liblog
> +  (package
> +    (name "liblog")
> +    (version (android-platform-version))
> +    (source (android-platform-system-core version))
> +    (build-system gnu-build-system)
> +    (arguments
> +     `(#:tests? #f ; TODO.
> +       #:make-flags '("CC=gcc")
> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'enter-source
> +           (lambda _ (chdir "liblog") #t))
> +         (add-after 'enter-source 'create-Makefile
> +           (lambda _
> +             ;; No useful makefile is shipped, so we create one.
> +             (with-output-to-file "Makefile"
> +               (lambda _
> +                 (display
> +                  (string-append
> +                   "NAME = liblog\n"
> +                   "SOURCES = log_event_list.c log_event_write.c"
> +                   " logger_write.c config_write.c logger_name.c"
> +                   " logger_lock.c fake_log_device.c fake_writer.c"
> +                   " event_tag_map.c\n"
> +
> +                   "CFLAGS += -fvisibility=hidden -fPIC\n"
> +                   "CPPFLAGS += -I../include -DFAKE_LOG_DEVICE=1"
> +                   ;; Keep these two in sync with "liblog/Android.bp".
> +                   " -DLIBLOG_LOG_TAG=1005"
> +                   " -DSNET_EVENT_LOG_TAG=1397638484\n"
> +                   "LDFLAGS += -shared -Wl,-soname,$(NAME).so.0 -lpthread\n"
> +
> +                   "build: $(SOURCES)\n"
> +                   "	$(CC) $^ -o $(NAME).so.0 $(CFLAGS) $(CPPFLAGS) $(LDFLAGS)\n"))))))

I'm actually not sure what the return value of "display" is and the
Guile manual does not mention. I'd end this on a #t just to be "safe".

> +         (delete 'configure)
> +         (replace 'install
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (let* ((out (assoc-ref outputs "out"))
> +                    (lib (string-append out "/lib")))
> +               (install-file "liblog.so.0" lib)
> +               (with-directory-excursion lib
> +                 (symlink "liblog.so.0" "liblog.so"))
> +               #t))))))
> +    (home-page "https://developer.android.com/")
> +    (synopsis "Logging library from the Android platform.")
> +    (description "@code{liblog} represents an interface to the volatile Android
> +Logging system for NDK (Native) applications and libraries and contain
> +interfaces for either writing or reading logs.  The log buffers are divided up
> +in Main, System, Radio and Events sub-logs.")
> +    (license license:asl2.0)))
> +
> +(define libbase
> +  (package
> +    (name "libbase")
> +    (version (android-platform-version))
> +    (source (origin
> +              (inherit (android-platform-system-core version))
> +              (patches
> +               (search-patches "libbase-use-own-logging.patch"
> +                               "libbase-fix-includes.patch"))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     `(#:tests? #f ; TODO.
> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'enter-source
> +           (lambda _ (chdir "base") #t))
> +         (add-after 'enter-source 'create-Makefile
> +           (lambda _
> +             ;; No useful makefile is shipped, so we create one.
> +             (with-output-to-file "Makefile"
> +               (lambda _
> +                 (display
> +                  (string-append
> +                   "NAME = libbase\n"
> +                   "SOURCES = file.cpp logging.cpp parsenetaddress.cpp"
> +                   " stringprintf.cpp strings.cpp errors_unix.cpp\n"
> +
> +                   "CXXFLAGS += -std=gnu++11 -fPIC\n"
> +                   "CPPFLAGS += -Iinclude -I../include\n"
> +                   "LDFLAGS += -shared -Wl,-soname,$(NAME).so.0"
> +                   " -L.. -llog\n"
> +
> +                   "build: $(SOURCES)\n"
> +                   "	$(CXX) $^ -o $(NAME).so.0 $(CXXFLAGS) $(CPPFLAGS) $(LDFLAGS)\n"
> +                   ))))))

Same here, and also move the parens to the previous line. Possibly also
break it into two lines :)

> +         (delete 'configure)
> +         (replace 'install
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (let* ((out (assoc-ref outputs "out"))
> +                    (lib (string-append out "/lib")))
> +               (install-file "libbase.so.0" lib)
> +               (with-directory-excursion lib
> +                 (symlink "libbase.so.0" "libbase.so"))
> +               (copy-recursively "include" out)
> +               #t))))))
> +    (inputs `(("liblog" ,liblog)))
> +    (home-page "https://developer.android.com/")
> +    (synopsis "Android platform base library")
> +    (description "@code{libbase} is a library in common use by the
> +various Android core host applications.")
> +    (license license:asl2.0)))
> +
> +(define libcutils
> +  (package
> +    (name "libcutils")
> +    (version (android-platform-version))
> +    (source (android-platform-system-core version))
> +    (build-system gnu-build-system)
> +    (arguments
> +     `(#:tests? #f ; TODO.
> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'enter-source
> +           (lambda _ (chdir "libcutils") #t))
> +         (add-after 'enter-source 'create-Makefile
> +           (lambda _
> +             ;; No useful makefile is shipped, so we create one.
> +             (with-output-to-file "Makefile"
> +               (lambda _
> +                 (display
> +                  (string-append
> +                   "NAME = libcutils\n"
> +                   "SOURCES = load_file.o socket_local_client_unix.o"
> +                   " socket_loopback_client_unix.o socket_network_client_unix.o"
> +                   " socket_loopback_server_unix.o socket_local_server_unix.o"
> +                   " sockets_unix.o socket_inaddr_any_server_unix.o"
> +                   " sockets.o\n"
> +                   "CC = gcc\n"
> +
> +                   "CFLAGS += -fPIC\n"
> +                   "CXXFLAGS += -std=gnu++11 -fPIC\n"
> +                   "CPPFLAGS += -Iinclude -I../include\n"
> +                   "LDFLAGS += -shared -Wl,-soname,$(NAME).so.0\n"
> +
> +                   "build: $(SOURCES)\n"
> +                   "	$(CXX) $^ -o $(NAME).so.0 $(CXXFLAGS) $(CPPFLAGS) $(LDFLAGS)\n"
> +                   ))))))

...and here.

> +         (delete 'configure)
> +         (replace 'install
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (let* ((out (assoc-ref outputs "out"))
> +                    (lib (string-append out "/lib")))
> +               (install-file "libcutils.so.0" lib)
> +               (with-directory-excursion lib
> +                 (symlink "libcutils.so.0" "libcutils.so"))
> +               #t))))))
> +    (home-page "https://developer.android.com/")
> +    (synopsis "Android platform c utils library")
> +    (description "@code{libcutils} is a library in common use by the
> +various Android core host applications.")
> +    (license license:asl2.0)))
> +
> +(define-public adb
> +  (package
> +    (name "adb")
> +    (version (android-platform-version))
> +    (source (origin
> +              (inherit (android-platform-system-core version))
> +              (patches
> +               (search-patches "libbase-use-own-logging.patch"
> +                               "libbase-fix-includes.patch"))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     `(#:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'enter-source
> +           (lambda _ (chdir "adb") #t))
> +         (add-before 'build 'fix-clang
> +           (lambda _
> +             (substitute* "adb_client.h"
> +                   (("_Nonnull") "")
> +                   (("_Nullable") ""))))

Can you add a comment explaining what this does? From the phase name,
I'd guess it removes a clang dependency, but it's not clear how or why.
Also check indentation here and end with an explicit #t.

> +         (add-before 'build 'fix-main
> +           (lambda _
> +             (copy-file "client/main.cpp" "adb_main.cpp")))

This should also end on a #t. A comment would be nice too if you know
why it's needed, what an odd build step.

> +         (add-after 'enter-source 'create-Makefile
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             ;; No useful makefile is shipped, so we create one.
> +             (with-output-to-file "Makefile"
> +               (lambda _
> +                 (display
> +                  (string-append
> +                   ;; Common for all components.
> +                   "CXXFLAGS += -std=gnu++14 -fpermissive\n"
> +                   "CPPFLAGS += -I../include -I../base/include -I. -DADB_HOST=1 "
> +                   "-DADB_REVISION='\"" ,version "\"' -fPIC\n"
> +                   "LDFLAGS += -lcrypto -lpthread -lbase -lcutils -L. -ladb\n"
> +
> +                   ;; Libadb specifics.
> +                   "LIBADB_SOURCES = adb.cpp adb_auth.cpp adb_io.cpp "
> +                   "adb_listeners.cpp adb_trace.cpp adb_utils.cpp fdevent.cpp "
> +                   "sockets.cpp transport.cpp transport_local.cpp transport_usb.cpp "
> +                   "get_my_path_linux.cpp sysdeps_unix.cpp usb_linux.cpp "
> +                   "adb_auth_host.cpp diagnose_usb.cpp services.cpp "
> +                   "shell_service_protocol.cpp bugreport.cpp line_printer.cpp\n"
> +
> +                   "LIBADB_LDFLAGS += -shared -Wl,-soname,libadb.so.0 "
> +                   "-lcrypto -lpthread -lbase\n"
> +
> +                   ;; Adb specifics.
> +                   "ADB_SOURCES = adb_main.cpp console.cpp commandline.cpp "
> +                   "adb_client.cpp file_sync_client.cpp\n"
> +                   "ADB_LDFLAGS += -Wl,-rpath=" (assoc-ref outputs "out") "/lib\n"
> +
> +                   "build: libadb $(ADB_SOURCES)\n"
> +                   "	$(CXX) $(ADB_SOURCES) -o adb $(CXXFLAGS) $(CPPFLAGS) "
> +                   "$(ADB_LDFLAGS) $(LDFLAGS)\n"
> +
> +                   "libadb: $(LIBADB_SOURCES)\n"
> +                   "	$(CXX) $^ -o libadb.so.0 $(CXXFLAGS) $(CPPFLAGS) "
> +                   "$(LIBADB_LDFLAGS)\n"
> +                   "	ln -sv libadb.so.0 libadb.so\n"
> +                   ))))))

...lonely parens here too (`lint` should complain about this) :)

LGTM with these fixes! Thank you!

[signature.asc (application/pgp-signature, inline)]

Message #11 received at 26522 <at> debbugs.gnu.org (full text, mbox):

From: Julien Lepiller <julien <at> lepiller.eu>
To: 26522 <at> debbugs.gnu.org
Subject: Re: bug#26522: gnu: Add adb
Date: Sun, 16 Apr 2017 11:22:11 +0200

[Message part 1 (text/plain, inline)]

Hi Marius,

thanks for the quick review. This should fix your comments.

[0001-gnu-Add-adb.patch (text/x-patch, attachment)]

Message #14 received at 26522 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Julien Lepiller <julien <at> lepiller.eu>, 26522 <at> debbugs.gnu.org
Subject: Re: bug#26522: gnu: Add adb
Date: Sun, 16 Apr 2017 23:54:47 +0200

[Message part 1 (text/plain, inline)]

Julien Lepiller <julien <at> lepiller.eu> writes:

> Hi Marius,
>
> thanks for the quick review. This should fix your comments.

Great! One thing I overlooked last time was the patches. It would be
good to include the origin in URL form, so we can dig them out later.

The arch patch shouldn't be too hard to find (but URL would be
convenient!), I'm curious where the other patch comes from.

Patches tend to evolve over time, so checking what happened "upstream"
can be handy when you're stumped. See e.g. any CVE patch for examples :)

Other than that looks good!

[signature.asc (application/pgp-signature, inline)]

Message #17 received at 26522 <at> debbugs.gnu.org (full text, mbox):

From: Julien Lepiller <julien <at> lepiller.eu>
To: 26522 <at> debbugs.gnu.org
Subject: Re: bug#26522: gnu: Add adb
Date: Mon, 17 Apr 2017 10:40:29 +0200

[Message part 1 (text/plain, inline)]

Le Sun, 16 Apr 2017 23:54:47 +0200,
Marius Bakke <mbakke <at> fastmail.com> a écrit :

> Julien Lepiller <julien <at> lepiller.eu> writes:
> 
> > Hi Marius,
> >
> > thanks for the quick review. This should fix your comments.
> 
> Great! One thing I overlooked last time was the patches. It would be
> good to include the origin in URL form, so we can dig them out later.
> 
> The arch patch shouldn't be too hard to find (but URL would be
> convenient!), I'm curious where the other patch comes from.
> 
> Patches tend to evolve over time, so checking what happened "upstream"
> can be handy when you're stumped. See e.g. any CVE patch for
> examples :)
> 
> Other than that looks good!

Here is an updated patch with a link to archlinux' patch. You were the
one to add the other patch, and I can't find it. I'm also curious where
it comes from :p

[0001-gnu-Add-adb.patch (text/x-patch, attachment)]

Message #20 received at 26522 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Julien Lepiller <julien <at> lepiller.eu>, 26522 <at> debbugs.gnu.org
Subject: Re: bug#26522: gnu: Add adb
Date: Mon, 17 Apr 2017 17:09:04 +0200

[Message part 1 (text/plain, inline)]

Julien Lepiller <julien <at> lepiller.eu> writes:

> Le Sun, 16 Apr 2017 23:54:47 +0200,
> Marius Bakke <mbakke <at> fastmail.com> a écrit :
>
>> Julien Lepiller <julien <at> lepiller.eu> writes:
>> 
>> > Hi Marius,
>> >
>> > thanks for the quick review. This should fix your comments.
>> 
>> Great! One thing I overlooked last time was the patches. It would be
>> good to include the origin in URL form, so we can dig them out later.
>> 
>> The arch patch shouldn't be too hard to find (but URL would be
>> convenient!), I'm curious where the other patch comes from.
>> 
>> Patches tend to evolve over time, so checking what happened "upstream"
>> can be handy when you're stumped. See e.g. any CVE patch for
>> examples :)
>> 
>> Other than that looks good!
>
> Here is an updated patch with a link to archlinux' patch. You were the
> one to add the other patch, and I can't find it. I'm also curious where
> it comes from :p

Hehe...I deliberately did not add a reference just to prove this point!

...but I remember now, it is this commit, which I don't think will hit a
release until Android 8 if I've understood their branching correctly:

https://android.googlesource.com/platform/system/core/+/e5dd71a290f664d3f3bf0dd8a4bad411dc7ad416

Please add it and push! :D

[signature.asc (application/pgp-signature, inline)]

Message #23 received at 26522 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Marius Bakke <mbakke <at> fastmail.com>
Cc: Julien Lepiller <julien <at> lepiller.eu>, 26522 <at> debbugs.gnu.org
Subject: Re: bug#26522: gnu: Add adb
Date: Tue, 18 Apr 2017 10:21:13 +0200

Marius Bakke <mbakke <at> fastmail.com> skribis:

> Please add it and push! :D

Awesome, nice work gentlefolks!

Ludo’.

Message #28 received at 26522-done <at> debbugs.gnu.org (full text, mbox):

From: julien lepiller <julien <at> lepiller.eu>
To: 26522-done <at> debbugs.gnu.org
Date: Thu, 20 Apr 2017 10:33:11 +0200

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.