[FIXED JENKINS-16457] Bogus Jenkins.rawBuildsDir not adequately diagn…

…osed.
jenkinsci · Jan 23, 2013 · 4ef6a64 · 4ef6a64
1 parent 77b49f5
commit 4ef6a64
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 27 deletions.
diff --git a/changelog.html b/changelog.html
@@ -55,7 +55,9 @@
 <!-- Record your changes in the trunk here. -->
 <div id="trunk" style="display:none"><!--=TRUNK-BEGIN=-->
 <ul class=image>
-  <li class=>
+  <li class=bug>
+    Bogus “Build Record Root Directory” inadequately diagnosed.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-16457">issue 16457</a>)
 </ul>
 </div><!--=TRUNK-END=-->
 

diff --git a/core/src/main/java/hudson/model/Run.java b/core/src/main/java/hudson/model/Run.java
@@ -104,20 +104,22 @@
 import org.apache.commons.io.input.NullInputStream;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.jelly.XMLOutput;
-import org.apache.tools.ant.taskdefs.email.Mailer;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.stapler.*;
 import org.kohsuke.stapler.export.Exported;
 import org.kohsuke.stapler.export.ExportedBean;
 
 import com.thoughtworks.xstream.XStream;
+import hudson.model.Run.RunExecution;
+import java.io.ByteArrayInputStream;
 import org.kohsuke.stapler.interceptor.RequirePOST;
 
 import java.io.FileOutputStream;
 import java.io.OutputStream;
 
 import static java.util.logging.Level.*;
+import javax.annotation.Nonnull;
 
 /**
  * A particular execution of {@link Job}.
@@ -1224,7 +1226,8 @@ public InputStream getLogInputStream() throws IOException {
     	    }
     	}
 
-    	return new NullInputStream(0);
+        String message = "No such file: " + logFile;
+    	return new ByteArrayInputStream(charset != null ? message.getBytes(charset) : message.getBytes());
     }
 
     public Reader getLogReader() throws IOException {
@@ -1465,7 +1468,7 @@ private synchronized void allDone() {
          * Among other things, this is often a necessary pre-condition
          * before invoking other builds that depend on this build.
          */
-        public abstract void cleanUp(BuildListener listener) throws Exception;
+        public abstract void cleanUp(@Nonnull BuildListener listener) throws Exception;
 
         public RunT getBuild() {
             return _this();
@@ -1597,19 +1600,17 @@ protected final void execute(RunExecution job) {
                 // see issue #980.
                 state = State.POST_PRODUCTION;
 
-                try {
-                    job.cleanUp(listener);
-                } catch (Exception e) {
-                    handleFatalBuildProblem(listener,e);
-                    // too late to update the result now
-                }
-
-                RunListener.fireCompleted(this,listener);
-
-                if(listener!=null)
+                if (listener != null) {
+                    try {
+                        job.cleanUp(listener);
+                    } catch (Exception e) {
+                        handleFatalBuildProblem(listener,e);
+                        // too late to update the result now
+                    }
+                    RunListener.fireCompleted(this,listener);
                     listener.finished(result);
-                if(listener!=null)
                     listener.closeQuietly();
+                }
 
                 try {
                     save();

diff --git a/core/src/main/java/hudson/model/listeners/RunListener.java b/core/src/main/java/hudson/model/listeners/RunListener.java
@@ -47,6 +47,7 @@
 import java.lang.reflect.Type;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import javax.annotation.Nonnull;
 
 /**
  * Receives notifications about builds.
@@ -86,7 +87,7 @@ protected RunListener() {
      *      which becomes a part of the "console output" of this build. But when this method runs,
      *      the build is considered completed, so its status cannot be changed anymore.
      */
-    public void onCompleted(R r, TaskListener listener) {}
+    public void onCompleted(R r, @Nonnull TaskListener listener) {}
 
     /**
      * Called after a build is moved to the {@link Run.State#COMPLETED} state.
@@ -174,7 +175,7 @@ public void unregister() {
     /**
      * Fires the {@link #onCompleted(Run, TaskListener)} event.
      */
-    public static void fireCompleted(Run r, TaskListener listener) {
+    public static void fireCompleted(Run r, @Nonnull TaskListener listener) {
         for (RunListener l : all()) {
             if(l.targetType.isInstance(r))
                 try {

diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java
@@ -94,7 +94,6 @@
 import hudson.DescriptorExtensionList;
 import hudson.Extension;
 import hudson.ExtensionList;
-import hudson.ExtensionPoint;
 import hudson.FilePath;
 import hudson.Functions;
 import hudson.Launcher;
@@ -111,7 +110,6 @@
 import hudson.Util;
 import static hudson.Util.fixEmpty;
 import static hudson.Util.fixNull;
-import hudson.WebAppMain;
 import hudson.XmlFile;
 import hudson.cli.CLICommand;
 import hudson.cli.CliEntryPoint;
@@ -135,7 +133,6 @@
 import hudson.security.ACL;
 import hudson.security.AccessControlled;
 import hudson.security.AuthorizationStrategy;
-import hudson.security.BasicAuthenticationFilter;
 import hudson.security.FederatedLoginService;
 import hudson.security.FullControlOnceLoggedInAuthorizationStrategy;
 import hudson.security.HudsonFilter;
@@ -263,7 +260,6 @@
 import java.nio.charset.Charset;
 import java.security.SecureRandom;
 import java.text.Collator;
-import java.text.ParseException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -381,17 +377,15 @@ public class Jenkins extends AbstractCIBase implements ModifiableTopLevelItemGro
     private ProjectNamingStrategy projectNamingStrategy = DefaultProjectNamingStrategy.DEFAULT_NAMING_STRATEGY;
 
     /**
-     * Root directory for the workspaces. This value will be variable-expanded against
-     * job name and JENKINS_HOME.
-     *
+     * Root directory for the workspaces.
+     * This value will be variable-expanded as per {@link #expandVariablesForDirectory}.
      * @see #getWorkspaceFor(TopLevelItem)
      */
     private String workspaceDir = "${ITEM_ROOTDIR}/"+WORKSPACE_DIRNAME;
 
     /**
-     * Root directory for the workspaces. This value will be variable-expanded against
-     * job name and JENKINS_HOME.
-     *
+     * Root directory for the builds.
+     * This value will be variable-expanded as per {@link #expandVariablesForDirectory}.
      * @see #getBuildDirFor(Job)
      */
     private String buildsDir = "${ITEM_ROOTDIR}/builds";
@@ -1772,6 +1766,17 @@ public FormValidation doCheckNumExecutors(@QueryParameter String value) {
             return FormValidation.validateNonNegativeInteger(value);
         }
 
+        public FormValidation doCheckRawBuildsDir(@QueryParameter String value) {
+            if (!value.contains("${")) {
+                File d = new File(value);
+                if (!d.isDirectory() && (d.getParentFile() == null || !d.getParentFile().canWrite())) {
+                    return FormValidation.error(value + " does not exist and probably cannot be created");
+                }
+                // XXX failure to use either ITEM_* variable might be an error too?
+            }
+            return FormValidation.ok(); // XXX assumes it will be OK after substitution, but can we be sure?
+        }
+
         // to route /descriptor/FQCN/xxx to getDescriptor(FQCN).xxx
         public Object getDynamic(String token) {
             return Jenkins.getInstance().getDescriptor(token);