[PATCH] gnu: Simplify LLVM build.

[PATCH] gnu: Simplify LLVM build.

[Andy Wingo]

[-- Attachment #1: 0001-gnu-Simplify-LLVM-build.patch --]
[-- Type: text/plain, Size: 1883 bytes --]

From db066d194d3b8359eddd0149234bfad29c11542d Mon Sep 17 00:00:00 2001
From: Andy Wingo <wingo@igalia.com>
Date: Mon, 17 Aug 2015 08:26:07 +0200
Subject: [PATCH] gnu: Simplify LLVM build.

* gnu/packages/llvm.scm (llvm): Simplify build.
---
 gnu/packages/llvm.scm | 20 ++------------------
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/gnu/packages/llvm.scm b/gnu/packages/llvm.scm
index 9e06a64..2c96e9d 100644
--- a/gnu/packages/llvm.scm
+++ b/gnu/packages/llvm.scm
@@ -49,24 +49,8 @@
      `(("python" ,python-wrapper)
        ("perl"   ,perl)))
     (arguments
-     `(#:phases (alist-cons-before
-                 'build 'link-lib-for-build-exec
-                 (lambda* (#:key outputs #:allow-other-keys)
-                   ;; This is a hacky fix that will allow binaries to run
-                   ;; before being installed.  -DCMAKE_SKIP_BUILD_RPATH=FALSE
-                   ;; seems to not help.  Nixpkgs does the same.
-                   (let* ((out       (assoc-ref outputs "out"))
-                          (out-lib   (string-append out "/lib"))
-                          (build-lib (string-append (getcwd) "/lib")))
-                     (mkdir-p out)
-                     (symlink build-lib out-lib)))
-                 (alist-cons-after
-                  'build 'cleanup-out
-                  (lambda* (#:key outputs #:allow-other-keys)
-                    ;; Cleanup the symlink that was created previously.  Let
-                    ;; the install phase repopulate out.
-                    (delete-file-recursively (assoc-ref outputs "out")))
-                  %standard-phases))))
+     `(#:configure-flags '("-DCMAKE_SKIP_BUILD_RPATH=FALSE"
+                           "-DCMAKE_BUILD_WITH_INSTALL_RPATH=FALSE")))
     (home-page "http://www.llvm.org")
     (synopsis "Optimizing compiler infrastructure")
     (description
-- 
2.4.3

Re: [PATCH] gnu: Simplify LLVM build.

[Eric Bavier]

Hello Andy,

Thanks for taking a closer look at this!

On Mon, 17 Aug 2015 08:27:38 +0200
Andy Wingo <wingo@igalia.com> wrote:

> From db066d194d3b8359eddd0149234bfad29c11542d Mon Sep 17 00:00:00 2001
> From: Andy Wingo <wingo@igalia.com>
> Date: Mon, 17 Aug 2015 08:26:07 +0200
> Subject: [PATCH] gnu: Simplify LLVM build.
> 
> * gnu/packages/llvm.scm (llvm): Simplify build.

Could this perhaps be a bit more descriptive?  It also doesn't
follow our standard.  Maybe:

gnu: llvm: Simplify build rpath handling.

* gnu/packages/llvm.scm (llvm)[arguments]: Remove phases argument.
  Add to configure-flags "-DCMAKE_SKIP_BUILD_RPATH=FALSE" and
  "-DCMAKE_BUILD_WITH_INSTALL_RPATH=FALSE".

`~Eric

> ---
>  gnu/packages/llvm.scm | 20 ++------------------
>  1 file changed, 2 insertions(+), 18 deletions(-)
> 
> diff --git a/gnu/packages/llvm.scm b/gnu/packages/llvm.scm
> index 9e06a64..2c96e9d 100644
> --- a/gnu/packages/llvm.scm
> +++ b/gnu/packages/llvm.scm
> @@ -49,24 +49,8 @@
>       `(("python" ,python-wrapper)
>         ("perl"   ,perl)))
>      (arguments
> -     `(#:phases (alist-cons-before
> -                 'build 'link-lib-for-build-exec
> -                 (lambda* (#:key outputs #:allow-other-keys)
> -                   ;; This is a hacky fix that will allow binaries
> to run
> -                   ;; before being installed.
> -DCMAKE_SKIP_BUILD_RPATH=FALSE
> -                   ;; seems to not help.  Nixpkgs does the same.
> -                   (let* ((out       (assoc-ref outputs "out"))
> -                          (out-lib   (string-append out "/lib"))
> -                          (build-lib (string-append (getcwd)
> "/lib")))
> -                     (mkdir-p out)
> -                     (symlink build-lib out-lib)))
> -                 (alist-cons-after
> -                  'build 'cleanup-out
> -                  (lambda* (#:key outputs #:allow-other-keys)
> -                    ;; Cleanup the symlink that was created
> previously.  Let
> -                    ;; the install phase repopulate out.
> -                    (delete-file-recursively (assoc-ref outputs
> "out")))
> -                  %standard-phases))))
> +     `(#:configure-flags '("-DCMAKE_SKIP_BUILD_RPATH=FALSE"
>                             "-DCMAKE_BUILD_WITH_INSTALL_RPATH=FALSE")))
> (home-page "http://www.llvm.org") (synopsis "Optimizing compiler
> infrastructure") (description

Re: [PATCH] gnu: Simplify LLVM build.

[Andy Wingo]

On Mon 17 Aug 2015 10:43, Eric Bavier <ericbavier@openmailbox.org> writes:

> Hello Andy,
>
> Thanks for taking a closer look at this!
>
> On Mon, 17 Aug 2015 08:27:38 +0200
> Andy Wingo <wingo@igalia.com> wrote:
>
>> From db066d194d3b8359eddd0149234bfad29c11542d Mon Sep 17 00:00:00 2001
>> From: Andy Wingo <wingo@igalia.com>
>> Date: Mon, 17 Aug 2015 08:26:07 +0200
>> Subject: [PATCH] gnu: Simplify LLVM build.
>> 
>> * gnu/packages/llvm.scm (llvm): Simplify build.
>
> Could this perhaps be a bit more descriptive?  It also doesn't
> follow our standard.  Maybe:
>
> gnu: llvm: Simplify build rpath handling.
>
> * gnu/packages/llvm.scm (llvm)[arguments]: Remove phases argument.
>   Add to configure-flags "-DCMAKE_SKIP_BUILD_RPATH=FALSE" and
>   "-DCMAKE_BUILD_WITH_INSTALL_RPATH=FALSE".
>
> `~Eric

It could :)  However I do not see how the change you mention would be
indicated by the GCS, HACKING, or guix.texi.  As per the GCS I think
this falls under "simple changes".  If Guix has additional requirements
they should be indicated somewhere; did I miss the document?

Cheers,

Andy

Re: [PATCH] gnu: Simplify LLVM build.

[Mark H Weaver]

Hi Andy,

Thank you for this most excellent, radical simplification of our llvm
package!  However, I agree with Eric about the commit log.

Andy Wingo <wingo@igalia.com> writes:

> On Mon 17 Aug 2015 10:43, Eric Bavier <ericbavier@openmailbox.org> writes:
>
>>> From db066d194d3b8359eddd0149234bfad29c11542d Mon Sep 17 00:00:00 2001
>>> From: Andy Wingo <wingo@igalia.com>
>>> Date: Mon, 17 Aug 2015 08:26:07 +0200
>>> Subject: [PATCH] gnu: Simplify LLVM build.
>>> 
>>> * gnu/packages/llvm.scm (llvm): Simplify build.
>>
>> Could this perhaps be a bit more descriptive?  It also doesn't
>> follow our standard.  Maybe:
>>
>> gnu: llvm: Simplify build rpath handling.
>>
>> * gnu/packages/llvm.scm (llvm)[arguments]: Remove phases argument.
>>   Add to configure-flags "-DCMAKE_SKIP_BUILD_RPATH=FALSE" and
>>   "-DCMAKE_BUILD_WITH_INSTALL_RPATH=FALSE".
>>
>> `~Eric
>
> It could :)  However I do not see how the change you mention would be
> indicated by the GCS, HACKING, or guix.texi.  As per the GCS I think
> this falls under "simple changes".

Whether changes are "simple" is a matter of judgment, I suppose.  In
this case you've replaced 18 out of 45 lines (40%) of the package
description, the entire customization of the cmake-build-system process.
Under Ludovic's leadership, we strive for high standards in our commit
logs, and I for one am thankful for that when looking over the logs.

> If Guix has additional requirements they should be indicated
> somewhere; did I miss the document?

No, you didn't miss it.  We have a lot to do, and writing precise
documentation of our coding standards is not something we've yet found
time for.  I'm not sure it's something that could be precisely
documented even if we tried.

I'd like to push your commit with Eric's proposed commit log.
Is that okay?

     Mark

Re: [PATCH] gnu: Simplify LLVM build.

[Andy Wingo]

On Fri 21 Aug 2015 23:16, Mark H Weaver <mhw@netris.org> writes:

> Andy Wingo <wingo@igalia.com> writes:
>
>> If Guix has additional requirements they should be indicated
>> somewhere; did I miss the document?
>
> No, you didn't miss it.  We have a lot to do, and writing precise
> documentation of our coding standards is not something we've yet found
> time for.  I'm not sure it's something that could be precisely
> documented even if we tried.

I'm pretty sure many things could be documented.  Specifically the rules
about the commit line formatting and the format of messages adding new
packages or upgrading packages have been explained enough times that it
is wasting your time not to put them in a document :)  Probably they
should be put into some automatic linter as well.

I would offer to do this, but I think Guix's commit log standards are
unnecessarily verbose so I am not the best one to do this :)

> I'd like to push your commit with Eric's proposed commit log.
> Is that okay?

Sure!  Thank you Mark :)

Andy

Re: [PATCH] gnu: Simplify LLVM build.

[Mark H Weaver]

Andy Wingo <wingo@igalia.com> writes:

> On Fri 21 Aug 2015 23:16, Mark H Weaver <mhw@netris.org> writes:
>
>> Andy Wingo <wingo@igalia.com> writes:
>>
>>> If Guix has additional requirements they should be indicated
>>> somewhere; did I miss the document?
>>
>> No, you didn't miss it.  We have a lot to do, and writing precise
>> documentation of our coding standards is not something we've yet found
>> time for.  I'm not sure it's something that could be precisely
>> documented even if we tried.
>
> I'm pretty sure many things could be documented.  Specifically the rules
> about the commit line formatting and the format of messages adding new
> packages or upgrading packages have been explained enough times that it
> is wasting your time not to put them in a document :)

Well, okay, you have a good point :)

> Probably they should be put into some automatic linter as well.

That would be more difficult.

>> I'd like to push your commit with Eric's proposed commit log.
>> Is that okay?
>
> Sure!  Thank you Mark :)

Pushed.  Thanks to you both!

     Mark