Re: 01/01: build-system/ruby: Use invoke.

Re: 01/01: build-system/ruby: Use invoke.

[Christopher Baines]

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


guix-commits@gnu.org writes:

> efraim pushed a commit to branch staging
> in repository guix.
>
> commit 0244952c11c0409597fce5c39dfbcafdfd2ea651
> Author: Efraim Flashner <efraim@flashner.co.il>
> Date:   Thu Mar 28 19:17:34 2019 +0200
>
>     build-system/ruby: Use invoke.
>
>     * guix/build/ruby-build-system.scm (install): Use invoke.
> ---
>  guix/build/ruby-build-system.scm | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/guix/build/ruby-build-system.scm b/guix/build/ruby-build-system.scm
> index ba0de12..49400b2 100644
> --- a/guix/build/ruby-build-system.scm
> +++ b/guix/build/ruby-build-system.scm
> @@ -143,14 +143,13 @@ GEM-FLAGS are passed to the 'gem' invokation, if present."
>           (gem-dir (string-append vendor-dir "/gems/" gem-name)))
>      (setenv "GEM_VENDOR" vendor-dir)
>
> -    (or (zero?
> -         (apply system* "gem" "install" gem-file
> -                "--verbose"
> -                "--local" "--ignore-dependencies" "--vendor"
> -                ;; Executables should go into /bin, not
> -                ;; /lib/ruby/gems.
> -                "--bindir" (string-append out "/bin")
> -                gem-flags))
> +    (or (apply invoke "gem" "install" gem-file
> +               "--verbose"
> +               "--local" "--ignore-dependencies" "--vendor"
> +               ;; Executables should go into /bin, not
> +               ;; /lib/ruby/gems.
> +               "--bindir" (string-append out "/bin")
> +               gem-flags)
>          (begin
>            (let ((failed-output-dir (string-append (getcwd) "/out")))
>              (mkdir failed-output-dir)

Hey Efraim,

Given that invoke raises an exception, I doubt the custom error handling
code here will ever be executed.

I guess you could change it to catch the exception, but using system*
and looking at the exit code seems pretty reasonable to me. What do you
think?

Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]

Re: 01/01: build-system/ruby: Use invoke.

[Efraim Flashner]

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

On Thu, Mar 28, 2019 at 06:14:48PM +0000, Christopher Baines wrote:
> 
> guix-commits@gnu.org writes:
> 
> > efraim pushed a commit to branch staging
> > in repository guix.
> >
> > commit 0244952c11c0409597fce5c39dfbcafdfd2ea651
> > Author: Efraim Flashner <efraim@flashner.co.il>
> > Date:   Thu Mar 28 19:17:34 2019 +0200
> >
> >     build-system/ruby: Use invoke.
> >
> >     * guix/build/ruby-build-system.scm (install): Use invoke.
> > ---
> >  guix/build/ruby-build-system.scm | 15 +++++++--------
> >  1 file changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/guix/build/ruby-build-system.scm b/guix/build/ruby-build-system.scm
> > index ba0de12..49400b2 100644
> > --- a/guix/build/ruby-build-system.scm
> > +++ b/guix/build/ruby-build-system.scm
> > @@ -143,14 +143,13 @@ GEM-FLAGS are passed to the 'gem' invokation, if present."
> >           (gem-dir (string-append vendor-dir "/gems/" gem-name)))
> >      (setenv "GEM_VENDOR" vendor-dir)
> >
> > -    (or (zero?
> > -         (apply system* "gem" "install" gem-file
> > -                "--verbose"
> > -                "--local" "--ignore-dependencies" "--vendor"
> > -                ;; Executables should go into /bin, not
> > -                ;; /lib/ruby/gems.
> > -                "--bindir" (string-append out "/bin")
> > -                gem-flags))
> > +    (or (apply invoke "gem" "install" gem-file
> > +               "--verbose"
> > +               "--local" "--ignore-dependencies" "--vendor"
> > +               ;; Executables should go into /bin, not
> > +               ;; /lib/ruby/gems.
> > +               "--bindir" (string-append out "/bin")
> > +               gem-flags)
> >          (begin
> >            (let ((failed-output-dir (string-append (getcwd) "/out")))
> >              (mkdir failed-output-dir)
> 
> Hey Efraim,
> 
> Given that invoke raises an exception, I doubt the custom error handling
> code here will ever be executed.
> 
> I guess you could change it to catch the exception, but using system*
> and looking at the exit code seems pretty reasonable to me. What do you
> think?
> 
> Chris

I was looking to get rid of some more of the 'zero? system*' code but I
think you're right. I'll go ahead and revert it and add a note about why
it should be left as-is.

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: 01/01: build-system/ruby: Use invoke.

[Christopher Baines]

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


Efraim Flashner <efraim@flashner.co.il> writes:

> On Thu, Mar 28, 2019 at 06:14:48PM +0000, Christopher Baines wrote:
>>
>> guix-commits@gnu.org writes:
>>
>> > -    (or (zero?
>> > -         (apply system* "gem" "install" gem-file
>> > -                "--verbose"
>> > -                "--local" "--ignore-dependencies" "--vendor"
>> > -                ;; Executables should go into /bin, not
>> > -                ;; /lib/ruby/gems.
>> > -                "--bindir" (string-append out "/bin")
>> > -                gem-flags))
>> > +    (or (apply invoke "gem" "install" gem-file
>> > +               "--verbose"
>> > +               "--local" "--ignore-dependencies" "--vendor"
>> > +               ;; Executables should go into /bin, not
>> > +               ;; /lib/ruby/gems.
>> > +               "--bindir" (string-append out "/bin")
>> > +               gem-flags)
>> >          (begin
>> >            (let ((failed-output-dir (string-append (getcwd) "/out")))
>> >              (mkdir failed-output-dir)
>>
>> Hey Efraim,
>>
>> Given that invoke raises an exception, I doubt the custom error handling
>> code here will ever be executed.
>>
>> I guess you could change it to catch the exception, but using system*
>> and looking at the exit code seems pretty reasonable to me. What do you
>> think?
>>
>> Chris
>
> I was looking to get rid of some more of the 'zero? system*' code but I
> think you're right. I'll go ahead and revert it and add a note about why
> it should be left as-is.

Great, thanks Efraim :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]