Re: [PATCH] Add LDC.

[Roel Janssen <roel@gnu.org>]

[-- Attachment #1: 0001-gnu-Add-LDC-v2.patch --]
[-- Type: text/x-patch, Size: 10014 bytes --]

From 2a07b6ca221a6466d05fcc434331748b80dc3809 Mon Sep 17 00:00:00 2001
From: Roel Janssen <roel@gnu.org>
Date: Tue, 5 Jan 2016 15:43:50 +0100
Subject: [PATCH] gnu: Add LDC.

* gnu/packages/ldc.scm: New file.
* gnu/packages/patches/ldc-disable-tests.patch: New file.
* gnu-system.am (GNU_SYSTEM_MODULES): Add ldc.scm.
  (dist_patch_DATA): Add patch file.
---
 gnu-system.am                                |   2 +
 gnu/packages/ldc.scm                         | 118 +++++++++++++++++++++++++++
 gnu/packages/patches/ldc-disable-tests.patch |  90 ++++++++++++++++++++
 3 files changed, 210 insertions(+)
 create mode 100644 gnu/packages/ldc.scm
 create mode 100644 gnu/packages/patches/ldc-disable-tests.patch

diff --git a/gnu-system.am b/gnu-system.am
index 4788bc7..3dd49fe 100644
--- a/gnu-system.am
+++ b/gnu-system.am
@@ -174,6 +174,7 @@ GNU_SYSTEM_MODULES =				\
   gnu/packages/key-mon.scm			\
   gnu/packages/kodi.scm				\
   gnu/packages/language.scm			\
+  gnu/packages/ldc.scm				\
   gnu/packages/less.scm				\
   gnu/packages/lesstif.scm			\
   gnu/packages/libcanberra.scm			\
@@ -522,6 +523,7 @@ dist_patch_DATA =						\
   gnu/packages/patches/jasper-CVE-2008-3522.patch		\
   gnu/packages/patches/jbig2dec-ignore-testtest.patch		\
   gnu/packages/patches/kmod-module-directory.patch		\
+  gnu/packages/patches/ldc-disable-tests.patch			\
   gnu/packages/patches/lftp-dont-save-unknown-host-fingerprint.patch \
   gnu/packages/patches/liba52-enable-pic.patch			\
   gnu/packages/patches/liba52-link-with-libm.patch		\
diff --git a/gnu/packages/ldc.scm b/gnu/packages/ldc.scm
new file mode 100644
index 0000000..0bafad0
--- /dev/null
+++ b/gnu/packages/ldc.scm
@@ -0,0 +1,118 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2015 Roel Janssen <roel@gnu.org>
+;;; Copyright © 2015 Pjotr Prins <pjotr.guix@thebird.nl>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(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)
+    ;; LDC currently only supports the x86_64 and i686 architectures.
+    (supported-systems '("x86_64-linux" "i686-linux"))
+    (arguments
+     `(#:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'unpack-submodule-sources
+           (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")))))
+         (add-after 'unpack-submodule-sources 'patch-phobos
+           (lambda* (#:key inputs #:allow-other-keys)
+             (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)))))
+    (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.")
+    ;; Most of the code is released under BSD-3, except for code originally
+    ;; written for GDC, which is released under GPLv2+, and the DMD frontend,
+    ;; which is released under the “Boost Software License version 1.0".
+    (license (list license:bsd-3
+                   license:gpl2+
+                   license:x11-style "http://www.boost.org/LICENSE_1_0.txt"))))
diff --git a/gnu/packages/patches/ldc-disable-tests.patch b/gnu/packages/patches/ldc-disable-tests.patch
new file mode 100644
index 0000000..3f5e6c2
--- /dev/null
+++ b/gnu/packages/patches/ldc-disable-tests.patch
@@ -0,0 +1,90 @@
+This patch fixes a failing unit test by feeding buildNormalizedPath to the
+tzdata properly.  Three other tests are disabled, one assumes /root and the
+two others use networking.  Not bad out of almost 700 tests!
+
+by Pjotr Prins <pjotr.guix@thebird.nl>
+
+diff --git a/std/datetime.d b/std/datetime.d
+index 8e4ed3b..6c15bc5 100644
+--- a/std/datetime.d
++++ b/std/datetime.d
+@@ -28018,6 +28018,7 @@ public:
+             The default directory where the TZ Database files are. It's empty
+             for Windows, since Windows doesn't have them.
+           +/
++          
+         enum defaultTZDatabaseDir = "/usr/share/zoneinfo/";
+     }
+     else version(Windows)
+@@ -28069,14 +28070,13 @@ assert(tz.dstName == "PDT");
+         import std.range : retro;
+         import std.format : format;
+ 
+-        name = strip(name);
+-
+         enforce(tzDatabaseDir.exists(), new DateTimeException(format("Directory %s does not exist.", tzDatabaseDir)));
+         enforce(tzDatabaseDir.isDir, new DateTimeException(format("%s is not a directory.", tzDatabaseDir)));
+ 
+-        immutable file = buildNormalizedPath(tzDatabaseDir, name);
++        auto filename = "./" ~ strip(name); // make sure the prefix is not stripped
++        immutable file = buildNormalizedPath(tzDatabaseDir, filename);
+ 
+-        enforce(file.exists(), new DateTimeException(format("File %s does not exist.", file)));
++        enforce(file.exists(), new DateTimeException(format("File %s does not exist in %s.", file, tzDatabaseDir)));
+         enforce(file.isFile, new DateTimeException(format("%s is not a file.", file)));
+ 
+         auto tzFile = File(file);
+diff --git a/std/path.d b/std/path.d
+index 254d8f0..b0fc04d 100644
+--- a/std/path.d
++++ b/std/path.d
+@@ -3080,8 +3080,11 @@ unittest
+         }
+         else
+         {
++            pragma(msg, "test disabled on GNU Guix");
++/*
+             assert(expandTilde("~root") == "/root", expandTilde("~root"));
+             assert(expandTilde("~root/") == "/root/", expandTilde("~root/"));
++*/
+         }
+         assert(expandTilde("~Idontexist/hey") == "~Idontexist/hey");
+     }
+diff --git a/std/socket.d b/std/socket.d
+index b85d1c9..7fbf346 100644
+--- a/std/socket.d
++++ b/std/socket.d
+@@ -517,6 +517,8 @@ class Protocol
+ 
+ unittest
+ {
++    pragma(msg, "test disabled on GNU Guix");
++/*
+     // getprotobyname,number are unimplemented on Android
+     softUnittest({
+         Protocol proto = new Protocol;
+@@ -530,6 +532,7 @@ unittest
+         assert(proto.name == "tcp");
+         assert(proto.aliases.length == 1 && proto.aliases[0] == "TCP");
+     });
++*/
+ }
+ 
+ 
+@@ -859,6 +862,8 @@ class InternetHost
+ 
+ unittest
+ {
++    pragma(msg, "test disabled on GNU Guix");
++    /*
+     InternetHost ih = new InternetHost;
+ 
+     ih.getHostByAddr(0x7F_00_00_01);
+@@ -889,6 +894,7 @@ unittest
+         //      writefln("aliases[%d] = %s", i, s);
+         // }
+     });
++    */
+ }
+ 
+ 
-- 
2.5.0


[-- Attachment #2: Type: text/plain, Size: 7796 bytes --]

Hello Ricardo,

Thanks again for your time and helpful response.  I hope this version of
the patch is fine.

Ricardo Wurmus writes:

> 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.

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