[PATCH] utils: Preserve makefile shell arguments during patch.

[PATCH] utils: Preserve makefile shell arguments during patch.

[Eric Bavier]

[-- Attachment #1: Type: text/plain, Size: 495 bytes --]

With the talk of potentially merging the core-updates branch, I thought
I should get this patch reviewed, since it triggers a full rebuild.

While working with some Imake-based packages recently, I found that
patch-makefile-SHELL did not like when the SHELL definition contained
arguments.  For example, one package would define::

  SHELL = /bin/sh -e

And patch-makefile-SHELL would turn that into::

  SHELL = /gnu/store/.../bin/bash
  -e

which Make would subsequently choke on.

Thoughts?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-utils-Preserve-makefile-shell-arguments-during-patch.patch --]
[-- Type: text/x-diff, Size: 1335 bytes --]

From e1743a6de5b72cc7150c4e01e79a200e1667f627 Mon Sep 17 00:00:00 2001
From: Eric Bavier <bavier@member.fsf.org>
Date: Wed, 27 Aug 2014 10:26:54 -0500
Subject: [PATCH] utils: Preserve makefile shell arguments during patch.

* guix/build/utils.scm (patch-makefile-SHELL): Preserve shell arguments.
---
 guix/build/utils.scm |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/guix/build/utils.scm b/guix/build/utils.scm
index 2f3dc9c..7ec44fd 100644
--- a/guix/build/utils.scm
+++ b/guix/build/utils.scm
@@ -582,14 +582,14 @@ When KEEP-MTIME? is true, the atime/mtime of FILE are kept unchanged."
 
   (let ((st (stat file)))
    (substitute* file
-     (("^ *SHELL[[:blank:]]*=[[:blank:]]*([[:graph:]]*/)([[:graph:]]+)[[:blank:]]*" _ dir shell)
+     (("^ *SHELL[[:blank:]]*=[[:blank:]]*([[:graph:]]*/)([[:graph:]]+)[[:blank:]]*(.*)$" _ dir shell args)
       (let* ((old (string-append dir shell))
              (new (or (find-shell shell) old)))
         (unless (string=? new old)
           (format (current-error-port)
                   "patch-makefile-SHELL: ~a: changing `SHELL' from `~a' to `~a'~%"
                   file old new))
-        (string-append "SHELL = " new "\n"))))
+        (string-append "SHELL = " new " " args))))
 
    (when keep-mtime?
      (set-file-time file st))))
-- 
1.7.9.5


[-- Attachment #3: Type: text/plain, Size: 17 bytes --]


-- 
Eric Bavier

Re: [PATCH] utils: Preserve makefile shell arguments during patch.

[Ludovic Courtès]

Eric Bavier <ericbavier@gmail.com> skribis:

> While working with some Imake-based packages recently, I found that
> patch-makefile-SHELL did not like when the SHELL definition contained
> arguments.  For example, one package would define::
>
>   SHELL = /bin/sh -e
>
> And patch-makefile-SHELL would turn that into::
>
>   SHELL = /gnu/store/.../bin/bash
>   -e

Oops.

> --- a/guix/build/utils.scm
> +++ b/guix/build/utils.scm
> @@ -582,14 +582,14 @@ When KEEP-MTIME? is true, the atime/mtime of FILE are kept unchanged."
>  
>    (let ((st (stat file)))
>     (substitute* file
> -     (("^ *SHELL[[:blank:]]*=[[:blank:]]*([[:graph:]]*/)([[:graph:]]+)[[:blank:]]*" _ dir shell)
> +     (("^ *SHELL[[:blank:]]*=[[:blank:]]*([[:graph:]]*/)([[:graph:]]+)[[:blank:]]*(.*)$" _ dir shell args)

Please add a line break after the regexp.

>        (let* ((old (string-append dir shell))
>               (new (or (find-shell shell) old)))
>          (unless (string=? new old)
>            (format (current-error-port)
>                    "patch-makefile-SHELL: ~a: changing `SHELL' from `~a' to `~a'~%"
>                    file old new))
> -        (string-append "SHELL = " new "\n"))))
> +        (string-append "SHELL = " new " " args))))

Are you sure the \n is no longer needed?  (I can never remember when it
is matched and when it’s not.)

OK to commit with \n correctly handled.

Thanks!

Ludo’.

Re: [PATCH] utils: Preserve makefile shell arguments during patch.

[Eric Bavier]

Ludovic Courtès writes:

>>        (let* ((old (string-append dir shell))
>>               (new (or (find-shell shell) old)))
>>          (unless (string=? new old)
>>            (format (current-error-port)
>>                    "patch-makefile-SHELL: ~a: changing `SHELL' from `~a' to `~a'~%"
>>                    file old new))
>> -        (string-append "SHELL = " new "\n"))))
>> +        (string-append "SHELL = " new " " args))))
>
> Are you sure the \n is no longer needed?  (I can never remember when it
> is matched and when it’s not.)

It's no longer needed in this case, since the '.' also matches a
newline, so it will end up in 'args'.  

BTW, this is contrary to the GNU Emacs Regular Expressions documentation
which states::

  . (Period)
  is a special character that matches any single character *except a
  newline*...

I'm not sure at what level the newline is being matched, whether in
substitute* or in Guile's regex library.

-- 
Eric Bavier

Please avoid sending me Word or PowerPoint attachments.
See http://www.gnu.org/philosophy/no-word-attachments.html

Re: [PATCH] utils: Preserve makefile shell arguments during patch.

[Ludovic Courtès]

Eric Bavier <ericbavier@gmail.com> skribis:

> Ludovic Courtès writes:
>
>>>        (let* ((old (string-append dir shell))
>>>               (new (or (find-shell shell) old)))
>>>          (unless (string=? new old)
>>>            (format (current-error-port)
>>>                    "patch-makefile-SHELL: ~a: changing `SHELL' from `~a' to `~a'~%"
>>>                    file old new))
>>> -        (string-append "SHELL = " new "\n"))))
>>> +        (string-append "SHELL = " new " " args))))
>>
>> Are you sure the \n is no longer needed?  (I can never remember when it
>> is matched and when it’s not.)
>
> It's no longer needed in this case, since the '.' also matches a
> newline, so it will end up in 'args'.  

Ah OK.  Then please commit, with the long line wrapped.

> BTW, this is contrary to the GNU Emacs Regular Expressions documentation
> which states::
>
>   . (Period)
>   is a special character that matches any single character *except a
>   newline*...
>
> I'm not sure at what level the newline is being matched, whether in
> substitute* or in Guile's regex library.

Guile’s (ice-9 regex) builds on top of libc’s regexps, which are POSIX
extended regular expressions by default.

Thanks,
Ludo’.