You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ant.apache.org by an...@apache.org on 2006/11/01 06:11:28 UTC

svn commit: r469781 - in /ant/core/trunk/src/main/org/apache/tools/ant: ProjectComponent.java types/AbstractFileSet.java types/DataType.java types/PatternSet.java

Author: antoine
Date: Tue Oct 31 21:11:27 2006
New Revision: 469781

URL: http://svn.apache.org/viewvc?view=rev&rev=469781
Log:
reactivate mergePatterns in AbstractFileSet, implement clone in DataType and ProjectComponent

Modified:
    ant/core/trunk/src/main/org/apache/tools/ant/ProjectComponent.java
    ant/core/trunk/src/main/org/apache/tools/ant/types/AbstractFileSet.java
    ant/core/trunk/src/main/org/apache/tools/ant/types/DataType.java
    ant/core/trunk/src/main/org/apache/tools/ant/types/PatternSet.java

Modified: ant/core/trunk/src/main/org/apache/tools/ant/ProjectComponent.java
URL: http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/ant/ProjectComponent.java?view=diff&rev=469781&r1=469780&r2=469781
==============================================================================
--- ant/core/trunk/src/main/org/apache/tools/ant/ProjectComponent.java (original)
+++ ant/core/trunk/src/main/org/apache/tools/ant/ProjectComponent.java Tue Oct 31 21:11:27 2006
@@ -18,12 +18,13 @@
 
 package org.apache.tools.ant;
 
+
 /**
  * Base class for components of a project, including tasks and data types.
  * Provides common facilities.
  *
  */
-public abstract class ProjectComponent {
+public abstract class ProjectComponent implements Cloneable {
 
     /**
      * Project object of this component.
@@ -121,5 +122,15 @@
                 System.err.println(msg);
             }
         }
+    }
+    /**
+     * @since Ant 1.7
+     * @return a shallow copy of this projectcomponent.
+     */
+    public Object clone() throws CloneNotSupportedException {
+        ProjectComponent pc = (ProjectComponent) super.clone();
+        pc.setLocation(getLocation());
+        pc.setProject(getProject());
+        return pc;
     }
 }

Modified: ant/core/trunk/src/main/org/apache/tools/ant/types/AbstractFileSet.java
URL: http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/ant/types/AbstractFileSet.java?view=diff&rev=469781&r1=469780&r2=469781
==============================================================================
--- ant/core/trunk/src/main/org/apache/tools/ant/types/AbstractFileSet.java (original)
+++ ant/core/trunk/src/main/org/apache/tools/ant/types/AbstractFileSet.java Tue Oct 31 21:11:27 2006
@@ -459,16 +459,12 @@
         }
         ds.setBasedir(dir);
 
-        final int count = additionalPatterns.size();
-        for (int i = 0; i < count; i++) {
-            Object o = additionalPatterns.elementAt(i);
-            defaultPatterns.append((PatternSet) o, p);
-        }
+        PatternSet ps = mergePatterns(p);
         p.log(getDataTypeName() + ": Setup scanner in dir " + dir
-            + " with " + defaultPatterns, Project.MSG_DEBUG);
+            + " with " + ps, Project.MSG_DEBUG);
 
-        ds.setIncludes(defaultPatterns.getIncludePatterns(p));
-        ds.setExcludes(defaultPatterns.getExcludePatterns(p));
+        ds.setIncludes(ps.getIncludePatterns(p));
+        ds.setExcludes(ps.getExcludePatterns(p));
         if (ds instanceof SelectorScanner) {
             SelectorScanner ss = (SelectorScanner) ds;
             ss.setSelectors(getSelectors(p));
@@ -803,8 +799,7 @@
         if (isReference()) {
             return getRef(p).mergePatterns(p);
         }
-        PatternSet ps = new PatternSet();
-        ps.append(defaultPatterns, p);
+        PatternSet ps = (PatternSet) defaultPatterns.clone();
         final int count = additionalPatterns.size();
         for (int i = 0; i < count; i++) {
             Object o = additionalPatterns.elementAt(i);

Modified: ant/core/trunk/src/main/org/apache/tools/ant/types/DataType.java
URL: http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/ant/types/DataType.java?view=diff&rev=469781&r1=469780&r2=469781
==============================================================================
--- ant/core/trunk/src/main/org/apache/tools/ant/types/DataType.java (original)
+++ ant/core/trunk/src/main/org/apache/tools/ant/types/DataType.java Tue Oct 31 21:11:27 2006
@@ -37,7 +37,7 @@
  * but not &lt;path&gt;).</p>
  *
  */
-public abstract class DataType extends ProjectComponent {
+public abstract class DataType extends ProjectComponent implements Cloneable {
 
     /**
      * The description the user has set.
@@ -352,5 +352,18 @@
         return d == null ? getDataTypeName() : getDataTypeName() + " " + d;
     }
 
+    /**
+     * @since Ant 1.7
+     * @return a shallow copy of this DataType.
+     */
+    public Object clone() throws CloneNotSupportedException {
+        DataType dt = (DataType) super.clone();
+        dt.setDescription(getDescription());
+        if (getRefid() != null) {
+	    dt.setRefid(getRefid());
+	}
+        dt.setChecked(isChecked());
+        return dt;
+    }
 }
 

Modified: ant/core/trunk/src/main/org/apache/tools/ant/types/PatternSet.java
URL: http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/ant/types/PatternSet.java?view=diff&rev=469781&r1=469780&r2=469781
==============================================================================
--- ant/core/trunk/src/main/org/apache/tools/ant/types/PatternSet.java (original)
+++ ant/core/trunk/src/main/org/apache/tools/ant/types/PatternSet.java Tue Oct 31 21:11:27 2006
@@ -497,19 +497,15 @@
      * @return a clone of this patternset.
      */
     public Object clone() {
-        if (isReference()) {
-            return getRef(getProject()).clone();
-        } else {
-            try {
-                PatternSet ps = (PatternSet) super.clone();
-                ps.includeList = (Vector) includeList.clone();
-                ps.excludeList = (Vector) excludeList.clone();
-                ps.includesFileList = (Vector) includesFileList.clone();
-                ps.excludesFileList = (Vector) excludesFileList.clone();
-                return ps;
-            } catch (CloneNotSupportedException e) {
-                throw new BuildException(e);
-            }
+        try {
+            PatternSet ps = (PatternSet) super.clone();
+            ps.includeList = (Vector) includeList.clone();
+            ps.excludeList = (Vector) excludeList.clone();
+            ps.includesFileList = (Vector) includesFileList.clone();
+            ps.excludesFileList = (Vector) excludesFileList.clone();
+            return ps;
+        } catch (CloneNotSupportedException e) {
+            throw new BuildException(e);
         }
     }
 



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


Re: svn commit: r469781 - in /ant/core/trunk/src/main/org/apache/tools/ant: ProjectComponent.java types/AbstractFileSet.java types/DataType.java types/PatternSet.java

Posted by Dominique Devienne <dd...@gmail.com>.
> these Clone methods are shallow anyway.

Ah, I see your point. In this case, cloning the referencer or the
referenced makes little difference.

> Do you think this level of detail should be in the javadoc ?

A little note about this wouldn't hurt, as this is subtle stuff,
although in practice it should never come into play, so don't waste time on it.

Thanks for all your answers. Cheers, --DD

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


Re: svn commit: r469781 - in /ant/core/trunk/src/main/org/apache/tools/ant: ProjectComponent.java types/AbstractFileSet.java types/DataType.java types/PatternSet.java

Posted by Antoine Levy-Lambert <an...@gmx.de>.
Hello Dominique,

these Clone methods are shallow anyway.

The only bit which is not shallow is that the Vectors held internally by the patternset are cloned.

The vector elements themselves are not cloned.

So you can add a PatternSet.NameEntry to a clone of a PatternSet without changing the original. 

But if you change a PatternSet.NameEntry in the clone, which is coming from the original, this will affect the original.

Do you think this level of detail should be in the javadoc ?

Regards,
Antoine

-------- Original-Nachricht --------
Datum: Wed, 1 Nov 2006 09:57:15 -0600
Von: "Dominique Devienne" <dd...@gmail.com>
An: "Ant Developers List" <de...@ant.apache.org>
Betreff: Re: svn commit: r469781 - in /ant/core/trunk/src/main/org/apache/tools/ant: ProjectComponent.java types/AbstractFileSet.java types/DataType.java types/PatternSet.java

> > I have decided that PatternSet.clone() should return also a reference
> rather than the referenced object if the object is a reference.
> >
> > When the object is used, the different getters will bring the user back
> to the referenced object.
> 
> The only little concern I had with this approach was that if the
> referenced object was changed between the moment the referencer is
> cloned, and the moment it effectively accesses the cloned referencer
> instance (and thus the modified referenced instance), accessing the
> cloned reference may gives a false sense of "security", the user
> thinking the clone is private and won't change, despite the fact that
> it's still "just" a proxy (reference in Ant terms) to another
> instance, that one not cloned.
> 
> Confusing statement, I know ;-) --DD
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org

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


Re: svn commit: r469781 - in /ant/core/trunk/src/main/org/apache/tools/ant: ProjectComponent.java types/AbstractFileSet.java types/DataType.java types/PatternSet.java

Posted by Dominique Devienne <dd...@gmail.com>.
> I have decided that PatternSet.clone() should return also a reference rather than the referenced object if the object is a reference.
>
> When the object is used, the different getters will bring the user back to the referenced object.

The only little concern I had with this approach was that if the
referenced object was changed between the moment the referencer is
cloned, and the moment it effectively accesses the cloned referencer
instance (and thus the modified referenced instance), accessing the
cloned reference may gives a false sense of "security", the user
thinking the clone is private and won't change, despite the fact that
it's still "just" a proxy (reference in Ant terms) to another
instance, that one not cloned.

Confusing statement, I know ;-) --DD

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


Re: svn commit: r469781 - in /ant/core/trunk/src/main/org/apache/tools/ant: ProjectComponent.java types/AbstractFileSet.java types/DataType.java types/PatternSet.java

Posted by Antoine Levy-Lambert <an...@gmx.de>.
Hello Dominique,

the reason for the changes was to make the mergePatterns(Project) method in AbstractFileSet work without bugs.

Before, it was only copying the patterns which are valid in respect to their if and unless attributes and the properties being set in the project.

I have decided that PatternSet.clone() should return also a reference rather than the referenced object if the object is a reference.

When the object is used, the different getters will bring the user back to the referenced object.

You will notice that in DataType.clone, I am only setting the refid field if it is not null, otherwise I had some tooManyAttributes exceptions creeping up.

Regards,

Antoine

-------- Original-Nachricht --------
Datum: Wed, 1 Nov 2006 07:43:10 -0600
Von: "Dominique Devienne" <dd...@gmail.com>
An: "Ant Developers List" <de...@ant.apache.org>
Betreff: Re: svn commit: r469781 - in /ant/core/trunk/src/main/org/apache/tools/ant: ProjectComponent.java types/AbstractFileSet.java types/DataType.java types/PatternSet.java

> > -public abstract class DataType extends ProjectComponent {
> > +public abstract class DataType extends ProjectComponent implements
> Cloneable {
> > +    public Object clone() throws CloneNotSupportedException {
> > +        DataType dt = (DataType) super.clone();
> > +        dt.setDescription(getDescription());
> > +        if (getRefid() != null) {
> > +           dt.setRefid(getRefid());
> > +       }
> > +        dt.setChecked(isChecked());
> > +        return dt;
> > +    }
> >  }
> >
> > --- ant/core/trunk/src/main/org/apache/tools/ant/types/PatternSet.java
> (original)
> > +++ ant/core/trunk/src/main/org/apache/tools/ant/types/PatternSet.java
> Tue Oct >     public Object clone() {
> > -        if (isReference()) {
> > -            return getRef(getProject()).clone();
> > -        } else {
> > ...
> > +        try {
> > +            PatternSet ps = (PatternSet) super.clone();
> 
> You've removed the isReference check Antoine. Did you really intend to
> clone the reference'r rather than the reference'd? I prefer to ask,
> since I don't understand the reason for these changes (which is not to
> say I context the changes). --DD
> 

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


Re: svn commit: r469781 - in /ant/core/trunk/src/main/org/apache/tools/ant: ProjectComponent.java types/AbstractFileSet.java types/DataType.java types/PatternSet.java

Posted by Dominique Devienne <dd...@gmail.com>.
> -public abstract class DataType extends ProjectComponent {
> +public abstract class DataType extends ProjectComponent implements Cloneable {
> +    public Object clone() throws CloneNotSupportedException {
> +        DataType dt = (DataType) super.clone();
> +        dt.setDescription(getDescription());
> +        if (getRefid() != null) {
> +           dt.setRefid(getRefid());
> +       }
> +        dt.setChecked(isChecked());
> +        return dt;
> +    }
>  }
>
> --- ant/core/trunk/src/main/org/apache/tools/ant/types/PatternSet.java (original)
> +++ ant/core/trunk/src/main/org/apache/tools/ant/types/PatternSet.java Tue Oct >     public Object clone() {
> -        if (isReference()) {
> -            return getRef(getProject()).clone();
> -        } else {
> ...
> +        try {
> +            PatternSet ps = (PatternSet) super.clone();

You've removed the isReference check Antoine. Did you really intend to
clone the reference'r rather than the reference'd? I prefer to ask,
since I don't understand the reason for these changes (which is not to
say I context the changes). --DD

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