Re: [PATCH]:Resend: Add LLVM, clang and clang-runtime-3.8 to GNU Guix

[Eric Bavier]

On 2016-07-06 06:54, Efraim Flashner wrote:
> On Wed, Jul 06, 2016 at 06:55:56AM +0200, Ricardo Wurmus wrote:
> > Roel Janssen <address@hidden> writes:
> >
> > >>From 451d93eaaab6166e38e33f0556fe40f83afefc5c Mon Sep 17 00:00:00 2001
> > > From: brainiarc7 <address@hidden>
> > > Date: Tue, 5 Jul 2016 22:42:24 +0200
> > > Subject: [PATCH] gnu: Add LLVM 3.8.
> > >
> > > * gnu/packages/llvm.scm (llvm-3.8): New variable.
> > > * gnu/packages/llvm.scm (clang-runtime-3.8): New variable.
> >
> > Please split this into two commits.  One for adding the “license:”
> > prefix, and another to add the new variables.
> >
> > Other than that it looks good to me.  Thank you, Roel and Dennis!
> >
> > ~~ Ricardo
>
> Do we want to change the default llvm and clang to 3.8 also?

Yes, I think we should.  And fix the fallout from that change.

I have a few other issues with this patch:

1) It copy-pastes too much from the llvm package, rather than relying on 
a simple inherit like other versions.

2) It introduces cmake flags for things that are default, e.g. 
"-DLLVM_ENABLE_PIC=ON"

3) It uses the "-DCMAKE_BUILD_TYPE=Release" flag rather that our 
cmake-build-system's #:build-type keyword.

4) It adds libffi and zlib inputs.  These are not strictly necessary for 
the build, so I would prefer they be added in a separate commit.   Also, 
there's a missing "-DLLVM_ENABLE_FFI:BOOL=TRUE" flag, so the build 
system doesn't use the new input.

5) It doesn't also add a address@hidden package.  While this could be done 
later, I'd prefer it be done at the same time, like previous llvm 
updates.

I have been working on this update in parallel.  I've incorporated some 
parts of this patch, and I'll post the series here soon.

--
`~Eric