changelog for JENKINS-8880 and minor follow-up changes

jenkinsci · Aug 16, 2011 · 3a57b10 · 3a57b10
1 parent 4d21547
commit 3a57b10
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 8 deletions.
diff --git a/changelog.html b/changelog.html
@@ -84,6 +84,9 @@ <h3><a name=v1.427>What's new in 1.427</a> <!--=DATE=--></h3>
   <li class=bug>
     Disable auto refresh in slave markOffline screen
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-10608">issue 10608</a>)
+  <li class=bug>
+    Workspace-cleanup thread shouldn't delete custom workspace directories
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-8880">issue 8880</a>)
   <li class=rfe>
     Improved the speed of shutdown
   <li class=rfe>

diff --git a/core/src/main/java/hudson/model/WorkspaceCleanupThread.java b/core/src/main/java/hudson/model/WorkspaceCleanupThread.java
@@ -92,10 +92,10 @@ private void process(Jenkins h) throws IOException, InterruptedException {
         }
     }
 
-    private boolean shouldBeDeleted(String jobName, FilePath dir, Node n) throws IOException, InterruptedException {
+    private boolean shouldBeDeleted(String workspaceDirectoryName, FilePath dir, Node n) throws IOException, InterruptedException {
         // TODO: the use of remoting is not optimal.
         // One remoting can execute "exists", "lastModified", and "delete" all at once.
-        TopLevelItem item = Jenkins.getInstance().getItem(jobName);
+        TopLevelItem item = Jenkins.getInstance().getItem(workspaceDirectoryName);
 
         if(!dir.exists())
             return false;
@@ -107,15 +107,16 @@ private boolean shouldBeDeleted(String jobName, FilePath dir, Node n) throws IOE
             return false;
         }
 
-        // Assuming build name == workspace name breaks custom workspaces. Just skip out if the 30-day window doesn't catch it yet.
-        // TODO: Add a check that covers custom workspaces, if possible.
-        // TODO: If and when we do the above, also add checkbox that lets users configure a workspace to never be auto-cleaned.
+        // Could mean that directory doesn't belong to a job. But can also mean that it's a custom workspace belonging to a job.
+        // So better leave it alone - will still be deleted after 30 days - until we have a proper check for custom workspaces.
+        // TODO: implement proper check for custom workspaces.
+        // TODO: If we do the above, could also be good to add checkbox that lets users configure a workspace to never be auto-cleaned.
         if(item==null) {
             return false;
         }
 
-        if (item instanceof AbstractProject) {
-            AbstractProject p = (AbstractProject) item;
+        if (item instanceof AbstractProject<?,?>) {
+            AbstractProject<?,?> p = (AbstractProject<?,?>) item;
             Node lb = p.getLastBuiltOn();
             LOGGER.finer("Directory "+dir+" is last built on "+lb);
             if(lb!=null && lb.equals(n)) {
@@ -177,5 +178,5 @@ public boolean accept(File f) {
     /**
      * Can be used to disable workspace clean up.
      */
-    public static boolean disabled = Boolean.getBoolean(WorkspaceCleanupThread.class.getName()+".disabled");
+    public static final boolean disabled = Boolean.getBoolean(WorkspaceCleanupThread.class.getName()+".disabled");
 }