On Mon, 17 Nov 2014, Peter Wu wrote:

Hi, sorry for my delay in responding to this.

>> Is this the only use case for this patch set? It's pretty intrusive,
>
> Yes it is intrusive in the sense that it touches many lines.

Not only that. It also completely changes how it invokes programs and tools.
It is also +180 lines of code.

>> makes it more difficult to see the control flow,
>
> How so? It is now more clear whether I/O redirection occurs or not.

I'm with Dan on this. I find this way of doing things MUCH harder to read and
figure out. Plain command lines is more what I am (and possibly others are)
used to.

I'm struggling to see how it is worth changing the script to this method. On
the contrary, I have a suspicion that it will give me reasons for swearing
down the road.

> When compiling with debugging enabled, it is already possible to override
> the host name.

Yeps, and I figure most developers can easily just build and test debug
enabled. I know I do that all the time.

> For the future it seems a better idea to explicitly format the command as a
> list of program and options rather than a command string. That would result
> in less surprises when the command contains meta-characters. The shell is
> not needed anyway, perl can perfectly execute commands by itself.

Still, I don't see the downside in letting the shell run them. Why is that
bad? In fact, the existing way allows for running the tests remotely with the
commented ssh trick that your patch removes.

I'm also not happy with how this way for example removes the ability to set
the order of options (unless I misunderstood something) and the stdout/stderr
tricks are really special-looking.

So, I don't think the benefits of this patch series outweigh the introduced
complexity.

-- 
  / daniel.haxx.se
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html