ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Erik Hatcher" <jakarta-...@ehatchersolutions.com>
Subject Re: Immutability improvement patch
Date Sat, 01 Dec 2001 02:43:58 GMT
----- Original Message -----
From: "Peter Donald" <peter@apache.org>


> almost. You missed a case. When <antcall/> uses propertys internally it
will
> display a warning when it shouldn't. I have reattached your test file with
a
> small demo and you test it by running test8
>
> Im not sure what the best way to fix this is. What do you think?

Well, you got me there!  :)

This one is an interesting one, as it comes close to the discussions about
"privileged" tasks being able to do more to get around things.  But maybe
this idea will work:

How about making Project have another constructor that takes a Hashtable of
properties to initialize it with?  This would require some refactoring in
Ant.initializeProject() so that it builds a temporary Hashtable of
properties and constructs (although the re-working of things might cause
some issues, not sure) the Project object using the properties constructor
instead of the empty one.

Thoughts?  Any drawbacks to another constructor?  I haven't looked at
Project's code to see what is involved there, although hopefully/probably
not much.

It doesn't seem like a straight-forward one-liner to allow <ant>/<antcall>
to essentially "override" properties when we don't want Project to allow
this.... but with a new Project constructor Ant.java could do the dirty work
of constructing a list of properties before constructing the Project by
walking through the ones it wants to carry to the new Ant invocation and
setting the embedded override ones in a temporary Hashtable first, giving
precedence to the embedded ones.

> > I'll go back and check all the e-mails on this topic tomorrow to make
sure
> > all the bases have been covered.  The one loose end is that
getProperties
> > and getUserProperties return back the actual Hashtable object - so to
close
> > that hole we need to make it return a copy instead.  Anyone object to
that
> > copy being made and returned?
>
> +1 to copy ... is there an actual use case for grabbing all properties
> anyways?

How about calls like this:

    ProjectHelper.replaceProperties(project, msg, project.getProperties());
(from Echo.java)

although in this case is there a reason why this substitution isn't done
before calling addText?  Its done automatically on calls to setXXX methods.
Do we not always want property expansion in element text?  I guess <script>
wouldn't be too happy with that, huh?!  :)

<junit> uses it to gather the list of properties to put in the output XML
files.

Your favorite: <script> uses it to expose properties to the BSF engine.  :)

And it shows up in a few other places (getProperties, that is).
getUserProperties is not used except in Ant, Project, and Script, so its not
a big deal, but it should be cloned also before returning of course.

I didn't check to see if there is code that is relying on changing a
property via the handle from getProperties... I sure hope not!

> BTW could you name your patches something more specific (like
> immutability.patch) - I got so many patchfile.txts it is not funny ;)

No problem.  I was just following the rules!

> BTW2 would you consider making immutability.xml part of a Junit testcase ?
;)

Attached.  I actually attached it to my original message too!  :))

> BTW3 would you mind sending apatch for WHATSNEW to describe these features

Sure... will do that this weekend.

> BTW4 maybe even add a FAQ entry because I am sure the new warnings are
bound
> to cause a few quesions ;)

Not a problem.... on my to-do list with the WHATSNEW and a few other
remaining things related to this.  The <antcall> issue is still unresolved -
hopefully some discussion on that will follow so we can figure out the best
way to handle it.

Anyone want to remove Project.unsetProperty?  Or shall I submit a patch for
that removal too?  :)  Thanks Stefan for refactoring your test case for
this.

Property immutability.... perhaps it will now be a concrete reality!  :))

    Erik


Mime
View raw message