Skip to content
Permalink
Browse files

[FIXED JENKINS-7214] FilePath.validateAntFileMask too slow for /confi…

…gure.
  • Loading branch information...
jglick committed Sep 19, 2012
1 parent 347bca5 commit a885cc2179ea76bced186c3ed223ce50fec58a75
@@ -58,6 +58,9 @@
<li class=bug>
Log recorders do not work reliably.
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-15226">issue 15226</a>)
<li class=bug>
<code>FilePath.validateAntFileMask</code> too slow for <code>/configure</code>.
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-7214">issue 7214</a>)
<li class=>
</ul>
</div><!--=TRUNK-END=-->
@@ -27,7 +27,6 @@

import hudson.Launcher.LocalLauncher;
import hudson.Launcher.RemoteLauncher;
import hudson.model.AbstractDescribableImpl;
import jenkins.model.Jenkins;
import hudson.model.TaskListener;
import hudson.model.AbstractProject;
@@ -840,7 +839,7 @@ public void copyFrom(FileItem file) throws IOException, InterruptedException {

return channel.call(wrapper);
} catch (TunneledInterruptedException e) {
throw (InterruptedException)new InterruptedException().initCause(e);
throw (InterruptedException)new InterruptedException(e.getMessage()).initCause(e);
} catch (AbortException e) {
throw e; // pass through so that the caller can catch it as AbortException
} catch (IOException e) {
@@ -1976,8 +1975,26 @@ public Boolean call() throws IOException {
* @see #validateFileMask(FilePath, String)
*/
public String validateAntFileMask(final String fileMasks) throws IOException, InterruptedException {
return validateAntFileMask(fileMasks, Integer.MAX_VALUE);
}

/**
* Like {@link #validateAntFileMask(String)} but performing only a bounded number of operations.
* <p>Whereas the unbounded overload is appropriate for calling from cancelable, long-running tasks such as build steps,
* this overload should be used when an answer is needed quickly, such as for {@link #validateFileMask(String)}
* or anything else returning {@link FormValidation}.
* <p>If a positive match is found, {@code null} is returned immediately.
* A message is returned in case the file pattern can definitely be determined to not match anything in the directory within the alloted time.
* If the time runs out without finding a match but without ruling out the possibility that there might be one, {@link InterruptedException} is thrown,
* in which case the calling code should give the user the benefit of the doubt and use {@link hudson.util.FormValidation.Kind#OK} (with or without a message).
* @param bound a maximum number of negative operations (deliberately left vague) to perform before giving up on a precise answer; 10_000 is a reasonable pick
* @throws InterruptedException not only in case of a channel failure, but also if too many operations were performed without finding any matches
* @since 1.484
*/
public String validateAntFileMask(final String fileMasks, final int bound) throws IOException, InterruptedException {
return act(new FileCallable<String>() {
public String invoke(File dir, VirtualChannel channel) throws IOException {
private static final long serialVersionUID = 1;
public String invoke(File dir, VirtualChannel channel) throws IOException, InterruptedException {
if(fileMasks.startsWith("~"))
return Messages.FilePath_TildaDoesntWork();

@@ -2074,10 +2091,28 @@ public String invoke(File dir, VirtualChannel channel) throws IOException {
return null; // no error
}

private boolean hasMatch(File dir, String pattern) {
FileSet fs = Util.createFileSet(dir,pattern);
DirectoryScanner ds = fs.getDirectoryScanner(new Project());

private boolean hasMatch(File dir, String pattern) throws InterruptedException {
class Cancel extends RuntimeException {}
DirectoryScanner ds = bound == Integer.MAX_VALUE ? new DirectoryScanner() : new DirectoryScanner() {
int ticks;
@Override public synchronized boolean isCaseSensitive() {
if (!filesIncluded.isEmpty() || !dirsIncluded.isEmpty() || ticks++ > bound) {
throw new Cancel();
}
return super.isCaseSensitive();
}
};
ds.setBasedir(dir);
ds.setIncludes(new String[] {pattern});
try {
ds.scan();
} catch (Cancel c) {
if (ds.getIncludedFilesCount()!=0 || ds.getIncludedDirsCount()!=0) {
return true;
} else {
throw new InterruptedException("no matches found within " + bound);
}
}
return ds.getIncludedFilesCount()!=0 || ds.getIncludedDirsCount()!=0;
}

@@ -2126,11 +2161,11 @@ public FormValidation validateFileMask(String value, boolean errorIfNotExist) th
if(!exists()) // no workspace. can't check
return FormValidation.ok();

String msg = validateAntFileMask(value);
String msg = validateAntFileMask(value, 10000);
if(errorIfNotExist) return FormValidation.error(msg);
else return FormValidation.warning(msg);
} catch (InterruptedException e) {
return FormValidation.ok();
return FormValidation.ok(Messages.FilePath_did_not_manage_to_validate_may_be_too_sl(value));
}
}

@@ -378,11 +378,11 @@ protected void check() throws IOException, ServletException {
return;
}

String msg = ws.validateAntFileMask(value);
String msg = ws.validateAntFileMask(value, 10000);
if(errorIfNotExist) error(msg);
else warning(msg);
} catch (InterruptedException e) {
ok(); // coundn't check
ok(Messages.FormFieldValidator_did_not_manage_to_validate_may_be_too_sl(value));
}
}

@@ -20,6 +20,7 @@
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
# THE SOFTWARE.

FilePath.did_not_manage_to_validate_may_be_too_sl=Did not manage to validate {0} (may be too slow)
FilePath.validateAntFileMask.whitespaceSeprator=\
Whitespace can no longer be used as the separator. Please Use '','' as the separator instead.
FilePath.validateAntFileMask.doesntMatchAndSuggest=\
@@ -24,6 +24,7 @@ ClockDifference.InSync=In sync
ClockDifference.Ahead=\ ahead
ClockDifference.Behind=\ behind
ClockDifference.Failed=Failed to check
FormFieldValidator.did_not_manage_to_validate_may_be_too_sl=Did not manage to validate {0} (may be too slow)
FormValidation.ValidateRequired=Required
FormValidation.Error.Details=(show details)
HttpResponses.Saved=Saved
HttpResponses.Saved=Saved
@@ -449,4 +449,30 @@ private static void assertValidateAntFileMask(String expected, FilePath d, Strin
assertEquals(expected, d.validateAntFileMask(fileMasks));
}

@Bug(7214)
public void testValidateAntFileMaskBounded() throws Exception {
File tmp = Util.createTempDir();
try {
FilePath d = new FilePath(french, tmp.getPath());
FilePath d2 = d.child("d1/d2");
d2.mkdirs();
for (int i = 0; i < 100; i++) {
FilePath d3 = d2.child("d" + i);
d3.mkdirs();
d3.child("f.txt").touch(0);
}
assertEquals(null, d.validateAntFileMask("d1/d2/**/f.txt"));
assertEquals(null, d.validateAntFileMask("d1/d2/**/f.txt", 10));
assertEquals(Messages.FilePath_validateAntFileMask_portionMatchButPreviousNotMatchAndSuggest("**/*.js", "**", "**/*.js"), d.validateAntFileMask("**/*.js", 1000));
try {
d.validateAntFileMask("**/*.js", 10);
fail();
} catch (InterruptedException x) {
// good
}
} finally {
Util.deleteRecursive(tmp);
}
}

}

0 comments on commit a885cc2

Please sign in to comment.
You can’t perform that action at this time.