[Buildbot-devel] Annoying behavior for ShellCommand under Win32

[Marcus Lindblom]

On 2010-01-27 20:12, fabrice fabrice wrote:
>
> I am integrating Buildbot 7.12 with Python 2.6 and Twisted 8.2 under
> Windows XP
>
> And I find the same annoying behavior than BuildBot 7.5 with Python 2.4
> and Twisted 2.4
>
> For windows user, It is very convenient to start directly the
> VisualStudio devenv command line to make a build
>
> This command line have a parameters like “plateform|configuration”
>
> If I run a simple build factory with a shell command step like
>
> from buildbot.steps.shell import ShellCommand
>
> factory.addStep(ShellCommand(command=r/'echo "Win32|Release"'/))

how about giving it a list instead? (possibly with cmd.exe as first arg)

factory.addStep(ShellCommand(command=["echo", "Win32|Release"]))

Does that help or have I misunderstood your problem?

> The stdio display the wrong result \"Win32|Release\"

What is wrong here? The | or the \" ?

> The problem come from the function cmdLineQuote(s) from
> twisted\python\win32.py
>
> The “quoting under windows” thread is a 5 years old bug :
> http://twistedmatrix.com/trac/ticket/1123
>
> For twisted 2.4, I resolve the issue by “hacking”
> twisted\internet\_dumbwin32proc.py
>
> Let the user make the correct quoting by changing
>
> cmdline = quoteArguments(args)
>
> by
>
> cmdline = " ".join(args)
>
> For twisted 8.2 no clean correction like subprocess.Popen that give the
> choice by providing
>
> List of argument that will be automatically quoted
>
> or
>
> Provide raw string (that is NOT changed)
>
> For my new integration, I like to find a cleaner solution than hacking
> the third party

I'm not really following your comments here.

  * Does twisted merge the list? or not?
  * Do you want the list from buildbot to go intoCreateProcess?
  * or, do you want Twisted to merge the args list correctly before 
calling CreateProcess?

I'd prefer to have a list all the way, and never a space-separated big 
string (this is an abomination that should never have been allowed, I 
spent quite some time removing such calls from the unit-tests). Having a 
pure list makes everything much cleaner.

Cheers,
/Marcus