From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ricardo Wurmus Subject: Re: [PATCH] gnu: postgresql: Substitute hard coded "/bin/sh". Date: Thu, 24 Mar 2016 22:24:32 +0100 Message-ID: <871t6z36y7.fsf@elephly.net> References: <1458853944-26415-1-git-send-email-jmd@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:38837) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ajCkH-0004Oc-2R for guix-devel@gnu.org; Thu, 24 Mar 2016 17:24:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ajCkG-0000Mi-8n for guix-devel@gnu.org; Thu, 24 Mar 2016 17:24:45 -0400 In-reply-to: <1458853944-26415-1-git-send-email-jmd@gnu.org> 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: John Darrington Cc: guix-devel@gnu.org Hi John, thanks for the patch! While the patch looks generally okay, I do want to make a few nit-picking comments. > * gnu/packages/databses.scm (postgresql): substitute /bin/sh > with location of bash binary. ^ \_ why two spaces? > (build-system gnu-build-system) > + (arguments > + `(#:phases > + (modify-phases %standard-phases > + (add-before > + 'configure 'patch-/bin/sh we usually keep these on the same line as “add-before”. > + (lambda* (#:key inputs #:allow-other-keys) > + (let ((bash (assoc-ref inputs "bash"))) > + ;; Refer to the actual shell. > + (substitute* '("src/bin/pg_ctl/pg_ctl.c" > + "src/bin/psql/command.c") > + (("/bin/sh") > + (string-append bash "/bin/sh"))))))))) I think we could just do this: (lambda _ (substitute* '(...) (("/bin/sh") (which "sh"))) #t) Also note the final “#t” because “substitute*” has an undetermined return value. What do you think? ~~ Ricardo