Re: #!/usr/bin/env perl

[Ludovic Courtès]

Andreas Enge <address@hidden> skribis:

> Am Samstag, 26. Januar 2013 schrieb Ludovic Courtès:
>> The function you want to patch ;-) is ‘patch-shebang’ in
>> build/utils.scm.  Since that entails a full rebuild, that’s something
>> for a new ‘core-updates’ branch.
>
> Please find attached a patch for review. I tested it on a few example 
> files, but not yet with a real packet - I am waiting for the full rebuild 
> ;-)

Heh, thanks.

> --- a/guix/build/utils.scm
> +++ b/guix/build/utils.scm
> @@ -426,7 +426,7 @@ bytes transferred and the continuation of the transfer as 
> a thunk."
>           (stat:mtimensec stat)))
>  
>  (define patch-shebang
> -  (let ((shebang-rx (make-regexp "^[[:blank:]]*([[:graph:]]+)(.*)$")))
> +  (let ((shebang-rx (make-regexp 
> "^[[:blank:]]*([[:graph:]]+)[[:blank:]]*([[:graph:]]*)(.*)$")))

I think it should be a + after the second [[:blank:]], because there
should be space between the interpreter name and its argument.

>      (lambda* (file
>                #:optional
>                (path (search-path-as-string->list (getenv "PATH")))
> @@ -465,16 +465,27 @@ FILE are kept unchanged."
>                 (let ((line (false-if-exception (read-line p))))
>                   (and=> (and line (regexp-exec shebang-rx line))
>                          (lambda (m)
> -                          (let* ((cmd (match:substring m 1))
> -                                 (bin (search-path path (basename cmd))))
> +                          (let* ((first (match:substring m 1))
> +                                 (second (match:substring m 2))
> +                                 (third (match:substring m 3))

What about more descriptive names like ‘interp’, ‘arg1’, and ‘rest’?

> +                                 (has-env (string=? (string-pad first 4) 
> "/env"))

Rather (has-env? (string-suffix? "/env" interp)).

> +                                    (if has-env
> +                                      (begin

Please align with the ‘h’ here...

> +                                        (patch p bin
> +                                          (string-append " " second 
> third)))))

... and with the ‘p’ here.

Modulo these details, looks good to me, so feel free to push!

Thanks,
Ludo’.