Make cvstagdiff work properly for multiple modules and modules with spaces in their name. Open for optimization. PR 33501.

git-svn-id: https://svn.apache.org/repos/asf/ant/core/trunk@705260 13f79535-47bb-0310-9956-ffa450edef68

WHATSNEW

@@ -257,6 +257,10 @@ Fixed bugs:
    command line arguments in some cases.
    Bugzilla Report 31601.
 
+ * <cvstagdiff> crippled file names and could miss some entries if
+   multiple modules have been specified.
+   Bugzilla Report 35301.
+
 Other changes:
 --------------
 

docs/manual/CoreTasks/cvstagdiff.html

@@ -133,7 +133,7 @@ operation may fail when using such an incompatible client.
   </tr>
 </table>
 
-<h3>Examples</h3>
+<h3>Parameters specified as nested elements</h3>
 
 <h4>module</h4>
 

src/main/org/apache/tools/ant/taskdefs/cvslib/CvsTagDiff.java

@@ -25,14 +25,18 @@ import java.io.IOException;
 import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
 import java.io.UnsupportedEncodingException;
-import java.util.Vector;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Iterator;
 import java.util.StringTokenizer;
+import java.util.Vector;
 
 import org.apache.tools.ant.BuildException;
 import org.apache.tools.ant.Project;
 import org.apache.tools.ant.taskdefs.AbstractCvsTask;
 import org.apache.tools.ant.util.DOMElementWriter;
 import org.apache.tools.ant.util.DOMUtils;
+import org.apache.tools.ant.util.CollectionUtils;
 import org.apache.tools.ant.util.FileUtils;
 
 import org.w3c.dom.Document;
@@ -141,6 +145,11 @@ public class CvsTagDiff extends AbstractCvsTask {
      */
     private boolean ignoreRemoved = false;
 
+    /**
+     * temporary list of package names.
+     */
+    private List packageNames = new ArrayList();
+
     /**
      * The package/module to analyze.
      * @param p the name of the package to analyse
@@ -232,16 +241,28 @@ public class CvsTagDiff extends AbstractCvsTask {
             addCommandArgument("-D");
             addCommandArgument(myendDate);
         }
-        // support multiple packages
-        StringTokenizer myTokenizer = new StringTokenizer(mypackage);
-        while (myTokenizer.hasMoreTokens()) {
-            addCommandArgument(myTokenizer.nextToken());
-        }
+
         // force command not to be null
         setCommand("");
         File tmpFile = null;
         try {
-            tmpFile = FILE_UTILS.createTempFile("cvstagdiff", ".log", null, true, true);
+            if (mypackage != null) {
+                // support multiple packages
+                StringTokenizer myTokenizer = new StringTokenizer(mypackage);
+                while (myTokenizer.hasMoreTokens()) {
+                    String pack = myTokenizer.nextToken();
+                    packageNames.add(pack);
+                    addCommandArgument(pack);
+                }
+            }
+            for (Iterator iter = getModules().iterator(); iter.hasNext();) {
+                AbstractCvsTask.Module m = (AbstractCvsTask.Module) iter.next();
+                packageNames.add(m.getName());
+                // will be added to command line in super.execute()
+            }
+
+            tmpFile = FILE_UTILS.createTempFile("cvstagdiff", ".log", null,
+                                                true, true);
             setOutput(tmpFile);
 
             // run the cvs command
@@ -254,6 +275,7 @@ public class CvsTagDiff extends AbstractCvsTask {
             writeTagDiff(entries);
 
         } finally {
+            packageNames.clear();
             if (tmpFile != null) {
                 tmpFile.delete();
             }
@@ -289,20 +311,13 @@ public class CvsTagDiff extends AbstractCvsTask {
             // File testantoine/antoine.bat is removed; TESTANTOINE_1 revision 1.1.1.1
             //
             // get rid of 'File module/"
-            String toBeRemoved = FILE_STRING + mypackage + "/";
-            int headerLength = toBeRemoved.length();
             Vector entries = new Vector();
 
             String line = reader.readLine();
 
             while (null != line) {
-                if (line.length() > headerLength) {
-                    if (line.startsWith(toBeRemoved)) {
-                        line = line.substring(headerLength);
-                    } else {
-                        line = line.substring(FILE_STRING.length());
-                    }
-
+                line = removePackageName(line, packageNames);
+                if (line != null) {
                     // use || in a perl like fashion
                     boolean processed
                         =  doFileIsNew(entries, line)
@@ -417,7 +432,8 @@ public class CvsTagDiff extends AbstractCvsTask {
             }
 
             root.setAttribute("cvsroot", getCvsRoot());
-            root.setAttribute("package", mypackage);
+            root.setAttribute("package",
+                              CollectionUtils.flattenToString(packageNames));
             DOM_WRITER.openElement(root, writer, 0, "\t");
             writer.println();
             for (int i = 0, c = entries.length; i < c; i++) {
@@ -470,7 +486,7 @@ public class CvsTagDiff extends AbstractCvsTask {
      * @exception BuildException if a parameter is not correctly set
      */
     private void validate() throws BuildException {
-        if (null == mypackage) {
+        if (null == mypackage && getModules().size() == 0) {
             throw new BuildException("Package/module must be set.");
         }
 
@@ -496,4 +512,29 @@ public class CvsTagDiff extends AbstractCvsTask {
                                      + "be set.");
         }
     }
+
+    /**
+     * removes a "File: module/" prefix if present.
+     *
+     * @return null if the line was shorter than expected.
+     */
+    private static String removePackageName(String line, List packageNames) {
+        boolean matched = false;
+        for (Iterator iter = packageNames.iterator(); iter.hasNext(); ) {
+            String toBeRemoved = FILE_STRING + iter.next() + "/";
+            int len = toBeRemoved.length();
+            if (line.length() > len) {
+                if (line.startsWith(toBeRemoved)) {
+                    matched = true;
+                    line = line.substring(len);
+                    break;
+                }
+            }
+        }
+        if (!matched && line.length() > FILE_STRING.length()) {
+            line = line.substring(FILE_STRING.length());
+            matched = true;
+        }
+        return !matched ? null : line;
+    }
 }

src/tests/antunit/taskdefs/cvs/cvs.xml

@@ -48,4 +48,31 @@
                   destfile="${output}/report.xml"/>
     <au:assertFileExists file="${output}/report.xml"/>
   </target>
+
+  <target name="testCvsTagDiffWithSpaceInModule">
+    <mkdir dir="${output}"/>
+    <cvstagdiff cvsroot="${cvsroot}"
+                startDate="2008-01-01"
+                endDate="2009-01-01"
+                destfile="${output}/tagdiff.xml">
+      <module name="ant module 2"/>
+    </cvstagdiff>
+    <au:assertFileExists file="${output}/tagdiff.xml"/>
+    <au:assertResourceContains resource="${output}/tagdiff.xml"
+                               value="[test.txt]"/>
+  </target>
+
+  <target name="testCvsTagDiffWithMultipleModules">
+    <mkdir dir="${output}"/>
+    <cvstagdiff cvsroot="${cvsroot}"
+                startDate="2008-01-01"
+                endDate="2009-01-01"
+                destfile="${output}/tagdiff.xml"
+                package="antmodule1 antmodule3"/>
+    <au:assertFileExists file="${output}/tagdiff.xml"/>
+    <au:assertResourceContains resource="${output}/tagdiff.xml"
+                               value="[foo.txt]"/>
+    <au:assertResourceContains resource="${output}/tagdiff.xml"
+                               value="[yet another test.txt]"/>
+  </target>
 </project>

src/tests/antunit/taskdefs/cvs/repository/CVSROOT/history

@@ -12,3 +12,7 @@ O48f75161|stefan|/tmp/testoutput/*0|ant module 2||ant module 2
 O48f75185|stefan|/tmp/testoutput/*0|ant module 2||ant module 2
 O48f75186|stefan|/tmp/testoutput/*0|antmodule1||antmodule1
 O48f75196|stefan|/tmp/testoutput/*0|ant module 2||ant module 2
+O48f75279|stefan|/tmp/testoutput/*0|ant module 2||ant module 2
+O48f75282|stefan|/tmp/testoutput/*0|ant module 2||ant module 2
+O48f75d32|stefan|/tmp/testoutput/*0|ant module 2||ant module 2
+O48f75d34|stefan|/tmp/testoutput/*0|antmodule1||antmodule1

src/tests/antunit/taskdefs/cvs/repository/antmodule3/yet another test.txt,v

@@ -0,0 +1,41 @@
+head     1.1;
+branch   1.1.1;
+access   ;
+symbols  start:1.1.1.1 ant:1.1.1;
+locks    ; strict;
+comment  @# @;
+
+
+1.1
+date     2008.10.16.14.51.11;  author stefan;  state Exp;
+branches 1.1.1.1;
+next     ;
+commitid        7f8d48f754df4567;
+
+1.1.1.1
+date     2008.10.16.14.51.11;  author stefan;  state Exp;
+branches ;
+next     ;
+commitid        7f8d48f754df4567;
+
+
+desc
+@@
+
+
+
+1.1
+log
+@Initial revision
+@
+text
+@dark and cloudy.
+@
+
+
+1.1.1.1
+log
+@three times is a charm
+@
+text
+@@