diff mbox series

[bug#41389] gnu: guile-dsv: Wrap script.

Message ID 20200519041421.10704-1-jackhill@jackhill.us
State Accepted
Headers show
Series [bug#41389] gnu: guile-dsv: Wrap script. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job

Commit Message

Jack Hill May 19, 2020, 4:14 a.m. UTC 
* gnu/packages/guile-xyz.scm (guile-dsv)[arguments]: Add "wrap program" phase.
---
 gnu/packages/guile-xyz.scm | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

Comments

Ludovic Courtès May 23, 2020, 2:28 p.m. UTC | #1 
Hi,

Jack Hill <jackhill@jackhill.us> skribis:

> * gnu/packages/guile-xyz.scm (guile-dsv)[arguments]: Add "wrap program" phase.

[...]

> +                  (add-after 'install 'wrap-program
> +                    (lambda* (#:key inputs outputs #:allow-other-keys)
> +                      (let* ((out (assoc-ref outputs "out"))
> +                             (bin (string-append out "/bin"))
> +                             (site (string-append out "/share/guile/site"))
> +                             (guile-lib (assoc-ref inputs "guile2.2-lib")))

We should eventually rename it to “guile-lib” and switch to Guile 3.0,
but that’s another story.

> +                        (match (scandir site)
> +                          (("." ".." version)

I recommend ‘target-guile-effective-version’ from (guix build
guile-build-system) instead of this trick.

Could you send an updated patch?

Thanks in advance!

Ludo’.
Jack Hill May 23, 2020, 10:48 p.m. UTC | #2 
Ludo’,

Thanks for the review!

On Sat, 23 May 2020, Ludovic Courtès wrote:

> Hi,
>
> Jack Hill <jackhill@jackhill.us> skribis:
>
>> * gnu/packages/guile-xyz.scm (guile-dsv)[arguments]: Add "wrap program" phase.
>
> [...]
>
>> +                  (add-after 'install 'wrap-program
>> +                    (lambda* (#:key inputs outputs #:allow-other-keys)
>> +                      (let* ((out (assoc-ref outputs "out"))
>> +                             (bin (string-append out "/bin"))
>> +                             (site (string-append out "/share/guile/site"))
>> +                             (guile-lib (assoc-ref inputs "guile2.2-lib")))
>
> We should eventually rename it to “guile-lib” and switch to Guile 3.0,
> but that’s another story.

I had some time today, so I've done this in the second patch. This 
required substituting configure.ac as discussed at: 
https://github.com/artyom-poptsov/guile-dsv/pull/8

Could the regular expression I used be improved?

>> +                        (match (scandir site)
>> +                          (("." ".." version)
>
> I recommend ‘target-guile-effective-version’ from (guix build
> guile-build-system) instead of this trick.
>
> Could you send an updated patch?

Thanks for the tip. I did this, and cleaned up the up the let binding and 
wrap-program calls while I was at it. I hope it makes it clearer.

> Thanks in advance!

You're welcome!

Best,
Jack
Ludovic Courtès May 24, 2020, 7:58 p.m. UTC | #3 
Jack Hill <jackhill@jackhill.us> skribis:

> I had some time today, so I've done this in the second patch. This
> required substituting configure.ac as discussed at:
> https://github.com/artyom-poptsov/guile-dsv/pull/8

Great.

> Could the regular expression I used be improved?

I had overlooked this question, what regexp are you referring to?

Ludo’.
Jack Hill May 24, 2020, 9:30 p.m. UTC | #4 
On Sun, 24 May 2020, Ludovic Courtès wrote:

> Jack Hill <jackhill@jackhill.us> skribis:
>
>> I had some time today, so I've done this in the second patch. This
>> required substituting configure.ac as discussed at:
>> https://github.com/artyom-poptsov/guile-dsv/pull/8
>
> Great.
>
>> Could the regular expression I used be improved?
>
> I had overlooked this question, what regexp are you referring to?

"GUILE_PKG.*" to match the line in configure.ac to be modified for Guile 
3.0 support. I think it's readable as is, and avoids the need to escape 
special characters, but it's very broad. Perhaps it should be more 
specific.

Best,
Jack
Ludovic Courtès May 25, 2020, 7:41 a.m. UTC | #5 
Hi,

Jack Hill <jackhill@jackhill.us> skribis:

> On Sun, 24 May 2020, Ludovic Courtès wrote:
>
>> Jack Hill <jackhill@jackhill.us> skribis:
>>
>>> I had some time today, so I've done this in the second patch. This
>>> required substituting configure.ac as discussed at:
>>> https://github.com/artyom-poptsov/guile-dsv/pull/8
>>
>> Great.
>>
>>> Could the regular expression I used be improved?
>>
>> I had overlooked this question, what regexp are you referring to?
>
> "GUILE_PKG.*" to match the line in configure.ac to be modified for
> Guile 3.0 support. I think it's readable as is, and avoids the need to
> escape special characters, but it's very broad. Perhaps it should be
> more specific.

I think it’s fine this way.  It’s unlikely to match anything else.

Ludo’.
Jack Hill May 25, 2020, 4 p.m. UTC | #6 
On Mon, 25 May 2020, Ludovic Courtès wrote:

> Jack Hill <jackhill@jackhill.us> skribis:

[…]

>> "GUILE_PKG.*" to match the line in configure.ac to be modified for
>> Guile 3.0 support. I think it's readable as is, and avoids the need to
>> escape special characters, but it's very broad. Perhaps it should be
>> more specific.
>
> I think it’s fine this way.  It’s unlikely to match anything else.

Ok, makes sense. Thanks again for the review and applying the patches.

Best,
Jack
diff mbox series

Patch

diff --git a/gnu/packages/guile-xyz.scm b/gnu/packages/guile-xyz.scm
index 674b1f922b..ee2e44653e 100644
--- a/gnu/packages/guile-xyz.scm
+++ b/gnu/packages/guile-xyz.scm
@@ -25,6 +25,7 @@ 
 ;;; Copyright © 2019 Timothy Sample <samplet@ngyro.com>
 ;;; Copyright © 2019, 2020 Martin Becze <mjbecze@riseup.net>
 ;;; Copyright © 2020 Evan Straw <evan.straw99@gmail.com>
+;;; Copyright © 2020 Jack Hill <jackhill@jackhill.us>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -467,7 +468,9 @@  you send to a FIFO file.")
     (inputs `(("guile" ,guile-2.2)))
     (propagated-inputs `(("guile2.2-lib" ,guile2.2-lib)))
     (arguments
-     '(#:phases (modify-phases %standard-phases
+     `(#:modules ((ice-9 match) (ice-9 ftw)
+                  ,@%gnu-build-system-modules)
+       #:phases (modify-phases %standard-phases
                   (add-before 'configure 'set-guilesitedir
                     (lambda _
                       (substitute* "Makefile.in"
@@ -482,6 +485,28 @@  $(datadir)/guile/site/$(GUILE_EFFECTIVE_VERSION)\n"))
                         (("^guilesitedir =.*$")
                          "guilesitedir = \
 $(datadir)/guile/site/$(GUILE_EFFECTIVE_VERSION)\n"))
+                      #t))
+                  (add-after 'install 'wrap-program
+                    (lambda* (#:key inputs outputs #:allow-other-keys)
+                      (let* ((out (assoc-ref outputs "out"))
+                             (bin (string-append out "/bin"))
+                             (site (string-append out "/share/guile/site"))
+                             (guile-lib (assoc-ref inputs "guile2.2-lib")))
+                        (match (scandir site)
+                          (("." ".." version)
+                           (let ((modules (string-append site "/" version))
+                                 (compiled-modules (string-append
+                                                    out "/lib/guile/" version
+                                                    "/site-ccache")))
+                             (wrap-program (string-append bin "/dsv")
+                               `("GUILE_LOAD_PATH" prefix
+                                 (,modules
+                                  ,(string-append guile-lib "/share/guile/site")))
+                               `("GUILE_LOAD_COMPILED_PATH" prefix
+                                 (,compiled-modules
+                                  ,(string-append guile-lib "/lib/guile/"
+                                                  version
+                                                  "/site-ccache"))))))))
                       #t)))))
     (home-page "https://github.com/artyom-poptsov/guile-dsv")
     (synopsis "DSV module for Guile")