Roel Janssen <r...@gnu.org> writes:
> +(define-module (gnu packages ldc)
> + #:use-module ((guix licenses) #:prefix license:)
> + #:use-module (guix packages)
> + #:use-module (guix download)
> + #:use-module (guix build-system cmake)
> + #:use-module (gnu packages)
> + #:use-module (gnu packages base)
> + #:use-module (gnu packages libedit)
> + #:use-module (gnu packages llvm)
> + #:use-module (gnu packages textutils)
> + #:use-module (gnu packages zip))
> +
> +(define-public ldc
> + (package
> + (name "ldc")
> + (version "0.16.1")
> + (source (origin
> + (method url-fetch)
> + (uri (string-append
> + "https://github.com/ldc-developers/ldc/archive/v";
> + version ".tar.gz"))
> + (file-name (string-append name "-" version ".tar.gz"))
> + (sha256
> + (base32
> + "1jvilxx0rpqmkbja4m69fhd5g09697xq7vyqp2hz4hvxmmmv4j40"))))
> + (build-system cmake-build-system)
> + (supported-systems '("x86_64-linux" "i686-linux")) ; other architectures
> are
> + (arguments ; not supported (yet).
This comment would better be placed above the ‘(supported-systems...)’
line. Having it be part of the ‘(arguments’ line as well is not nice.
As a line comment it would then start with a double-semicolon.
> + `(#:phases
> + (modify-phases %standard-phases
> + (add-after 'unpack 'unpack-phobos-source
> + (lambda* (#:key inputs #:allow-other-keys)
> + (with-directory-excursion "runtime/phobos"
> + (zero? (system* "tar" "xvf" (assoc-ref inputs "phobos-src")
> + "--strip-components=1")))))
> + (add-after 'unpack 'unpack-druntime-source
> + (lambda* (#:key inputs #:allow-other-keys)
> + (with-directory-excursion "runtime/druntime"
> + (zero? (system* "tar" "xvzf" (assoc-ref inputs
> "druntime-src")
> + "--strip-components=1")))))
> + (add-after 'unpack 'unpack-dmd-testsuite-source
> + (lambda* (#:key inputs #:allow-other-keys)
> + (with-directory-excursion "tests/d2/dmd-testsuite"
> + (zero? (system* "tar" "xvzf"
> + (assoc-ref inputs "dmd-testsuite-src")
> + "--strip-components=1")))))
I still think that using one phase for unpacking additional tarballs
would totally suffice. Something like this, maybe:
(add-after 'unpack 'unpack-phobos-source
(lambda* (#:key inputs #:allow-other-keys)
(let ((unpack (lambda (source target)
(with-directory-excursion target
(zero? (system* "tar" "xvf"
(assoc-ref inputs source)
"--strip-components=1"))))))
(and (unpack "phobos-src" "runtime/phobos")
(unpack "druntime-src" "runtime/druntime")
(unpack "dmd-testsuite-src" "tests/d2/dmd-testsuite")))))
It’s just a matter of taste, but I really find that your proposed three
phases look rather noisy with lots of boilerplate, which I don’t think
needs repeating.
> + (add-after
> + 'unpack-phobos-source 'patch-phobos
Please pull the symbols onto the same line as “add-after”.
> + (lambda* (#:key inputs #:allow-other-keys)
> + (begin
“begin” is not needed in “lambda”.
> + (substitute* "runtime/phobos/std/process.d"
> + (("/bin/sh") (which "sh"))
> + (("echo") (which "echo")))
> + (substitute* "runtime/phobos/std/datetime.d"
> + (("/usr/share/zoneinfo/")
> + (string-append (assoc-ref inputs "tzdata")
> "/share/zoneinfo")))
> + (substitute* "tests/d2/dmd-testsuite/Makefile"
> + (("/bin/bash") (which "bash"))))
> + #t))
> + (add-after 'unpack-dmd-testsuite-source 'patch-dmd-testsuite
> + (lambda _
> + #t)))))
I don’t think this phase is needed.
> + (inputs
> + `(("libconfig" ,libconfig)
> + ("libedit" ,libedit)
> + ("tzdata" ,tzdata)))
> + (native-inputs
> + `(("llvm" ,llvm)
> + ("clang" ,clang)
> + ("unzip" ,unzip)
> + ("phobos-src"
> + ,(origin
> + (method url-fetch)
> + (uri (string-append
> + "https://github.com/ldc-developers/phobos/archive/ldc-v";
> + version ".tar.gz"))
> + (sha256
> + (base32
> + "0sgdj0536c4nb118yiw1f8lqy5d3g3lpg9l99l165lk9xy45l9z4"))
> + (patches (list (search-patch "ldc-disable-tests.patch")))))
> + ("druntime-src"
> + ,(origin
> + (method url-fetch)
> + (uri (string-append
> + "https://github.com/ldc-developers/druntime/archive/ldc-v";
> + version ".tar.gz"))
> + (sha256
> + (base32
> + "0z4mkyddx6c4sy1vqgqvavz55083dsxws681qkh93jh1rpby9yg6"))))
> + ("dmd-testsuite-src"
> + ,(origin
> + (method url-fetch)
> + (uri (string-append
> +
> "https://github.com/ldc-developers/dmd-testsuite/archive/ldc-v";
> + version ".tar.gz"))
> + (sha256
> + (base32
> + "0yc6miidzgl9k33ygk7xcppmfd6kivqj02cvv4fmkbs3qz4yy3z1"))))))
> + (home-page "http://wiki.dlang.org/LDC";)
> + (synopsis "LLVM compiler for the D programming language")
> + (description
> + "LDC is a compiler for the D programming language. It is based on the
> +latest DMD frontend and uses LLVM as backend.")
> + (license license:bsd-3))) ; with exceptions for the DMD frontend
> (custom) and code from GDC (GPLv2+)
This comment is too long for a margin comment. Better place it above
the ‘(license ...’ line (with double semicolon).
I don’t understand the comment. What exceptions apply to the DMD
frontend? What does “(custom)” mean? Is it a different license? If
this package contains code under different licenses it should be made
clear by providing a list of licenses:
;; Most of the code is released under BSD-3, except for code from
;; GDC (what is this?), which is released under GPLv2+, and the DMD
;; frontend, which is released under the “whatever” license.
(license (list license:bsd-3
license:gpl2+
license:whatever-custom-is))
If there is no matching license value for “custom” you can use
“(license:non-copyleft uri)”, where “uri” is a string holding the URL
where the license can be read.
I think with these changes it’s okay.
~~ Ricardo
