Re: [PATCH 4/7] gnu: Add llvm-for-extempore.

[Ricardo Wurmus]

Thompson, David  writes:

> On Wed, Sep 14, 2016 at 5:38 AM, Ricardo Wurmus  wrote:
>> * gnu/packages/llvm.scm (llvm-for-extempore): New variable.
>> ---
>>  gnu/packages/llvm.scm | 8 
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/gnu/packages/llvm.scm b/gnu/packages/llvm.scm
>> index a9eba79..b55a200 100644
>> --- a/gnu/packages/llvm.scm
>> +++ b/gnu/packages/llvm.scm
>> @@ -3,6 +3,7 @@
>>  ;;; Copyright © 2015 Mark H Weaver 
>>  ;;; Copyright © 2015 Ludovic Courtès 
>>  ;;; Copyright © 2016 Dennis Mungai 
>> +;;; Copyright © 2016 Ricardo Wurmus 
>>  ;;;
>>  ;;; This file is part of GNU Guix.
>>  ;;;
>> @@ -267,3 +268,10 @@ code analysis tools.")
>>  (define-public clang-3.5
>>(clang-from-llvm llvm-3.5 clang-runtime-3.5
>> "0846h8vn3zlc00jkmvrmy88gc6ql6014c02l4jv78fpvfigmgssg"))
>> +
>> +(define-public llvm-for-extempore
>> +  (package (inherit llvm-3.7)
>> +(source
>> + (origin
>> +   (inherit (package-source llvm-3.7))
>> +   (patches (list (search-patch "llvm-for-extempore.patch")))
>
> This could use a comment explaining the rationale.  Also, you forgot
> to include the patch file in this commit.

Oh, I did indeed forget it.  I’ve added it and put a long comment to the
top, which I’ll quote here:

~~
This patch to LLVM is required by the developers of the Extempore language.
The following explanation was posted to the extemporel...@googlegroups.com
mailing list:

"There is an assumption in the parser that all definitions occur within the
same compilation unit - i.e. the parser has local state about what has been
parsed in this unit of work.  Extempore obviously does lots of little units
rather than one big unit and this causes problems for named types that were
defined in another unit - which they always are.  The patch simply checks the
current module to see if the type has been previously defined, and intervenes
appropriately if it has."

Message-ID: 
~~

Thanks for the review!

~~ Ricardo

Re: [PATCH 4/7] gnu: Add llvm-for-extempore.

[Danny Milosavljevic]

> +   (patches (list (search-patch "llvm-for-extempore.patch")))

Hmm... is this patch file somewhere?

Re: [PATCH 4/7] gnu: Add llvm-for-extempore.

[Thompson, David]

On Wed, Sep 14, 2016 at 5:38 AM, Ricardo Wurmus  wrote:
> * gnu/packages/llvm.scm (llvm-for-extempore): New variable.
> ---
>  gnu/packages/llvm.scm | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/gnu/packages/llvm.scm b/gnu/packages/llvm.scm
> index a9eba79..b55a200 100644
> --- a/gnu/packages/llvm.scm
> +++ b/gnu/packages/llvm.scm
> @@ -3,6 +3,7 @@
>  ;;; Copyright © 2015 Mark H Weaver 
>  ;;; Copyright © 2015 Ludovic Courtès 
>  ;;; Copyright © 2016 Dennis Mungai 
> +;;; Copyright © 2016 Ricardo Wurmus 
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -267,3 +268,10 @@ code analysis tools.")
>  (define-public clang-3.5
>(clang-from-llvm llvm-3.5 clang-runtime-3.5
> "0846h8vn3zlc00jkmvrmy88gc6ql6014c02l4jv78fpvfigmgssg"))
> +
> +(define-public llvm-for-extempore
> +  (package (inherit llvm-3.7)
> +(source
> + (origin
> +   (inherit (package-source llvm-3.7))
> +   (patches (list (search-patch "llvm-for-extempore.patch")))

This could use a comment explaining the rationale.  Also, you forgot
to include the patch file in this commit.

- Dave