Re: [PATCH] Add LDC.

[Pjotr Prins]

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.