[Buildbot-commits] [SPAM] Re: [Buildbot] #377: funny arg splitting for cmd.exe in some cases but not others

[Buildbot]

#377: funny arg splitting for cmd.exe in some cases but not others
-------------------+--------------------------------------------------------
Reporter:  zooko   |        Owner:       
    Type:  defect  |       Status:  new  
Priority:  major   |    Milestone:  0.8.0
 Version:  0.7.9   |   Resolution:       
Keywords:          |  
-------------------+--------------------------------------------------------
Changes (by dustin):

  * milestone:  0.8.+ => 0.8.0


Comment:

 Discussion from #buildbot:

 {{{
 14:54 < ddunbar> at the risk of being boringly repetitive, the simple
 solution is to make ShellCommand create a .bat file on the slave
 14:54 < ddunbar> imho, trying to understand cmd.exe is a lost cause
 14:54 < djmitche> ddunbar: hmm
 14:54 < djmitche> that just seems too simple
 14:55 < ddunbar> this is what msbuild etc do, and there is a reason
 14:55 < djmitche> does that introduce other metacharacters? %-thingies?
 14:55 < maruel> ddunbar: not a bad idea per se
 14:55 < djmitche> I'm not saying it's *wrong*, I'm just amazed the
 solution is that simple .. I kinda thought cmd.exe basically "executed"
 each line of a batch file?
 14:56 < ddunbar> see #377 (and #595) if thats not whats already talked
 about
 14:56 < exarkun> djmitche: I think it works because it makes it really
 obvious which things you need to do cmd.exe quoting for and which things
 you need to do CreateProcess quoting for.
 14:56 < exarkun> djmitche: Instead of mixing them together and making it
 really hard for everyone to understand.
 14:56 < ddunbar> I think its actually impossible to do the right escaping
 14:57 < exarkun> ddunbar: Surely not in any particular case.
 14:57 < exarkun> ddunbar: Just in general.
 14:57 < ddunbar> that doesn't make sense
 14:57 < ddunbar> if its possible in every particular case it is possible
 in general :)
 14:57 < exarkun> mmm.  no.
 14:57 < exarkun> If you know what program you're running, and you know
 what its quoting rules are, then you can do it right.
 14:57 < exarkun> If you're running an arbitrary program with arbitrary
 quoting rules, then you can't.
 14:57 < ddunbar> thats not true
 14:57 < exarkun> Sure it is.
 14:57 < ddunbar> or rather, thats not what I am saying
 14:58 < djmitche> woah, before we go all metaphysical here.. can someone
 with access to a windows box hack up the slave side of ShellCommand to do
 this?
 14:58 < ddunbar> I think there are arguments (strings of characters) which
 you cannot pass to programs, if you call createprocess which calls cmd.exe
 14:58 < exarkun> ddunbar: Ah, I see.
 14:58 < djmitche> ddunbar: that's what it looks like is going on here
 14:58 < ddunbar> I could be wrong, I just think that their quoting schemes
 aren't strong enough
 14:58 < djmitche> in particular, |
 14:58 < exarkun> ddunbar: I'm not sure if that's true.
 14:58 < ddunbar> right
 14:58 < ddunbar> I tried really hard to quote | correctly and failed
 14:58 < exarkun> Did you try ^?
 14:59 < ddunbar> I don't remember
 14:59 < exarkun> ^| should work for that.
 14:59 < djmitche> so it seems like ShellCommand on win32 should *always*
 write to a .bat and run it?
 14:59 < ddunbar> giving up and moving to .bat files made everything much
 simpler
 14:59 < djmitche> or is that appreciably slower?
 14:59 < maruel> djmitche: it wouldn't
 14:59 < catlee> seems wrong somehow
 14:59 < exarkun> djmitche: everything on windows is already so slow you
 probably won't notice.
 14:59 < catlee> lol
 14:59 < djmitche> hah
 14:59 < ddunbar> true
 14:59 < djmitche> anyway, I'd like this to be a part of ShellCommand ..
 the question is which way should the default go?
 15:00 < exarkun> the default should be to work correctly
 15:00 < djmitche> .. a *optional* part of ..
 15:00 < exarkun> which is probably closer to the .bat thing than the
 current behavior
 15:00 < djmitche> which means .bat all the time? or does that just
 introduce some other edge cases?
 15:00 < exarkun> beats me :)
 15:00 < djmitche> let's do .bat as the default in 0.8.0, and change it
 back if we find problems
 15:00 < exarkun> someone should try and see
 15:00 < djmitche> sound like a plan?
 15:00 < ddunbar> I would heart that
 15:01 < exarkun> it shouldn't be very hard to write a comprehensive set of
 tests
 }}}

 The plan was subsequently revised to not make this the *default* in 0.8.0,
 but we should have optional support available, at least.  Something like
 {{{ShellCommand(.., useBatch=True)}}}.  The NEWS should state that this
 will become the default in 0.8.1, and those who do *not* want to use a
 batch file for certain commands should include {{{useBatch=False}}} now,
 even though it's the current default.

 Now, I can put some code together, but I have no capacity to execute it,
 which means I'm not so helpful.  So who's going to write this?

-- 
Ticket URL: <http://buildbot.net/trac/ticket/377#comment:16>
Buildbot <http://buildbot.net/>
Buildbot: build/test automation