From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lukas Gradl Subject: Re: [PATCH] Add procmail Date: Mon, 29 Feb 2016 22:35:39 -0600 Message-ID: <20160301043539.GA607@serenity.lan> References: <87io1aqzrd.wl-lgradl@openmailbox.org> <87povffwqm.fsf@igalia.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:50654) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aac2O-0002fc-7I for guix-devel@gnu.org; Mon, 29 Feb 2016 23:35:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aac2J-0005CZ-LV for guix-devel@gnu.org; Mon, 29 Feb 2016 23:35:56 -0500 Received: from smtp26.openmailbox.org ([62.4.1.60]:52413) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aac2J-0005CC-CL for guix-devel@gnu.org; Mon, 29 Feb 2016 23:35:51 -0500 Content-Disposition: inline In-Reply-To: <87povffwqm.fsf@igalia.com> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org To: Andy Wingo Cc: guix-devel@gnu.org On Mon, Feb 29, 2016 at 02:54:09PM +0100, Andy Wingo wrote: > Hi Lukas, >=20 > On Sat 27 Feb 2016 10:16, Lukas Gradl writes: >=20 > > I am new to Guix and Scheme, so I would very much welcome any comment= s > > you might have. >=20 > Welcome :) Thank you! >=20 > > + ;; the following patch fixes an ambiguous definition of > > + ;; getline() in formail.c. The patch is provided by Debian as > > + ;; patch 24 >=20 > We tend to prefer full sentences in comments like this one, including > ending punctuation. Also as a GNU-ism, we are in the church of > two-spaces-after-full-stops :) > OK, sounds good. Fixed. > > + (arguments > > + `(#:phases (alist-replace > > + 'configure > > + (lambda _ > > + (substitute* "Makefile" > > + (("/bin/sh") > > + (which "sh"))) > > + (substitute* "Makefile" > > + (("/usr") > > + (assoc-ref %outputs "out"))) > > + (substitute* "Makefile" > > + (("/bin/rm") > > + (which "rm")))) > > + %standard-phases) >=20 > There are many packages that still use this syntax, but the current > Best Practice=E2=84=A2 is to use `modify-phases'. Search Guix for some > examples. I changed the patch to use 'modify-phases'. Thanks for pointing this out, I like this much better than the 'alist-replace'! > Also, substitute* can do all these clauses in one: >=20 > (substitute* "Makefile" > (("/bin/sh") > (which "sh")) > (("/usr") > (assoc-ref %outputs "out")) > (("/bin/rm") > (which "rm"))) >=20 > The * in substitute* is like the * in let*, meaning the substitutions > will be done in order. Also, the return value of substitute* is > unspecified (see the Guile manual for more), so be sure to return a tru= e > value from this lambda by just ending the lambda with #t. > I updated this. Thank you for the explanation! > > + #:tests? #f)) ; no tests >=20 > Please mention why the tests are disabled. I think procmail comes without tests. There are no targets like 'tests' or 'check' in the Makefile. However, it does perform some test during 'make install' to verify if the filesystem has locking capabilities. These are performed before the actual build, so they do not check if the build was successful. I changed the comment to better explain the situation. >=20 > > + (build-system gnu-build-system) > > + (inputs `(("glibc" ,glibc) > > + ("exim" ,exim))) > > + (home-page "http://www.procmail.org/") > > + (synopsis "Versatile mail delivery agent (MDA)") > > + (description "Procmail is a mail delivery agent (MDA) featuring = support > > +for a variety of mailbox formats such as mbox, mh and maildir. Inco= ming mail > > +can be sorted into separate files/directories and arbitrary commands= can be > > +executed on mail arrival. Procmail is considered stable, but is no = longer > > +maintained.") > > + (license gpl2+))) ; procmail allows to choose the > > + ; nonfree Artistic License 1.0 > > + ; as alternative to the GPL2+. > > + ; This option is not listed here. >=20 > This sort of a comment is best done as a two-semicolon comment, I think= . >=20 > Looking pretty good! > Thank you! This was the last thing keeping me from switching to GuixSD. I will send an updated patch shortly. Thank you for your review! Best, Lukas