[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Add LDC.
From: |
Roel Janssen |
Subject: |
Re: [PATCH] Add LDC. |
Date: |
Tue, 05 Jan 2016 15:48:33 +0100 |
User-agent: |
mu4e 0.9.15; emacs 25.1.50.3 |
0001-gnu-Add-LDC-v2.patch
Description: Text Data
Hello Ricardo,
Thanks again for your time and helpful response. I hope this version of
the patch is fine.
Ricardo Wurmus writes:
> Roel Janssen <address@hidden> 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.
Ok.
>> + `(#: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.
After seeing how you would go about this I totally agree. I envisioned
three times the (with-directory-excursion) part, which wasn't much
better than what I had.
This looks really good. Thanks for your guidance!
>> + (add-after
>> + 'unpack-phobos-source 'patch-phobos
>
> Please pull the symbols onto the same line as “add-after”.
Ok.
>> + (lambda* (#:key inputs #:allow-other-keys)
>> + (begin
>
> “begin” is not needed in “lambda”.
Cool. I didn't know that, thanks!
>> + (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.
You're right.
>> + (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).
Done.
> 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.
It is the Boost Software License v1. So I peeked at boost.scm and
copied that license (license:x11-style ...).
The attached patch should be good, I believe. I hope the description
for the licenses is fine now. The LDC developers have just copied
source code files from GDC.
Thanks,
Roel Janssen