Subject: bug#32153: closed (Re: [bug#32153] [PATCH 1/2] ruby-build-system: Error or return #t from all phases.)

From: [email-masked] (GNU bug Tracking System)
Date: Sun, 15 Jul 2018 21:28:03 +0000

Your bug report

#32153: [PATCH 0/2]: ruby-build-system: Error or return #t from all phases.

which was filed against the guix-patches package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
32153: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=32153
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: [bug#32153] [PATCH 1/2] ruby-build-system: Error or return #t from all phases.
From: Christopher Baines <[email-masked]>
To: Marius Bakke <[email-masked]>
Cc: [email-masked]
Date: Sun, 15 Jul 2018 22:27:03 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

Marius Bakke <[email-masked]> writes:

> Christopher Baines <[email-masked]> writes:
>
>> Previously, if the tests didn't pass, the check phase would evaluate to #f,
>> but the package would be built sucessfully. This changes all the phases to
>> raise exceptions if errors are encountered, and return #t otherwise.
>>
>> This involves using invoke rather than system*, so that exceptions are raised
>> if the program exits with a status other than 0, and also returning #t at the
>> end of functions.
>>
>> * gnu/build/ruby-build-system.scm (unpack): Use invoke rather than system*,
>> and return #t at the end.
>> (build, check): Use invoke rather than system*.
>> (install): Remove the use of "and", and rewrite the error handling to raise an
>> exception.
>> (wrap): Return #t.
>
> Thanks!  The changes LGTM.  I will suggest a micro-improvement not
> related to this patch since it was there from before:
>
>> +    ;; For gems with native extensions, several Makefile-related files
>> +    ;; are created that contain timestamps or other elements making
>> +    ;; them not reproducible.  They are unnecessary so we remove them.
>> +    (if (file-exists? (string-append vendor-dir "/ext"))
>> +        (begin
>> +          (for-each (lambda (file)
>> +                      (log-file-deletion file)
>> +                      (delete-file file))
>> +                    (append
>> +                     (find-files (string-append vendor-dir "/doc")
>> +                                 "page-Makefile.ri")
>> +                     (find-files (string-append vendor-dir "/extensions")
>> +                                 "gem_make.out")
>> +                     (find-files (string-append vendor-dir "/ext")
>> +                                 "Makefile")))))
>> +
>> +    #t))
>
> I was confused whether the #t was the "else" clause for the "if"
> expression until I realized it didn't have one.
>
> Could you turn this into a (when (file-exists? ...) (for-each ...))
> while at it?  It also makes the (begin ...) redundant.

I've made this change, and pushed the patches to master, thanks again
for taking a look :)

Chris

Subject: [PATCH 0/2]: ruby-build-system: Error or return #t from all phases.
From: Christopher Baines <[email-masked]>
To: [email-masked]
Date: Sat, 14 Jul 2018 12:05:43 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

I'm trying to continue along with the Rails packaging (#30689), but I
noticed that currently if the tests fail for packages using the ruby
build system, then the package build doesn't fail.

These patches should get most of the packages using the ruby build
system to raise exceptions when there are errors, and return #t
otherwise.

I'm hopeful that this can be merged directly in to master, I build 180
packages in not that much time at all to test this change [1].

1: ./pre-inst-env guix package -s ruby- | recsel -P name | xargs ./pre-inst-env guix build

Christopher Baines (2):
  ruby-build-system: Error or return #t from all phases.
  gnu: ruby-options: Return #t from set-LIB phase.

 gnu/packages/ruby.scm            |   3 +-
 guix/build/ruby-build-system.scm | 109 ++++++++++++++++---------------
 2 files changed, 58 insertions(+), 54 deletions(-)