From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ricardo Wurmus Subject: Re: [PATCH v2 3/3] gnu: Add openocd. Date: Fri, 28 Oct 2016 08:14:44 +0200 Message-ID: <87h97xf19n.fsf@elephly.net> References: <20161026210807.27390-1-theodoros.for@openmailbox.org> <20161026210807.27390-4-theodoros.for@openmailbox.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:57372) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c00RP-0007sG-IN for guix-devel@gnu.org; Fri, 28 Oct 2016 02:15:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c00RK-0002nb-LK for guix-devel@gnu.org; Fri, 28 Oct 2016 02:14:59 -0400 Received: from sender163-mail.zoho.com ([74.201.84.163]:21439) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c00RK-0002mc-DM for guix-devel@gnu.org; Fri, 28 Oct 2016 02:14:54 -0400 In-reply-to: <20161026210807.27390-4-theodoros.for@openmailbox.org> 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: Theodoros Foradis Cc: guix-devel@gnu.org 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’d prefer packaging proper releases of libjaylink and openocd, if that’s the case. > +(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 "0p8rcqhkx3f29j08w33fkp8xnzj4xxa41lzdfq5wd1i4x8s07s0p")) > + (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" "osbdm" > + "parport" "aice" "cmsis-dap" "dummy" "jtag_vpi" "remote-bitbang" > + "rlink" "stlink" "ti-icdi" "ulink" "usbprog" "vsllink" > + "usb-blaster-2" "usb_blaster" "presto" "openjtag"))) > + #:phases > + (modify-phases %standard-phases > + (add-before 'configure 'autoreconf > + (lambda _ > + (zero? (system* "autoreconf" "-vfi")))) > + (add-after 'autoreconf 'patch-configure > + (lambda _ > + (substitute* "configure" > + (("SHELL = /bin/sh") (string-append "SHELL = " > (which "sh")))) Is this really necessary? Or can we just pass “SHELL” as a configure flag to override? > + (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’t clear to me. What happens here? Why is the additional configure flag needed? Could you add a comment as to the intent of this substitution? > + #t))))) > + (inputs > + `(("hidapi" ,hidapi) > + ("libftdi" ,libftdi) > + ("libusb" ,libusb) > + ("libusb-compat" ,libusb-compat))) Both libusb AND libusb-compat? > + (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’s what OpenOCD stands for, but everything is free software (or “open source”) in Guix anyway, so we usually don’t mention “free” or “open”. > + (description > + "OpenOCD provides on-chip programming and debugging support with a > +layered architecture of JTAG interface and TAP support.") > + (license (list license:gpl2 ;; openocd and git2cl submodule > + license:gpl2+ ;; libjaylink submodule > + license:bsd-2))))) ;; jimctl submodule Please use a single “;” for margin comments like this. > diff --git a/gnu/packages/patches/openocd-nrf52.patch b/gnu/packages/patches/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 from > +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 in <…>, so that the space between URL and comma can be removed. ~~ Ricardo