From: Pjotr Prins <pjotr.public12@thebird.nl>
To: Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de>
Cc: guix-devel@gnu.org
Subject: Re: [PATCH] Add LDC.
Date: Tue, 29 Dec 2015 16:37:05 +0100 [thread overview]
Message-ID: <20151229153705.GB9031@thebird.nl> (raw)
In-Reply-To: <idjio3hi9hz.fsf@bimsb-sys02.mdc-berlin.net>
Just a few comments from my end:
On Tue, Dec 29, 2015 at 04:02:00PM +0100, Ricardo Wurmus wrote:
> Why “dlanguage.scm” and not just “d.scm”?
We could do. But in general it is referred to as the D language. I
don't know why.
> > + (supported-systems '("x86_64-linux" "i686-linux"))
>
> Could you add a comment here? Does upstream say that only these two
> systems are supported?
Other targets are untested - the authors say. I think someone is
working on ARM - there is evidence of Android in the tests.
> > + (add-after 'unpack 'unpack-druntime-source
> [...]
> > + (add-after 'unpack 'unpack-dmd-testsuite-source
>
> I think all these three phases could be merged into one appropriately
> named phase.
The order matters. Something to keep in mind.
> > + ("tzdata" ,tzdata))) ;; needed for tests
>
> If it’s needed for tests shouldn’t it be among the native-inputs then?
The comment is wrong, sorry (it is mine).
> > + (native-inputs
> > + `(("llvm" ,llvm)
>
> The home page says that the compiler “relies on the LLVM Core libraries
> for code generation”. Doesn’t this mean that llvm should be a regular
> input?
Perhaps, but the compiler works fine.
> > + (patches (list (search-patch "ldc-disable-tests.patch")))))
>
> Why is this patch needed? Can they not be disabled elsewhere?
Unfortunately not. D compiles with all unit tests. Also we don't want
to disable the other tests in the files by removing them from the
Makefile. Only 4 tests out of almost 700 are patched out. I think it
is amazingly good. Actually 1 test works after this patch. We'll send
it upstream.
> > + (synopsis "LLVM compiler for the D programming language")
>
> Must “LLVM” be part of the synopsis? I’d think of this as just a
> compiler, not an “LLVM compiler”.
Yes, there are 3 different D compilers. This one is specific for LLVM.
> Is there no other way to disable tests, e.g. by name or by passing some
> kind of variable to the build system?
No, there is no easy way that I am aware. Actually we spent quite some
time getting the tests to pass. The patch is simple and easy to
maintain.
I am very excited to get a working D compiler in GNU Guix! This is a
nice bit of team work.
Pj.
next prev parent reply other threads:[~2015-12-29 15:37 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 [this message]
2016-01-04 14:23 ` Roel Janssen
2016-01-05 10:06 ` Ricardo Wurmus
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=20151229153705.GB9031@thebird.nl \
--to=pjotr.public12@thebird.nl \
--cc=guix-devel@gnu.org \
--cc=ricardo.wurmus@mdc-berlin.de \
/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.