LogRotator is now pluggable.

This is done by introducing a new base type 'BuildDiscarder' that's
extensible. Plugins can implement their own logics.

changelog.html

@@ -71,6 +71,8 @@ Upcoming changes</a>
   <li class=rfe>
     JNLP slave installers can now work transparently with secured Jenkins.
     (SECURITY-54 / despite the ticket marker, this is not a security vulnerability)
+  <li class=rfe>
+    "Discard old build records" behavior is now pluggable, allowing plugins to define custom logic.
 </ul>
 </div><!--=TRUNK-END=-->
 

core/src/main/java/hudson/matrix/MatrixConfiguration.java

@@ -35,6 +35,7 @@ import hudson.model.Cause;
 import hudson.model.CauseAction;
 import hudson.model.DependencyGraph;
 import hudson.model.Descriptor;
+import jenkins.model.BuildDiscarder;
 import jenkins.model.Jenkins;
 import hudson.model.Item;
 import hudson.model.ItemGroup;
@@ -297,10 +298,14 @@ public class MatrixConfiguration extends Project<MatrixConfiguration,MatrixRun>
     }
 
     @Override
-    public LogRotator getLogRotator() {
-        LogRotator lr = getParent().getLogRotator();
-        return new LinkedLogRotator(lr != null ? lr.getArtifactDaysToKeep() : -1,
-                                    lr != null ? lr.getArtifactNumToKeep() : -1);
+    public BuildDiscarder getBuildDiscarder() {
+        // TODO: LinkedLogRotator doesn't work very well in the face of pluggable BuildDiscarder but I don't know what to do
+        BuildDiscarder bd = getParent().getBuildDiscarder();
+        if (bd instanceof LogRotator) {
+            LogRotator lr = (LogRotator) bd;
+            return new LinkedLogRotator(lr.getArtifactDaysToKeep(),lr.getArtifactNumToKeep());
+        }
+        return new LinkedLogRotator();
     }
 
     @Override
@@ -328,7 +333,7 @@ public class MatrixConfiguration extends Project<MatrixConfiguration,MatrixRun>
      *      Value is controlled by {@link MatrixProject}.
      */
     @Override
-    public void setLogRotator(LogRotator logRotator) {
+    public void setBuildDiscarder(BuildDiscarder logRotator) {
         throw new UnsupportedOperationException();
     }
 

core/src/main/java/hudson/model/Job.java

@@ -26,6 +26,7 @@ package hudson.model;
 import com.google.common.base.Function;
 import com.google.common.collect.Collections2;
 import com.infradna.tool.bridge_method_injector.WithBridgeMethods;
+import hudson.BulkChange;
 import hudson.EnvVars;
 import hudson.Extension;
 import hudson.ExtensionPoint;
@@ -60,8 +61,10 @@ import hudson.util.TextFile;
 import hudson.widgets.HistoryWidget;
 import hudson.widgets.HistoryWidget.Adapter;
 import hudson.widgets.Widget;
+import jenkins.model.BuildDiscarder;
 import jenkins.model.Jenkins;
 import jenkins.model.ProjectNamingStrategy;
+import jenkins.scm.SCMCheckoutStrategy;
 import jenkins.security.HexStringConfidentialKey;
 import jenkins.util.io.OnMaster;
 import net.sf.json.JSONException;
@@ -124,7 +127,7 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
      */
     private transient volatile boolean holdOffBuildUntilSave;
 
-    private volatile LogRotator logRotator;
+    private volatile BuildDiscarder logRotator;
 
     /**
      * Not all plugins are good at calculating their health report quickly.
@@ -362,23 +365,45 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
     }
 
     /**
-     * Returns the log rotator for this job, or null if none.
+     * Returns the configured build discarder for this job, or null if none.
      */
-    public LogRotator getLogRotator() {
+    public BuildDiscarder getBuildDiscarder() {
         return logRotator;
     }
 
-    public void setLogRotator(LogRotator logRotator) {
-        this.logRotator = logRotator;
+    public void setBuildDiscarder(BuildDiscarder bd) throws IOException {
+        this.logRotator = bd;
+        save();
+    }
+
+    /**
+     * Left for backward compatibility. Returns non-null if and only
+     * if {@link LogRotator} is configured as {@link BuildDiscarder}.
+     *
+     * @deprecated as of 1.503
+     *      Use {@link #getBuildDiscarder()}.
+     */
+    public LogRotator getLogRotator() {
+        if (logRotator instanceof LogRotator)
+            return (LogRotator) logRotator;
+        return null;
+    }
+
+    /**
+     * @deprecated as of 1.503
+     *      Use {@link #setBuildDiscarder(BuildDiscarder)}
+     */
+    public void setLogRotator(LogRotator logRotator) throws IOException {
+        setBuildDiscarder(logRotator);
     }
 
     /**
      * Perform log rotation.
      */
     public void logRotate() throws IOException, InterruptedException {
-        LogRotator lr = getLogRotator();
-        if (lr != null)
-            lr.perform(this);
+        BuildDiscarder bd = getBuildDiscarder();
+        if (bd != null)
+            bd.perform(this);
     }
 
     /**
@@ -1029,8 +1054,8 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
 
             setDisplayName(json.optString("displayNameOrNull"));
 
-            if (req.getParameter("logrotate") != null)
-                logRotator = LogRotator.DESCRIPTOR.newInstance(req,json.getJSONObject("logrotate"));
+            if (json.optBoolean("logrotate"))
+                logRotator = req.bindJSON(BuildDiscarder.class, json.optJSONObject("buildDiscarder"));
             else
                 logRotator = null;
 

core/src/main/java/hudson/model/Run.java

@@ -51,7 +51,6 @@ import hudson.security.ACL;
 import hudson.security.AccessControlled;
 import hudson.security.Permission;
 import hudson.security.PermissionGroup;
-import hudson.tasks.LogRotator;
 import hudson.tasks.BuildWrapper;
 import hudson.tasks.BuildStep;
 import hudson.tasks.test.AbstractTestResultAction;
@@ -97,6 +96,7 @@ import java.util.zip.GZIPInputStream;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletResponse;
 
+import jenkins.model.BuildDiscarder;
 import jenkins.model.Jenkins;
 import jenkins.model.JenkinsLocationConfiguration;
 import jenkins.util.io.OnMaster;
@@ -110,7 +110,6 @@ 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;
 
@@ -181,7 +180,7 @@ public abstract class Run <JobT extends Job<JobT,RunT>,RunT extends Run<JobT,Run
      *
      * For historical reasons, 0 means no value is recorded.
      *
-     * @see #getStartTime()
+     * @see #getStartTimeInMillis()
      */
     private long startTime;
 
@@ -513,7 +512,7 @@ public abstract class Run <JobT extends Job<JobT,RunT>,RunT extends Run<JobT,Run
     /**
      * Returns true if this log file should be kept and not deleted.
      *
-     * This is used as a signal to the {@link LogRotator}.
+     * This is used as a signal to the {@link BuildDiscarder}.
      */
     @Exported
     public final boolean isKeepLog() {

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

@@ -24,10 +24,11 @@
 package hudson.tasks;
 
 import com.google.common.collect.Lists;
-import hudson.model.Describable;
-import hudson.model.Descriptor;
+import hudson.Extension;
 import hudson.model.Job;
 import hudson.model.Run;
+import jenkins.model.BuildDiscarder;
+import jenkins.model.BuildDiscarderDescriptor;
 import org.kohsuke.stapler.DataBoundConstructor;
 
 import java.io.IOException;
@@ -40,15 +41,15 @@ import java.util.logging.Logger;
 import static java.util.logging.Level.*;
 
 /**
- * Deletes old builds.
- * 
- * Historically this is called LogRotator, though it deletes the whole build including all artifacts.
+ * Default implementation of {@link BuildDiscarder}.
+ *
+ * For historical reason, this is called LogRotator, but it does not rotate logs :-)
  * 
  * Since 1.350 it has also the option to keep the build, but delete its recorded artifacts.
  * 
  * @author Kohsuke Kawaguchi
  */
-public class LogRotator implements Describable<LogRotator> {
+public class LogRotator extends BuildDiscarder {
 
     /**
      * If not -1, history is only kept up to this days.
@@ -75,9 +76,9 @@ public class LogRotator implements Describable<LogRotator> {
     private final Integer artifactNumToKeep;
 
     @DataBoundConstructor
-    public LogRotator (String logrotate_days, String logrotate_nums, String logrotate_artifact_days, String logrotate_artifact_nums) {
-        this (parse(logrotate_days),parse(logrotate_nums),
-              parse(logrotate_artifact_days),parse(logrotate_artifact_nums));
+    public LogRotator (String daysToKeepStr, String numToKeepStr, String artifactDaysToKeepStr, String artifactNumToKeepStr) {
+        this (parse(daysToKeepStr),parse(numToKeepStr),
+              parse(artifactDaysToKeepStr),parse(artifactNumToKeepStr));
     }
 
     public static int parse(String p) {
@@ -252,14 +253,8 @@ public class LogRotator implements Describable<LogRotator> {
         return String.valueOf(i);
     }
 
-
-    public LRDescriptor getDescriptor() {
-        return DESCRIPTOR;
-    }
-
-    public static final LRDescriptor DESCRIPTOR = new LRDescriptor();
-
-    public static final class LRDescriptor extends Descriptor<LogRotator> {
+    @Extension
+    public static final class LRDescriptor extends BuildDiscarderDescriptor {
         public String getDisplayName() {
             return "Log Rotation";
         }

core/src/main/java/jenkins/model/BuildDiscarder.java

+package jenkins.model;
+
+import hudson.ExtensionPoint;
+import hudson.model.AbstractDescribableImpl;
+import hudson.model.Job;
+import hudson.model.Run;
+
+import java.io.IOException;
+
+/**
+ * Implementation of "Discard old build records" feature.
+ *
+ * <p>
+ * This extension point allows plugins to implement a different strategy to decide what builds to discard
+ * and what builds to keep.
+ *
+ * @author Kohsuke Kawaguchi
+ * @since 1.503
+ */
+public abstract class BuildDiscarder extends AbstractDescribableImpl<BuildDiscarder> implements ExtensionPoint {
+    /**
+     * Called to perform "garbage collection" on the job to discard old build records.
+     *
+     * <p>
+     * Normally invoked automatically jobs when new builds occur.
+     * The general expectation is that those marked as {@link Run#isKeepLog()} will be kept untouched.
+     * To delete the build record, call {@link Run#delete()}.
+     *
+     * @see Job#logRotate()
+     */
+    public abstract void perform(Job<?,?> job) throws IOException, InterruptedException;
+
+    @Override
+    public BuildDiscarderDescriptor getDescriptor() {
+        return (BuildDiscarderDescriptor)super.getDescriptor();
+    }
+}

core/src/main/java/jenkins/model/BuildDiscarderDescriptor.java

+package jenkins.model;
+
+import hudson.DescriptorExtensionList;
+import hudson.model.Descriptor;
+
+/**
+ * {@link Descriptor} for {@link BuildDiscarder}.
+ *
+ * @author Kohsuke Kawaguchi
+ */
+public abstract class BuildDiscarderDescriptor extends Descriptor<BuildDiscarder> {
+    protected BuildDiscarderDescriptor(Class clazz) {
+        super(clazz);
+    }
+
+    protected BuildDiscarderDescriptor() {
+    }
+
+    /**
+     * Returns all the registered {@link BuildDiscarderDescriptor}s.
+     */
+    public static DescriptorExtensionList<BuildDiscarder,BuildDiscarderDescriptor> all() {
+        return Jenkins.getInstance().<BuildDiscarder,BuildDiscarderDescriptor>getDescriptorList(BuildDiscarder.class);
+    }
+}

core/src/main/resources/hudson/model/Job/configure.jelly

@@ -49,8 +49,8 @@ THE SOFTWARE.
           <!-- log rotator -->
           <f:optionalBlock name="logrotate"
             help="/help/project-config/log-rotation.html"
-            title="${%Discard Old Builds}" checked="${it.logRotator!=null}">
-            <st:include page="/hudson/tasks/LogRotator/config.jelly"/>
+            title="${%Discard Old Builds}" checked="${it.buildDiscarder!=null}" inline="true">
+            <f:dropdownDescriptorSelector field="buildDiscarder" title="${%Strategy}"/>
           </f:optionalBlock>
         </j:if>
 

core/src/main/resources/hudson/tasks/LogRotator/config.jelly

@@ -25,25 +25,21 @@ THE SOFTWARE.
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
   <f:entry title="${%Days to keep builds}"
-    description="${%if not empty, build records are only kept up to this number of days}">
-    <f:number clazz="positive-number" min="1" step="1"
-      name="logrotate_days" value="${it.logRotator.daysToKeepStr}"/>
+    description="${%if not empty, build records are only kept up to this number of days}" field="daysToKeepStr">
+    <f:number clazz="positive-number" min="1" step="1" /> ${instance}/${instance.daysToKeepStr}/x
   </f:entry>
   <f:entry title="${%Max # of builds to keep}"
-    description="${%if not empty, only up to this number of build records are kept}">
-    <f:number clazz="positive-number" min="1" step="1"
-      name="logrotate_nums" value="${it.logRotator.numToKeepStr}"/>
+    description="${%if not empty, only up to this number of build records are kept}" field="numToKeepStr">
+    <f:number clazz="positive-number" min="1" step="1" />
   </f:entry>
   <f:advanced>
     <f:entry title="${%Days to keep artifacts}"
-             description="${%if not empty, artifacts from builds older than this number of days will be deleted, but the logs, history, reports, etc for the build will be kept}">
-      <f:number clazz="positive-number" min="1" step="1"
-                 name="logrotate_artifact_days" value="${it.logRotator.artifactDaysToKeepStr}" />
+             description="${%if not empty, artifacts from builds older than this number of days will be deleted, but the logs, history, reports, etc for the build will be kept}" field="artifactDaysToKeepStr">
+      <f:number clazz="positive-number" min="1" step="1" />
     </f:entry>
     <f:entry title="${%Max # of builds to keep with artifacts}"
-             description="${%if not empty, only up to this number of builds have their artifacts retained}">
-      <f:number clazz="positive-number" min="1" step="1"
-                 name="logrotate_artifact_nums" value="${it.logRotator.artifactNumToKeepStr}" />
+             description="${%if not empty, only up to this number of builds have their artifacts retained}" field="artifactNumToKeepStr">
+      <f:number clazz="positive-number" min="1" step="1" />
     </f:entry>
   </f:advanced>
 </j:jelly>