Theodoros Foradis <theodoros....@openmailbox.org> 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
