DO NOT REPLY [Bug 23090] New: - AbstractFileSet copy-constructor makes shallow copy

[bu...@apache.org]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://nagoya.apache.org/bugzilla/show_bug.cgi?id=23090>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://nagoya.apache.org/bugzilla/show_bug.cgi?id=23090

AbstractFileSet copy-constructor makes shallow copy

           Summary: AbstractFileSet copy-constructor makes shallow copy
           Product: Ant
           Version: 1.5.4
          Platform: Other
        OS/Version: Other
            Status: NEW
          Severity: Normal
          Priority: Other
         Component: Core
        AssignedTo: dev@ant.apache.org
        ReportedBy: gthb@dimon.is


When a FileSet is cloned, using the copy constructor or the clone() method
(which calls the copy constructor internally anyway), the mutable-object
members of AbstractFileSet are copied to the new instance by reference, not
by cloning. These are:

    private PatternSet defaultPatterns = new PatternSet();
    private Vector additionalPatterns = new Vector();
    private Vector selectors = new Vector();

and they are modified through their mutator methods (setExcludes, append,
etc.) by the AbstractFileSet class.

This means the cloned FileSet will be coupled to the original one: changes
made to the original FileSet *after* cloning will be reflected in the new
FileSet. Worse: only *some* changes to the original FileSet affect the clone.
The primitive-type members are not shared, of course, so these methods:

    setDefaultexcludes
    setCaseSensitive
    setFollowSymlinks

do not affect the cloned object when called on the original object after
cloning. Also setDir won't affect things, since the dir member is replaced
by setDir, not modified (File is immutable anyway). But adding excludes or
includes to the original fileset certainly affects the cloned fileset, and
vice versa.

This makes it unnecessarily hard for a custom task developer to do useful
things like allowing the task user to specify one fileset for copying and
then a pattern that specifies which files should have filters applied to
them and which should not. I was trying to do this in a custom task by
invoking two successive copy tasks, first cloning the fileset and adding the
"nofilter" as an exclusion pattern with setExcludes on the clone, and then
cloning the original fileset again and adding the same "nofilter" pattern
as an inclusion pattern with setIncludes on the new clone. This doesn't
work, because the filesets all share the same pattern sets: the inclusion
pattern I tried to add was already an exclusion pattern in the new cloned
fileset, so the inclusion pattern is overriden.

This is not easy to work around without modifying Ant code itself. The
fields are private in AbstractFileSet -- which makes sense I guess, but
amplifies the need for a fix to this.

I can't know for sure, but I doubt that anyone is actually depending on
this behavior in their tasks! I sure hope not :) ... but if so, then this
could at least be worked around e.g. with a deepClone() method. If the
copy constructor and clone method are not fixed, then a warning should
be added to their javadocs pointing out that the resulting clone may have
a few surprises in store for the unwary developer :)

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