From mboxrd@z Thu Jan  1 00:00:00 1970
From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=)
Subject: Re: [PATCH] Add tcsh.
Date: Mon, 04 Feb 2013 23:32:39 +0100
Message-ID: <87txprogl4.fsf@gnu.org>
References: <1360013226-16132-1-git-send-email-tipecaml@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Return-path: <bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org>
Received: from eggs.gnu.org ([208.118.235.92]:53859)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <ludo@gnu.org>) id 1U2Uau-0006zp-RL
	for bug-guix@gnu.org; Mon, 04 Feb 2013 17:32:58 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <ludo@gnu.org>) id 1U2Uag-00075X-Ar
	for bug-guix@gnu.org; Mon, 04 Feb 2013 17:32:43 -0500
Received: from mail3-relais-sop.national.inria.fr ([192.134.164.104]:53095)
	by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from <ludo@gnu.org>)
	id 1U2Uag-000757-41
	for bug-guix@gnu.org; Mon, 04 Feb 2013 17:32:42 -0500
In-Reply-To: <1360013226-16132-1-git-send-email-tipecaml@gmail.com> (Cyril
	Roelandt's message of "Mon, 4 Feb 2013 22:27:06 +0100")
List-Id: Bug reports for GNU Guix <bug-guix.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/bug-guix>,
	<mailto:bug-guix-request@gnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/archive/html/bug-guix>
List-Post: <mailto:bug-guix@gnu.org>
List-Help: <mailto:bug-guix-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/bug-guix>,
	<mailto:bug-guix-request@gnu.org?subject=subscribe>
Errors-To: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org
Sender: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org
To: Cyril Roelandt <tipecaml@gmail.com>
Cc: bug-guix@gnu.org

Cyril Roelandt <tipecaml@gmail.com> skribis:

> This patch adds tcsh. It was a bit hard to make the testsuite work: I dis=
abled a
> few tests that I could not get working during the "check" phase, but it s=
hould
> not be a problem, since they work fine with the installed binary.

Good!

> +    (inputs
> +     `(("autoconf" ,autoconf)
> +       ("coreutils" ,coreutils)
> +       ("ncurses" ,ncurses)
> +       ("patch/skip-tests"
> +        ,(search-patch "tcsh-fix-autotest.patch"))))

In general, rebuilding the build infrastructure with Autoconf &
co. should be avoided for several reasons: we may get it wrong, and it
will yield a rebuild on every Autoconf update.

Could this be easily avoided here?  (I suspect you already tried...)
One option would be to make the patch against =E2=80=98testsuite=E2=80=99 i=
nstead of
against the .at files, with the risk of it no longer being applicable on
the next release.

WDYT?

> +        (lambda* (#:key inputs #:allow-other-keys #:rest args)
> +          (let ((check (assoc-ref %standard-phases 'check)))
> +            ;; Take care of pwd
> +            (substitute* "tests/commands.at" (("/bin/pwd") (which "pwd")=
))
> +            (substitute* "tests/variables.at" (("/bin/pwd") (which "pwd"=
)))

=E2=80=98substitute*=E2=80=99 can be passed a list of files instead of a si=
ngle file.

> +            ;; The .at files create shell scripts without shebangs. Erk.
> +            (substitute* "tests/commands.at"
> +                         (("./output.sh")
> +                          (string-append (which "bash") " output.sh")))

(which "sh") may be more correct (Bash behaves differently depending no
whether it=E2=80=99s invoked as sh or bash.)

Please align the opening parenthesis under the =E2=80=98u=E2=80=99 of =E2=
=80=98substitute*=E2=80=99.

Thanks!

Ludo=E2=80=99.