all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [PATCH] build: ruby: Add 'gem-flags' key to ruby build system
@ 2015-07-13 13:44 Pjotr Prins
  2015-07-17 21:05 ` Pjotr Prins
  2015-07-18 15:20 ` Ludovic Courtès
  0 siblings, 2 replies; 7+ messages in thread
From: Pjotr Prins @ 2015-07-13 13:44 UTC (permalink / raw)
  To: guix-devel

[-- Attachment #1: Type: text/plain, Size: 215 bytes --]

Introducing gem-flags in preparation of ruby-nokogiri package

* guix/build-system/ruby.scm (build): add 'gem-flags' key
* guix/build/ruby-build-system.scm (build): use 'gem-flags' key

(this looks clean to me...)


[-- Attachment #2: 0001-build-ruby-Add-gem-flags-key-to-ruby-build-system.patch --]
[-- Type: text/x-diff, Size: 2257 bytes --]

From ce8cfeadc8a661ff0fe0b96dc241d0063ed49ba3 Mon Sep 17 00:00:00 2001
From: pjotrp <pjotr.public01@thebird.nl>
Date: Mon, 13 Jul 2015 15:32:36 +0200
Subject: [PATCH] build: ruby: Add 'gem-flags' key to ruby build system

* guix/build-system/ruby.scm (build): add 'gem-flags' key
* guix/build/ruby-build-system.scm (build): use 'gem-flags' key
---
 guix/build-system/ruby.scm       |    2 ++
 guix/build/ruby-build-system.scm |   11 +++++++----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/guix/build-system/ruby.scm b/guix/build-system/ruby.scm
index e4fda30..d601700 100644
--- a/guix/build-system/ruby.scm
+++ b/guix/build-system/ruby.scm
@@ -71,6 +71,7 @@
 
 (define* (ruby-build store name inputs
                      #:key
+                     (gem-flags ''())
                      (test-target "test")
                      (tests? #t)
                      (phases '(@ (guix build ruby-build-system)
@@ -95,6 +96,7 @@
                                (source
                                 source))
                    #:system ,system
+                   #:gem-flags ,gem-flags
                    #:test-target ,test-target
                    #:tests? ,tests?
                    #:phases ,phases
diff --git a/guix/build/ruby-build-system.scm b/guix/build/ruby-build-system.scm
index fce39b8..9cbc8a5 100644
--- a/guix/build/ruby-build-system.scm
+++ b/guix/build/ruby-build-system.scm
@@ -63,7 +63,8 @@ directory."
       (zero? (system* "rake" test-target))
       #t))
 
-(define* (install #:key source inputs outputs #:allow-other-keys)
+(define* (install #:key source inputs outputs (gem-flags '())
+                  #:allow-other-keys)
   (let* ((ruby-version
           (match:substring (string-match "ruby-(.*)\\.[0-9]$"
                                          (assoc-ref inputs "ruby"))
@@ -73,9 +74,11 @@ directory."
     (setenv "GEM_HOME" gem-home)
     (mkdir-p gem-home)
     (zero? (system* "gem" "install" "--local"
-                    "--bindir" (string-append out "/bin")))))
+                    "--bindir" (string-append out "/bin") "--"
+                    (string-join (cond (null? gem-flags)('())(gem-flags)))))))
 
 (define %standard-phases
   (modify-phases gnu:%standard-phases
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] build: ruby: Add 'gem-flags' key to ruby build system
  2015-07-13 13:44 [PATCH] build: ruby: Add 'gem-flags' key to ruby build system Pjotr Prins
@ 2015-07-17 21:05 ` Pjotr Prins
  2015-07-18 15:20 ` Ludovic Courtès
  1 sibling, 0 replies; 7+ messages in thread
From: Pjotr Prins @ 2015-07-17 21:05 UTC (permalink / raw)
  To: Pjotr Prins; +Cc: guix-devel

Can this patch go in? I sent it 4 days ago. It allows building
extensions for Ruby gems, by passing in flags to gem, as shown in the
Nokogiri patch. Even without accepting Nokogiri, this is valuable for 
building and installing gems.

Pj.

On Mon, Jul 13, 2015 at 03:44:51PM +0200, Pjotr Prins wrote:
> Introducing gem-flags in preparation of ruby-nokogiri package
> 
> * guix/build-system/ruby.scm (build): add 'gem-flags' key
> * guix/build/ruby-build-system.scm (build): use 'gem-flags' key
> 
> (this looks clean to me...)
> 

> From ce8cfeadc8a661ff0fe0b96dc241d0063ed49ba3 Mon Sep 17 00:00:00 2001
> From: pjotrp <pjotr.public01@thebird.nl>
> Date: Mon, 13 Jul 2015 15:32:36 +0200
> Subject: [PATCH] build: ruby: Add 'gem-flags' key to ruby build system
> 
> * guix/build-system/ruby.scm (build): add 'gem-flags' key
> * guix/build/ruby-build-system.scm (build): use 'gem-flags' key
> ---
>  guix/build-system/ruby.scm       |    2 ++
>  guix/build/ruby-build-system.scm |   11 +++++++----
>  2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/guix/build-system/ruby.scm b/guix/build-system/ruby.scm
> index e4fda30..d601700 100644
> --- a/guix/build-system/ruby.scm
> +++ b/guix/build-system/ruby.scm
> @@ -71,6 +71,7 @@
>  
>  (define* (ruby-build store name inputs
>                       #:key
> +                     (gem-flags ''())
>                       (test-target "test")
>                       (tests? #t)
>                       (phases '(@ (guix build ruby-build-system)
> @@ -95,6 +96,7 @@
>                                 (source
>                                  source))
>                     #:system ,system
> +                   #:gem-flags ,gem-flags
>                     #:test-target ,test-target
>                     #:tests? ,tests?
>                     #:phases ,phases
> diff --git a/guix/build/ruby-build-system.scm b/guix/build/ruby-build-system.scm
> index fce39b8..9cbc8a5 100644
> --- a/guix/build/ruby-build-system.scm
> +++ b/guix/build/ruby-build-system.scm
> @@ -63,7 +63,8 @@ directory."
>        (zero? (system* "rake" test-target))
>        #t))
>  
> -(define* (install #:key source inputs outputs #:allow-other-keys)
> +(define* (install #:key source inputs outputs (gem-flags '())
> +                  #:allow-other-keys)
>    (let* ((ruby-version
>            (match:substring (string-match "ruby-(.*)\\.[0-9]$"
>                                           (assoc-ref inputs "ruby"))
> @@ -73,9 +74,11 @@ directory."
>      (setenv "GEM_HOME" gem-home)
>      (mkdir-p gem-home)
>      (zero? (system* "gem" "install" "--local"
> -                    "--bindir" (string-append out "/bin")))))
> +                    "--bindir" (string-append out "/bin") "--"
> +                    (string-join (cond (null? gem-flags)('())(gem-flags)))))))
>  
>  (define %standard-phases
>    (modify-phases gnu:%standard-phases
> -- 
> 1.7.10.4
> 


-- 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] build: ruby: Add 'gem-flags' key to ruby build system
  2015-07-13 13:44 [PATCH] build: ruby: Add 'gem-flags' key to ruby build system Pjotr Prins
  2015-07-17 21:05 ` Pjotr Prins
@ 2015-07-18 15:20 ` Ludovic Courtès
  2015-07-19  9:29   ` Pjotr Prins
  1 sibling, 1 reply; 7+ messages in thread
From: Ludovic Courtès @ 2015-07-18 15:20 UTC (permalink / raw)
  To: Pjotr Prins; +Cc: guix-devel

Pjotr Prins <pjotr.public12@thebird.nl> skribis:

> From ce8cfeadc8a661ff0fe0b96dc241d0063ed49ba3 Mon Sep 17 00:00:00 2001
> From: pjotrp <pjotr.public01@thebird.nl>
> Date: Mon, 13 Jul 2015 15:32:36 +0200
> Subject: [PATCH] build: ruby: Add 'gem-flags' key to ruby build system
>
> * guix/build-system/ruby.scm (build): add 'gem-flags' key
> * guix/build/ruby-build-system.scm (build): use 'gem-flags' key

[...]

>      (zero? (system* "gem" "install" "--local"
> -                    "--bindir" (string-append out "/bin")))))
> +                    "--bindir" (string-append out "/bin") "--"
> +                    (string-join (cond (null? gem-flags)('())(gem-flags)))))))

The ‘cond’ form is syntactically invalid, and ‘gem-flags’ is not a
procedure so it cannot be called.  So I’ve changed that, also removing
the “--”.  Pushed as 6e9f291.

Thanks!

Ludo’.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] build: ruby: Add 'gem-flags' key to ruby build system
  2015-07-18 15:20 ` Ludovic Courtès
@ 2015-07-19  9:29   ` Pjotr Prins
  2015-07-19 20:44     ` Ludovic Courtès
  0 siblings, 1 reply; 7+ messages in thread
From: Pjotr Prins @ 2015-07-19  9:29 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guix-devel

On Sat, Jul 18, 2015 at 05:20:28PM +0200, Ludovic Courtès wrote:
> >      (zero? (system* "gem" "install" "--local"
> > -                    "--bindir" (string-append out "/bin")))))
> > +                    "--bindir" (string-append out "/bin") "--"
> > +                    (string-join (cond (null? gem-flags)('())(gem-flags)))))))
> 
> , and ‘gem-flags’ is not a procedure so it cannot be called.  So
> I’ve changed that, also removing the “--”.  Pushed as 6e9f291.

The "--" is required when gem-flags is passed in. It is optional when
gem-flags is empty. Do you think gem authors should always prepend the
"--" in front of the other options? I would think it is a builder
thing if we can abstract it away. Right? If that is so, we can leave it
there since it is harmless if gem-flags is empty.

Pj.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] build: ruby: Add 'gem-flags' key to ruby build system
  2015-07-19  9:29   ` Pjotr Prins
@ 2015-07-19 20:44     ` Ludovic Courtès
  2015-07-20  6:37       ` Pjotr Prins
  2015-07-20 17:43       ` Thompson, David
  0 siblings, 2 replies; 7+ messages in thread
From: Ludovic Courtès @ 2015-07-19 20:44 UTC (permalink / raw)
  To: Pjotr Prins; +Cc: guix-devel

Pjotr Prins <pjotr.public12@thebird.nl> skribis:

> On Sat, Jul 18, 2015 at 05:20:28PM +0200, Ludovic Courtès wrote:
>> >      (zero? (system* "gem" "install" "--local"
>> > -                    "--bindir" (string-append out "/bin")))))
>> > +                    "--bindir" (string-append out "/bin") "--"
>> > +                    (string-join (cond (null? gem-flags)('())(gem-flags)))))))
>> 
>> , and ‘gem-flags’ is not a procedure so it cannot be called.  So
>> I’ve changed that, also removing the “--”.  Pushed as 6e9f291.
>
> The "--" is required when gem-flags is passed in. It is optional when
> gem-flags is empty. Do you think gem authors should always prepend the
> "--" in front of the other options? I would think it is a builder
> thing if we can abstract it away. Right? If that is so, we can leave it
> there since it is harmless if gem-flags is empty.

My guess is that there are options like --bindir (maybe “--docdir”?) for
which “--” does not need to be added, and others for which it is
needed.  That’s why I left it out (also: users can easily add “--” but
cannot remove it if it’s hard-coded.)

Now, Dave and you definitely know this better than me, so I’ll rely on
your judgment.  Thoughts?

Ludo’.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] build: ruby: Add 'gem-flags' key to ruby build system
  2015-07-19 20:44     ` Ludovic Courtès
@ 2015-07-20  6:37       ` Pjotr Prins
  2015-07-20 17:43       ` Thompson, David
  1 sibling, 0 replies; 7+ messages in thread
From: Pjotr Prins @ 2015-07-20  6:37 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guix-devel

On Sun, Jul 19, 2015 at 10:44:18PM +0200, Ludovic Courtès wrote:
> > On Sat, Jul 18, 2015 at 05:20:28PM +0200, Ludovic Courtès wrote:
> >> >      (zero? (system* "gem" "install" "--local"
> >> > -                    "--bindir" (string-append out "/bin")))))
> >> > +                    "--bindir" (string-append out "/bin") "--"
> >> > +                    (string-join (cond (null? gem-flags)('())(gem-flags)))))))

> My guess is that there are options like --bindir (maybe “--docdir”?) for
> which “--” does not need to be added, and others for which it is
> needed.  That’s why I left it out (also: users can easily add “--” but
> cannot remove it if it’s hard-coded.)

OK. Makes sense. Arguably, in that case the default options (--local,
--bindir) should be overridable too. But, as they are sensible, let's
leave it the way it is now. We'll just add "--" for every gem (it is
actually a pass-through of options to the underlying build system, so
I could have named it ext-flags, or something, and have a separate
gem-flags).

But I think the current version is fine. Next up the ruby-nokogiri
package.

Pj.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] build: ruby: Add 'gem-flags' key to ruby build system
  2015-07-19 20:44     ` Ludovic Courtès
  2015-07-20  6:37       ` Pjotr Prins
@ 2015-07-20 17:43       ` Thompson, David
  1 sibling, 0 replies; 7+ messages in thread
From: Thompson, David @ 2015-07-20 17:43 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guix-devel

On Sun, Jul 19, 2015 at 4:44 PM, Ludovic Courtès <ludo@gnu.org> wrote:
> Pjotr Prins <pjotr.public12@thebird.nl> skribis:
>
>> On Sat, Jul 18, 2015 at 05:20:28PM +0200, Ludovic Courtès wrote:
>>> >      (zero? (system* "gem" "install" "--local"
>>> > -                    "--bindir" (string-append out "/bin")))))
>>> > +                    "--bindir" (string-append out "/bin") "--"
>>> > +                    (string-join (cond (null? gem-flags)('())(gem-flags)))))))
>>>
>>> , and ‘gem-flags’ is not a procedure so it cannot be called.  So
>>> I’ve changed that, also removing the “--”.  Pushed as 6e9f291.
>>
>> The "--" is required when gem-flags is passed in. It is optional when
>> gem-flags is empty. Do you think gem authors should always prepend the
>> "--" in front of the other options? I would think it is a builder
>> thing if we can abstract it away. Right? If that is so, we can leave it
>> there since it is harmless if gem-flags is empty.
>
> My guess is that there are options like --bindir (maybe “--docdir”?) for
> which “--” does not need to be added, and others for which it is
> needed.  That’s why I left it out (also: users can easily add “--” but
> cannot remove it if it’s hard-coded.)
>
> Now, Dave and you definitely know this better than me, so I’ll rely on
> your judgment.  Thoughts?

I don't really use the 'gem' tool directly most of the time, but not
hardcoding '--' sounds reasonable based on your thoughts above.

- Dave

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-07-20 17:43 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-13 13:44 [PATCH] build: ruby: Add 'gem-flags' key to ruby build system Pjotr Prins
2015-07-17 21:05 ` Pjotr Prins
2015-07-18 15:20 ` Ludovic Courtès
2015-07-19  9:29   ` Pjotr Prins
2015-07-19 20:44     ` Ludovic Courtès
2015-07-20  6:37       ` Pjotr Prins
2015-07-20 17:43       ` Thompson, David

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.