From mboxrd@z Thu Jan 1 00:00:00 1970 From: Theodoros Foradis Subject: Re: [PATCH v2 3/3] gnu: Add openocd. Date: Fri, 28 Oct 2016 20:36:03 +0300 Message-ID: <20161028173606.31099-1-theodoros.for@openmailbox.org> References: <87h97xf19n.fsf@elephly.net> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:41701) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c0B7y-0003lU-I1 for guix-devel@gnu.org; Fri, 28 Oct 2016 13:39:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c0B7u-0002Dd-HN for guix-devel@gnu.org; Fri, 28 Oct 2016 13:39:38 -0400 Received: from smtp13.openmailbox.org ([62.4.1.47]:51316) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1c0B7u-0002Ck-6n for guix-devel@gnu.org; Fri, 28 Oct 2016 13:39:34 -0400 In-Reply-To: <87h97xf19n.fsf@elephly.net> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: guix-devel@gnu.org Ricardo Wurmus writes: > Theodoros Foradis writes: > >> +;; We build openocd from git, because the JTAG library libjaylink >> +;; is not included in tarball releases. > > Is there a separate release of libjaylink? Does the git version bundle > libjaylink? I=E2=80=99d prefer packaging proper releases of libjaylink= and > openocd, if that=E2=80=99s the case. > The git version does not bundle libjaylink. It is included in a submodule. Should it be make clear in that initial comment? >> +(define-public openocd >> + (let* ((commit "674141e8a7a6413cb803d90c2a20150260015f81") >> + (revision "1")) >> + (package >> + (name "openocd") >> + (version (string-append "0.9.0-" revision "." >> + (string-take commit 7))) >> + (source (origin >> + (method git-fetch) >> + (uri (git-reference >> + (url (string-append "git://git.code.sf.net/p/" = name "/code.git")) >> + (commit commit) >> + (recursive? #t))) >> + (sha256 >> + (base32 "0p8rcqhkx3f29j08w33fkp8xnzj4xxa41lzdfq5wd1i= 4x8s07s0p")) >> + (file-name (string-append name "-" version "-checkout= .tar.xz")) >> + (patches >> + (search-patches "openocd-nrf52.patch")))) >> + (build-system gnu-build-system) >> + (arguments >> + '(#:configure-flags >> + (append (list "--disable-werror") >> + (map (lambda (programmer) >> + (string-append "--enable-" programmer)) >> + '("amtjtagaccel" "armjtagew" "buspirate" "ftdi" >> + "gw16012" "jlink" "oocd_trace" "opendous" "os= bdm" >> + "parport" "aice" "cmsis-dap" "dummy" "jtag_vp= i" "remote-bitbang" >> + "rlink" "stlink" "ti-icdi" "ulink" "usbprog" = "vsllink" >> + "usb-blaster-2" "usb_blaster" "presto" "openj= tag"))) >> + #:phases >> + (modify-phases %standard-phases >> + (add-before 'configure 'autoreconf >> + (lambda _ >> + (zero? (system* "autoreconf" "-vfi")))) >> + (add-after 'autoreconf 'patch-configure >> + (lambda _ >> + (substitute* "configure" >> + (("SHELL =3D /bin/sh") (string-append "SHELL =3D " >> (which "sh")))) > > Is this really necessary? Or can we just pass =E2=80=9CSHELL=E2=80=9D = as a configure > flag to override? > This is in fact not necessary, and removed. >> + (substitute* "configure" >> + (("srcdir/src/jtag/drivers/libjaylink/configure.gnu"= ) >> + (string-append "echo -e '#!" (which "sh") "\nexec \= "`dirname \"'\\$'0\"` >> +/configure\" --enable-subproject-build \"'\\$'@\"' > \" >> +$srcdir/src/jtag/drivers/libjaylink/configure.gnu\""))) > > This isn=E2=80=99t clear to me. What happens here? Why is the additio= nal > configure flag needed? Could you add a comment as to the intent of thi= s > substitution? > This substitution was changed, to substitute with an empty string. The actual intention of the substitution was to replace /bin/sh in the genera= ted configure.gnu, which proved to be unneeded, so it's just removed, and the file will be run with the correct /bin/sh. >> + #t))))) >> + (inputs >> + `(("hidapi" ,hidapi) >> + ("libftdi" ,libftdi) >> + ("libusb" ,libusb) >> + ("libusb-compat" ,libusb-compat))) > > Both libusb AND libusb-compat? > Yes, it won't build without both. >> + (native-inputs >> + `(("autoconf" ,autoconf) >> + ("automake" ,automake) >> + ("libtool" ,libtool) >> + ("pkg-config" ,pkg-config))) >> + (home-page "http://openocd.org") >> + (synopsis "Open On-Chip Debugger") > > s/Open// > > I know that that=E2=80=99s what OpenOCD stands for, but everything is f= ree > software (or =E2=80=9Copen source=E2=80=9D) in Guix anyway, so we usual= ly don=E2=80=99t mention > =E2=80=9Cfree=E2=80=9D or =E2=80=9Copen=E2=80=9D. > Understood. I removed it. >> + (description >> + "OpenOCD provides on-chip programming and debugging support wi= th a >> +layered architecture of JTAG interface and TAP support.") >> + (license (list license:gpl2 ;; openocd and git2cl submodu= le >> + license:gpl2+ ;; libjaylink submodule >> + license:bsd-2))))) ;; jimctl submodule > > Please use a single =E2=80=9C;=E2=80=9D for margin comments like this. > Done. >> diff --git a/gnu/packages/patches/openocd-nrf52.patch b/gnu/packages/p= atches/openocd-nrf52.patch >> new file mode 100644 >> index 0000000..d136735 >> --- /dev/null >> +++ b/gnu/packages/patches/openocd-nrf52.patch >> @@ -0,0 +1,843 @@ >> +This patch adds support for nRF52 series devices. It is patchset 7 fr= om >> +http://openocd.zylin.com/#/c/3511/ , which has been tested, but not >> +merged yet in master. > > Nitpick: please use two spaces between sentences and surround the URL i= n > <=E2=80=A6>, so that the space between URL and comma can be removed. > > ~~ Ricardo Changed this as well. Thanks for the information, and excuse me for ignoring those conventions. I reply to this with the updated patches. Theodoros Foradis (3): gnu: Add gdb-arm-none-eabi. gnu: Add hidapi. gnu: Add openocd. gnu/local.mk | 1 + gnu/packages/embedded.scm | 77 ++++++++ gnu/packages/libusb.scm | 38 ++++ gnu/packages/patches/openocd-nrf52.patch | 843 +++++++++++++++++++++++++= ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 959 insertions(+) --=20 Theodoros Foradis