From mboxrd@z Thu Jan 1 00:00:00 1970 From: Danny Milosavljevic Subject: Re: [PATCH] gnu: Add perl-db_file. Date: Tue, 26 Jul 2016 21:01:38 +0200 Message-ID: <20160726210138.6b90aef7@scratchpost.org> References: <20160726173513.572a6d5d@scratchpost.org> <20160726183531.GA11804@solar> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:52995) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bS7by-0005jT-0O for guix-devel@gnu.org; Tue, 26 Jul 2016 15:01:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bS7bt-0005hQ-PZ for guix-devel@gnu.org; Tue, 26 Jul 2016 15:01:49 -0400 Received: from dd1012.kasserver.com ([85.13.128.8]:54792) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bS7bt-0005h3-IM for guix-devel@gnu.org; Tue, 26 Jul 2016 15:01:45 -0400 In-Reply-To: <20160726183531.GA11804@solar> 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" To: Andreas Enge Cc: guix-devel@gnu.org, Danny Milosavljevic Hi Andreas, On Tue, 26 Jul 2016 20:35:31 +0200 Andreas Enge wrote: > > * gnu/packages/databases.scm (perl-db_file): New variable. > > The name should be "perl-db-file": we replace all special characters with "-". The package name (not the variable name) was chosen by "guix import cpan". So it should probably be fixed in the CPAN importer, too. > Here I would not rewrite the complete file, but instead use substitute* to > replace "/usr/local/BerkeleyDB" with the assoc-ref. You will find many > examples of this in the repository. Yeah, I thought about it but decided against it - there are very few options in that file, substitute* can't substitute entire lines (or only at the beginning of the line) or entire words (so it's not safe), the user is supposed to set PREFIX and HASH (it's just a coincidence we didn't have to change them) and if we did that then new versions of the package could sneak in new options we wouldn't notice but we should have changed. Better for it to fail instead of silently doing something strange. But I will use substitute* in the next version of the patch - the unit tests should be able to fail for some of the errors. > + (description "DB_File provides access to Berkeley DB version 1.x.") > > Maybe add "perl" somewhere in the description? "provides Perl bindings to..." > or something like that? OK!