Subject: Re: [bug#28053] Add radare2

From: Ricardo Wurmus <[email-masked]>
Date: Wed, 16 Aug 2017 17:19:19 +0200

Hi Julien,

> Hi, these patches add radare2, a reversing framework.>From fa5478ac8eccf51a62af574ef2be6316ba5c247b Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <[email-masked]>
> Date: Fri, 11 Aug 2017 20:41:39 +0200
> Subject: [PATCH 1/3] gnu: Add capstone.
>
> * gnu/packages/engineering.scm (capstone): New variable.
> ---

Don’t forget to add a copyright line for yourself!

> +
> +(define-public capstone
> +  (package
> +    (name "capstone")
> +    (version "3.0.5-rc2")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append "https://github.com/aquynh/capstone/archive/"
> +                                  version ".tar.gz"))
> +              (file-name (string-append name "-" version ".tar.gz"))
> +              (sha256
> +               (base32
> +                "1cqms9r2p43aiwp5spd84zaccp16ih03r7sjhrv16nddahj0jz2q"))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     `(#:tests? #f
> +       #:make-flags (list (string-append "PREFIX=" %output)
> +                          "CC=gcc")
> +       #:phases
> +       (modify-phases %standard-phases
> +         (delete 'configure)
> +         (add-before 'build 'fix-cstool-ldflags
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (substitute* "cstool/Makefile"
> +               (("LDFLAGS =")
> +                (string-append "LDFLAGS = -Wl,-rpath=" (assoc-ref outputs "out")
> +                               "/lib"))))))))

Please add “#t” to the end of this phase.  Is it necessary to do this in
a build phase or could we add this to the make-flags instead?

>>>From 078c0cdedb620e7acdf9baea8924499ad5f8311a Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <[email-masked]>
> Date: Fri, 11 Aug 2017 20:49:04 +0200
> Subject: [PATCH 2/3] gnu: Add python-capstone.
>
> * gnu/packages/engineering.scm (python-capstone, python2-capstone): New
> variables.
> ---
>  gnu/packages/engineering.scm | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/packages/engineering.scm b/gnu/packages/engineering.scm
> index 8c6280c14..6ba5131e7 100644
> --- a/gnu/packages/engineering.scm
> +++ b/gnu/packages/engineering.scm
> @@ -32,7 +32,7 @@
>    #:use-module ((guix licenses) #:prefix license:)
>    #:use-module (guix build-system cmake)
>    #:use-module (guix build-system gnu)
> -  #:use-module (guix build-system cmake)
> +  #:use-module (guix build-system python)
>    #:use-module (gnu packages)
>    #:use-module (gnu packages algebra)
>    #:use-module (gnu packages autotools)
> @@ -1048,3 +1048,28 @@ framework.  Capstone can disassemble machine code for many supported architectur
>  such as x86, x86_64, arm, arm64, mips, ppc, sparc, sysz and xcore.  It provides
>  bindings for Python, Java, OCaml and more.")
>      (license license:bsd-3)))
> +
> +;; This package has a timestamp embedded in
> +;; lib/python3.5/site-packages/capstone/__pycache__/__iti__.cpython-35.pyc

Please add “FIXME: ” before the comment.

> +(define-public python-capstone
> +  (package
> +    (inherit capstone)
> +    (name "python-capstone")
> +    (propagated-inputs
> +     `(("capstone" ,capstone)))
> +    (build-system python-build-system)
> +    (arguments
> +     `(#:tests? #f

Please add a comment that tells us why the tests are disabled.

> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'chdir-and-fix-setup-py
> +           (lambda _
> +             (chdir "bindings/python")
> +             (substitute* "setup.py" ((".*   build_libraries.*") ""))
> +             (substitute* "capstone/__init__.py"
> +               (("pkg_resources.resource_filename.*")
> +                (string-append "'" (assoc-ref %build-inputs "capstone") "/lib',\n")))
> +             #t)))))))

Could you please add a comment explaining why these substitutions are
necessary?

>>>From ad50754454fdda9d4be91f6bc739df07fe63ecb6 Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <[email-masked]>
> Date: Fri, 11 Aug 2017 20:54:40 +0200
> Subject: [PATCH 3/3] gnu: Add radare2.
>
> * gnu/packages/engineering.scm (radare2): New variable.
> ---
>  gnu/packages/engineering.scm | 49 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
>
> diff --git a/gnu/packages/engineering.scm b/gnu/packages/engineering.scm
> index 6ba5131e7..130703e93 100644
> --- a/gnu/packages/engineering.scm
> +++ b/gnu/packages/engineering.scm
> @@ -1073,3 +1073,52 @@ bindings for Python, Java, OCaml and more.")
>
>  (define-public python2-capstone
>    (package-with-python2 python-capstone))
> +
> +(define-public radare2
> +  (package
> +    (name "radare2")
> +    (version "1.6.0")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append "http://radare.mikelloc.com/get/" version "/"
> +                                  name "-" version ".tar.gz"))
> +              (sha256
> +               (base32
> +                "16ggsk40zz6hyvclvqj1r4bh4hb78jf0d6ppry1jk4r0j30wm7cm"))
> +              (modules '((guix build utils)))
> +              (snippet
> +                '(begin
> +                  (substitute* "libr/asm/p/Makefile"
> +                    (("LDFLAGS\\+=") "LDFLAGS+=-Wl,-rpath=$(LIBDIR) "))
> +                  (substitute* "libr/parse/p/Makefile"
> +                    (("LDFLAGS\\+=") "LDFLAGS+=-Wl,-rpath=$(LIBDIR) "))
> +                  (substitute* "libr/bin/p/Makefile"
> +                    (("LDFLAGS\\+=") "LDFLAGS+=-Wl,-rpath=$(LIBDIR) "))))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     '(#:tests? #f

As above, please add a comment here.

> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-before 'configure 'mklibdir
> +           (lambda* (#:key inputs #:allow-other-keys)
> +             (mkdir-p (string-append (assoc-ref %outputs "out") "/lib")))))

Please use

        (add-before 'configure 'mklibdir
          (lambda* (#:key outputs #:allow-other-keys)
            (mkdir-p (string-append (assoc-ref outputs "out") "/lib"))
            #t)))

Why does this need to happen before the configure phase?  Can this be
done before the install phase?

> +       #:configure-flags
> +       (list "--with-sysmagic" "--with-syszip" "--with-openssl"
> +             "--without-nonpic" "--with-rpath" "--with-syscapstone")

Why “without-nonpic”?  Should we add support for nonpic at a later
point?

> +    (description
> +      "Radare project started as a forensics tool, a scriptable commandline
> +hexadecimal editor able to open disk files, but later support for analyzing
> +binaries, disassembling code, debugging programs, attaching to remote gdb
> +servers, ...")

The description could be improved.  The sentence is not complete as it’s
missing a verb.

> +    (license license:lgpl3)))

LGPL version 3 only?

--
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net