Re: [PATCH] Add tcsh.

[Ludovic Courtès]

Cyril Roelandt <address@hidden> skribis:

> This patch adds tcsh. It was a bit hard to make the testsuite work: I 
> disabled a
> few tests that I could not get working during the "check" phase, but it should
> 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 ‘testsuite’ instead 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")))

‘substitute*’ can be passed a list of files instead of a single 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’s invoked as sh or bash.)

Please align the opening parenthesis under the ‘u’ of ‘substitute*’.

Thanks!

Ludo’.