ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nigel Magnay <Nigel.Mag...@Parsec.co.uk>
Subject RE: [PATCH] [FIX] RE: cvs commit: jakarta-ant/src/main/org/apache /tools/ant/taskdefs/optional/net TelnetTask.java
Date Wed, 27 Mar 2002 09:25:58 GMT
> Nice tests.
> 
> There is an interesting issue here regarding the property translation
> behavior.
> 
> 1. the old behaviour is a bug, pure and simple, right?

Weeeeelll... possibly. I would say yes, but as far as I'm
aware, it would be the only task that expands properties
in an XML node rather than an attribute..

> 2. if we fix it, then all new code will be written expecting 
> translation;
> escaping dollar signs &c
> 3. if we dont make the fix a default, then we ar just going 
> to cause grief
> for people down the line
> 4. but if we do fix it, we cant guarantee that you can write 
> a build file
> which works on both 1.5. and earlier versions. Actually you 
> can, you just
> dont nest the text, you use <read string=>; if you know what 
> you are doing
> you can do a cross-version <telnet> that works
> 

I'd buy all that. There was a debate however when the patch got
submitted as revision 1.8 that this would break earlier build
files..

> I want to patch how we expand properties overally, so that $$ 
> -> $ and $ ->
> $. with this patch in, we eliminate the 'doesnt exclude 
> class$subclass'
> bugreps, and any <telnet> use where you do something like
>  <read>ready$</read> will still works. So all that we would 
> break would be
> the ${PATH} use case.
> 

I believe that is what should work in the current version in
CVS - it's doing project.replaceProperties(s); (though I believe
it should do getProject().replaceProperties(s) as it wasn't
building for me..).

> But here is the best bit, what will the result of  
> <read>${PATH}</read> be
> if PATH is undefined? That's right: it'll be ${PATH}
> 
> So we will only have a problem if the user is using 
> ${something} and <isset
> property="something"> holds. Which is kind of insidious; that could
> introduce intermittent failures in obscure cases.
> 

A bit of 'buyer beware' in the docs should be enough for this - 
I think ANT users expect ${thing} to be treated as a property
(hence your statement (1)) - tell them to be careful out there.

Actually, the worst case is probably if you need to do an *untranslated*
<write>set PATH=${PATH}</write>
and PATH is set as a property. One fix would be to have a way for the write
task to not send CRLF at the end of the string, so you could then do

<write nocrlf="true">set PATH=$</write>
<write>{PATH}</write>

> So, I am fairly tempted to move to a model where we always 
> expand properties
> in the telnet task, we doc it as a 'this may break stuff' 
> flag and move on.
> If we are going to have a switch, make it in the task nto the 
> read and write
> datatypes, and then add a way to set it from a property
> ant.telnet.brokenMode so that you can turn it on without 
> changing the build
> file.
> 

Sounds good to me... I can probably cook up a patch if this
is the way we want it to go...

Rgds,
Nigel

--
To unsubscribe, e-mail:   <mailto:ant-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:ant-dev-help@jakarta.apache.org>


Mime
View raw message