[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Eric Bavier
Subject: Re: [PATCH]:Resend: Add LLVM, clang and clang-runtime-3.8 to GNU Guix
Date: Wed, 06 Jul 2016 13:08:24 -0500
User-agent: Roundcube Webmail/1.0.6

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.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]