ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From DJohn...@desknetinc.com
Subject Re: SSHSession design (was: Re: First-time contributor - advice needed)
Date Thu, 09 Aug 2007 21:49:51 GMT
Dominique Devienne wrote:
>On 8/9/07, DJohnson@desknetinc.com <DJohnson@desknetinc.com> wrote:
>> Steve Loughran said:
>> >2. I'd put the nested commands into a <sequence>, the way we did in
>> ><macrodef>. This makes it clear it is sequential, and it leaves room 
to
>> >add new things alongside <localtunnel>.
>
>> I'd like to discuss further your suggestion of incorporating <sequence>
>> (<sequential>, according to 1.7.0 doc??) into sshsession to hold the
>> nested tasks.  Please explain further why this is prefereable.
>
>Forward compatibility. Right now you allow:
>
><sshsession ...>
><LocalTunnel ... />
>any tasks...
></sshsession>
>
>So someone somewhere use <foo/> in "any tasks...", and later on you
>want to enhance <sshsession> with a new <foo/> element. That someone's
>build would now break with your new enhanced version.
>
>Contrast this with this model:
>
><sshsession ...>
><LocalTunnel ... />
><sequential>
>any tasks...
></sequential>
></sshsession>
>
>Now you are free to add new elements directly under <sshsession>
>without potentially breaking any builds.
>
>Code-wise, composing a Sequential instance is better/simpler IMHO than
>inheriting TaskContainer (or Sequential).

I don't know about simpler code, but certainly not much harder, and 
probably better, for the reason you cite.  I see your argument for 
avoiding conflicting/ambiguous <foo/> elements, and after looking over 
MacroDef, I see that the <Sequential> there is built by MacroDef, not 
the same as the <Sequential> Task, so it is still free to process some 
future <sshremotecommand/> (renamed from my earlier <command/> suggestion, 

to eliminate the future conflict issue you raise) nested along with 
other tasks *within* <Sequential>.

>
>An alternative in line with your proposed extension is to move the
>inheritance away from <sshsession> and into <localtunnel> and
><remotetunnel>, to allow:
>
><sshsession ...>
><LocalTunnel ...>
>any tasks...
></LocalTunnel>
><RemoteTunnel ...>
>any tasks...
></RemoteTunnel>
></sshsession>
>
>Here as well, there's no possible conflict with a user script using an
>element name you'd want to add to <sshsession> itself.

I'm not keen on this.  It restricts you to having only one tunnel at a 
time.  I think I'd like to establish all tunnels up front, then any of
the nested tasks can use any of the tunnels, in any order.  There is
already logic checking that you aren't defining conflicting tunnels
(unique lport on localtunnel and unique rport on remotetunnel) with 
the expectation that you may have multiple <localtunnel/> and/or 
<remotetunnel/> elements for a single <sshsession>.

>
>I hope this helps. --DD
>

Yes, thanks.

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message