fab-user
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Fab-user] Bug in multiline append


From: Jorge Vargas
Subject: Re: [Fab-user] Bug in multiline append
Date: Thu, 21 May 2009 23:56:58 -0400

Actually it's a different bug. and it's inside append/contains

This patch illustrates the issue. contains is calling egrep and my
replace string is.

append('[program:%(app_name)s]' % env, env.home / 'supervisord.conf')

which means egrep is having troubles with the [ ]

diff --git a/fabric/contrib/files.py b/fabric/contrib/files.py
index 587c09e..adf9808 100644
--- a/fabric/contrib/files.py
+++ b/fabric/contrib/files.py
@@ -222,6 +222,6 @@ def append(text, filename, use_sudo=False):
     """
     func = use_sudo and sudo or run
     with settings(warn_only=True):
-        if contains('^' + text, filename, use_sudo=use_sudo):
+        if contains('^' + text.replace("[","\[").replace("]","\]"),
filename, use_sudo=use_sudo):
             return None
     return func("echo '%s' >> %s" % (text.replace("'", r'\''), filename))

now this patch seems a little more proper.

diff --git a/fabric/contrib/files.py b/fabric/contrib/files.py
index 587c09e..04f599c 100644
--- a/fabric/contrib/files.py
+++ b/fabric/contrib/files.py
@@ -201,7 +201,7 @@ def contains(text, filename, exact=False, use_sudo=False):
     if exact:
         text = "^%s$" % text
     return func('egrep "%s" "%s"' % (
-        text.replace('"', r'\"'),
+        text.replace('"', r'\"').replace("[","\[").replace("]","\]"),
         filename.replace('"', r'\"')
     ))


That said I see there is a _shell_escape function in operations.py
that isn't used a lot. maybe the ideal solution will be to have a
bunch of escape functions that should be used to wrap strings.
shell_escape, regexp_escape, etc.

Ideally I think this is a case where the internals should be messy to
leave the external api clean. You do not want regexp in the high level
file functions (append, replace, comment, etc.) or do we?

On Thu, May 21, 2009 at 10:56 PM, Jorge Vargas <address@hidden> wrote:
> On Thu, May 21, 2009 at 10:10 PM, Jeff Forcier <address@hidden> wrote:
>> Yea, I've never used append() on more than one line at a time. Could
>> easily upgrade it to take a list of lines which would result in
>> several internal calls; would that work for you?
>>
> that won't make it an atomic call. What if one one line works?
>
>> Given that it's simply passing through to shell mechanisms, true
>> multiline could get tricky (though honestly I'm not sure why it
>> doesn't work right off, I've done that in an actual shell before
>> without problems).
>>
> rly? I ran the command on the shell and it gave me the wrong output,
> maybe there is some escaping issue?
>
>> -Jeff
>>
>> On Thu, May 21, 2009 at 9:48 PM, Jorge Vargas <address@hidden> wrote:
>>> Hi,
>>>
>>> It seems that the following code fails to run on the server.
>>>
>>> def test_append():
>>>    env.home = path('/home/mae')
>>>    append('''mae was here
>>>    multiline fails
>>>    ''', env.home / 'supervisord.conf')
>>>
>>>
>>> This is the debug output
>>>  run: /bin/bash -l -c "egrep \"^mae was here
>>> sdds
>>>    \" \"/home/mae/supervisord.conf\""
>>>
>>> and the whole file gets printed to out
>>>
>>> work around use several call to append :)
>>>
>>>
>>> _______________________________________________
>>> Fab-user mailing list
>>> address@hidden
>>> http://lists.nongnu.org/mailman/listinfo/fab-user
>>>
>>
>




reply via email to

[Prev in Thread] Current Thread [Next in Thread]