From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ricardo Wurmus Subject: Re: [PATCH] Add LDC. Date: Tue, 5 Jan 2016 11:06:26 +0100 Message-ID: References: <87ziwtec5x.fsf@gnu.org> <87si2dfmpl.fsf@gnu.org> 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]:58779) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aGOVh-0000Ot-QR for guix-devel@gnu.org; Tue, 05 Jan 2016 05:06:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aGOVe-0002eL-I0 for guix-devel@gnu.org; Tue, 05 Jan 2016 05:06:37 -0500 In-Reply-To: <87si2dfmpl.fsf@gnu.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-bounces+gcggd-guix-devel=m.gmane.org@gnu.org To: Roel Janssen Cc: guix-devel@gnu.org Roel Janssen 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 archite= ctures are > + (arguments ; not supported= (yet). This comment would better be placed above the =E2=80=98(supported-systems= ...)=E2=80=99 line. Having it be part of the =E2=80=98(arguments=E2=80=99 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=3D1"))))) > + (add-after 'unpack 'unpack-druntime-source > + (lambda* (#:key inputs #:allow-other-keys) > + (with-directory-excursion "runtime/druntime" > + (zero? (system* "tar" "xvzf" (assoc-ref inputs "drunt= ime-src") > + "--strip-components=3D1"))))) > + (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=3D1"))))) 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=3D1")))= ))) (and (unpack "phobos-src" "runtime/phobos") (unpack "druntime-src" "runtime/druntime") (unpack "dmd-testsuite-src" "tests/d2/dmd-testsuite"))= ))) It=E2=80=99s just a matter of taste, but I really find that your proposed= three phases look rather noisy with lots of boilerplate, which I don=E2=80=99t = think needs repeating. > + (add-after > + 'unpack-phobos-source 'patch-phobos Please pull the symbols onto the same line as =E2=80=9Cadd-after=E2=80=9D= . > + (lambda* (#:key inputs #:allow-other-keys) > + (begin =E2=80=9Cbegin=E2=80=9D is not needed in =E2=80=9Clambda=E2=80=9D. > + (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/zo= neinfo"))) > + (substitute* "tests/d2/dmd-testsuite/Makefile" > + (("/bin/bash") (which "bash")))) > + #t)) > + (add-after 'unpack-dmd-testsuite-source 'patch-dmd-testsuite > + (lambda _ > + #t))))) I don=E2=80=99t 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/ld= c-v" > + version ".tar.gz")) > + (sha256 > + (base32 > + "0z4mkyddx6c4sy1vqgqvavz55083dsxws681qkh93jh1rpby9yg6")))) > + ("dmd-testsuite-src" > + ,(origin > + (method url-fetch) > + (uri (string-append > + "https://github.com/ldc-developers/dmd-testsuite/archi= ve/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 o= n 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 =E2=80=98(license ...=E2=80=99 line (with double semicolon). I don=E2=80=99t understand the comment. What exceptions apply to the DMD frontend? What does =E2=80=9C(custom)=E2=80=9D 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 =E2=80=9Cwhatever=E2=80=9D l= icense. (license (list license:bsd-3 license:gpl2+ license:whatever-custom-is)) If there is no matching license value for =E2=80=9Ccustom=E2=80=9D you ca= n use =E2=80=9C(license:non-copyleft uri)=E2=80=9D, where =E2=80=9Curi=E2=80=9D= is a string holding the URL where the license can be read. I think with these changes it=E2=80=99s okay. ~~ Ricardo