improved the file mask validation handling.

When ArtifactArchiver reports file mask problem, also try to diagnose the problem.


git-svn-id: https://hudson.dev.java.net/svn/hudson/trunk/hudson/main@6990 71c3de6d-444a-0410-be80-ed276b4c234a

core/src/main/java/hudson/FilePath.java

@@ -44,6 +44,8 @@ import java.net.URI;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.StringTokenizer;
+import java.util.Arrays;
+import java.util.Comparator;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
@@ -912,46 +914,117 @@ public final class FilePath implements Serializable {
             public String invoke(File dir, VirtualChannel channel) throws IOException {
                 StringTokenizer tokens = new StringTokenizer(fileMasks,",");
 
-                OUTER:
                 while(tokens.hasMoreTokens()) {
                     final String fileMask = tokens.nextToken().trim();
-                    String previous = null;
+                    if(hasMatch(dir,fileMask))
-                    String pattern = fileMask;
+                        continue;   // no error on this portion
 
-                    while(true) {
+                    // in 1.172 we introduced an incompatible change to stop using ' ' as the separator
-                        FileSet fs = Util.createFileSet(dir,pattern);
+                    // so see if we can match by using ' ' as the separator
+                    if(fileMask.contains(" ")) {
+                        boolean matched = true;
+                        for (String token : Util.tokenize(fileMask))
+                            matched &= hasMatch(dir,token);
+                        if(matched)
+                            return Messages.FilePath_validateAntFileMask_whitespaceSeprator();
+                    }
 
-                        DirectoryScanner ds = fs.getDirectoryScanner(new org.apache.tools.ant.Project());
+                    // a common mistake is to assume the wrong base dir, and there are two variations
+                    // to this: (1) the user gave us aa/bb/cc/dd where cc/dd was correct
+                    // and (2) the user gave us cc/dd where aa/bb/cc/dd was correct.
 
-                        if(ds.getIncludedFilesCount()!=0 || ds.getIncludedDirsCount()!=0) {
+                    {// check the (1) above first
-                            // found a match
+                        String f=fileMask;
-                            if(pattern.equals(fileMask))
+                        while(true) {
-                                continue OUTER;    // no error
+                            int idx = findSeparator(f);
-                            if(previous==null)
+                            if(idx==-1)     break;
-                                return String.format("'%s' doesn't match anything, although '%s' exists",
+                            f=f.substring(idx+1);
-                                    fileMask, pattern );
+
-                            else
+                            if(hasMatch(dir,f))
-                                return String.format("'%s' doesn't match anything: '%s' exists but not '%s'",
+                                return Messages.FilePath_validateAntFileMask_doesntMatchAndSuggest(fileMask,f);
-                                    fileMask, pattern, previous );
                         }
+                    }
 
-                        int idx = Math.max(pattern.lastIndexOf('\\'),pattern.lastIndexOf('/'));
+                    {// check the (1) above next as this is more expensive.
-                        if(idx<0) {
+                        // Try prepending "**/" to see if that results in a match
-                            if(pattern.equals(fileMask))
+                        FileSet fs = Util.createFileSet(dir,"**/"+fileMask);
-                                return String.format("'%s' doesn't match anything", fileMask );
+                        DirectoryScanner ds = fs.getDirectoryScanner(new Project());
-                            else
+                        if(ds.getIncludedFilesCount()!=0) {
-                                return String.format("'%s' doesn't match anything: even '%s' doesn't exist",
+                            // try shorter name first so that the suggestion results in least amount of changes
-                                    fileMask, pattern );
+                            String[] names = ds.getIncludedFiles();
+                            Arrays.sort(names,SHORTER_STRING_FIRST);
+                            for( String f : names) {
+                                // now we want to decompose f to the leading portion that matched "**"
+                                // and the trailing portion that matched the file mask, so that
+                                // we can suggest the user error.
+                                //
+                                // this is not a very efficient/clever way to do it, but it's relatively simple
+
+                                String prefix="";
+                                while(true) {
+                                    int idx = findSeparator(f);
+                                    if(idx==-1)     break;
+
+                                    prefix+=f.substring(0,idx)+'/';
+                                    f=f.substring(idx+1);
+                                    if(hasMatch(dir,prefix+fileMask))
+                                        return Messages.FilePath_validateAntFileMask_doesntMatchAndSuggest(fileMask, prefix+fileMask);
+                                }
+                            }
                         }
+                    }
 
-                        // cut off the trailing component and try again
+                    {// finally, see if we can identify any sub portion that's valid. Otherwise bail out
-                        previous = pattern;
+                        String previous = null;
-                        pattern = pattern.substring(0,idx);
+                        String pattern = fileMask;
+
+                        while(true) {
+                            if(hasMatch(dir,pattern)) {
+                                // found a match
+                                if(previous==null)
+                                    return String.format("'%s' doesn't match anything, although '%s' exists",
+                                        fileMask, pattern );
+                                else
+                                    return String.format("'%s' doesn't match anything: '%s' exists but not '%s'",
+                                        fileMask, pattern, previous );
+                            }
+
+                            int idx = findSeparator(pattern);
+                            if(idx<0) {// no more path component left to go back
+                                if(pattern.equals(fileMask))
+                                    return String.format("'%s' doesn't match anything", fileMask );
+                                else
+                                    return String.format("'%s' doesn't match anything: even '%s' doesn't exist",
+                                        fileMask, pattern );
+                            }
+
+                            // cut off the trailing component and try again
+                            previous = pattern;
+                            pattern = pattern.substring(0,idx);
+                        }
                     }
                 }
 
                 return null; // no error
             }
+
+            private boolean hasMatch(File dir, String pattern) {
+                FileSet fs = Util.createFileSet(dir,pattern);
+                DirectoryScanner ds = fs.getDirectoryScanner(new Project());
+
+                return ds.getIncludedFilesCount()!=0 || ds.getIncludedDirsCount()!=0;
+            }
+
+            /**
+             * Finds the position of the first path separator.
+             */
+            private int findSeparator(String pattern) {
+                int idx1 = pattern.indexOf('\\');
+                int idx2 = pattern.indexOf('/');
+                if(idx1==-1)    return idx2;
+                if(idx2==-1)    return idx1;
+                return Math.min(idx1,idx2);
+            }
         });
     }
 
@@ -1017,4 +1090,10 @@ public final class FilePath implements Serializable {
 
         private static final long serialVersionUID = 1L;
     }
+
+    private static final Comparator<String> SHORTER_STRING_FIRST = new Comparator<String>() {
+        public int compare(String o1, String o2) {
+            return o1.length()-o2.length();
+        }
+    };
 }

core/src/main/java/hudson/tasks/ArtifactArchiver.java

@@ -68,8 +68,12 @@ public class ArtifactArchiver extends Publisher {
         dir.mkdirs();
 
         try {
-            if(p.getWorkspace().copyRecursiveTo(artifacts,excludes,new FilePath(dir))==0) {
+            FilePath ws = p.getWorkspace();
+            if(ws.copyRecursiveTo(artifacts,excludes,new FilePath(dir))==0) {
                 listener.error("No artifacts found that match the file pattern \""+artifacts+"\". Configuration error?");
+                String msg = ws.validateAntFileMask(artifacts);
+                if(msg!=null)
+                    listener.error(msg);
                 build.setResult(Result.FAILURE);
                 return true;
             }

core/src/main/resources/hudson/Messages.properties

@@ -1,3 +1,8 @@
+FilePath.validateAntFileMask.whitespaceSeprator=\
+  Whitespace can no longer be used as the separator. Please Use '','' as the separator instead.
+FilePath.validateAntFileMask.doesntMatchAndSuggest=\
+  ''{0}'' doesn''t match anything, but ''{1}'' does. Perhaps that''s what you mean?
+
 Util.second={0} {0,choice,0#seconds|1#second|1<seconds}
 Util.minute={0} {0,choice,0#minutes|1#minute|1<minutes}
 Util.hour  ={0} {0,choice,0#hours|1#hour|1<hours}