From: Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de>
To: Roel Janssen <roel@gnu.org>
Cc: guix-devel@gnu.org
Subject: Re: [PATCH] Add LDC.
Date: Tue, 5 Jan 2016 11:06:26 +0100 [thread overview]
Message-ID: <idjvb78gx25.fsf@bimsb-sys02.mdc-berlin.net> (raw)
In-Reply-To: <87si2dfmpl.fsf@gnu.org>
Roel Janssen <roel@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
next prev parent reply other threads:[~2016-01-05 10:06 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-29 11:18 [PATCH] Add LDC Roel Janssen
2015-12-29 15:02 ` Ricardo Wurmus
2015-12-29 15:37 ` Pjotr Prins
2016-01-04 14:23 ` Roel Janssen
2016-01-05 10:06 ` Ricardo Wurmus [this message]
2016-01-05 14:48 ` Roel Janssen
2016-01-05 15:22 ` Ricardo Wurmus
2016-01-05 18:46 ` Roel Janssen
2016-02-27 6:20 ` Pjotr Prins
2016-02-29 9:58 ` Ludovic Courtès
2015-12-29 15:42 ` Ricardo Wurmus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=idjvb78gx25.fsf@bimsb-sys02.mdc-berlin.net \
--to=ricardo.wurmus@mdc-berlin.de \
--cc=guix-devel@gnu.org \
--cc=roel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.