extra decision recording infrastructure for future debugging! Also fix for pr133532

4 files changed, 84 insertions, 10 deletions

diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AbstractStateListener.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AbstractStateListener.java
index b92f806e7..f854cd9c8 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AbstractStateListener.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AbstractStateListener.java
@@ -31,5 +31,9 @@ public abstract class AbstractStateListener implements IStateListener {
 	public void detectedAspectDeleted(File f) {	}
 
 	public void buildSuccessful(boolean wasFullBuild) {	}
+	
+	public void recordDecision(String decision) {}
+	
+	public void recordInformation(String info) {}
 
 }
diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjBuildManager.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjBuildManager.java
index b08298b0a..7e6678278 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjBuildManager.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjBuildManager.java
@@ -192,7 +192,8 @@ public class AjBuildManager implements IOutputClassFileNameProvider,IBinarySourc
         	
             boolean canIncremental = state.prepareForNextBuild(buildConfig);
             if (!canIncremental && !batch) { // retry as batch?
-               	CompilationAndWeavingContext.leavingPhase(ct);
+             CompilationAndWeavingContext.leavingPhase(ct);
+             if (state.listenerDefined()) state.getListener().recordDecision("Falling back to batch compilation");
             	return doBuild(buildConfig, baseHandler, true);
             }
             this.handler = 
@@ -225,10 +226,10 @@ public class AjBuildManager implements IOutputClassFileNameProvider,IBinarySourc
             }
             
             if (buildConfig.getOutputJar() != null) {
-            	if (!openOutputStream(buildConfig.getOutputJar())) {
-                   	CompilationAndWeavingContext.leavingPhase(ct);
-                   	return false;
-            	}
+            	 if (!openOutputStream(buildConfig.getOutputJar())) {
+                CompilationAndWeavingContext.leavingPhase(ct);
+                return false;
+            	 }
             }
             
             if (batch) {
@@ -261,15 +262,19 @@ public class AjBuildManager implements IOutputClassFileNameProvider,IBinarySourc
 				    AsmManager.getDefault().processDelta(files,state.getAddedFiles(),state.getDeletedFiles());
                 boolean hereWeGoAgain = !(files.isEmpty() && binarySourcesForTheNextCompile.isEmpty());
                 for (int i = 0; (i < 5) && hereWeGoAgain; i++) {
+                	   if (state.listenerDefined()) 
+                		   state.getListener().recordInformation("Starting incremental compilation loop "+(i+1)+" of possibly 5");
                     // System.err.println("XXXX inc: " + files);
                
                     performCompilation(files);
                     if (handler.hasErrors() || (progressListener!=null && progressListener.isCancelledRequested())) {
-                       	CompilationAndWeavingContext.leavingPhase(ct);
+                        CompilationAndWeavingContext.leavingPhase(ct);
                         return false;
                     } 
                     
                     if (state.requiresFullBatchBuild()) {
+                    	if (state.listenerDefined()) 
+                 		   state.getListener().recordInformation(" Dropping back to full build");
                     	return batchBuild(buildConfig, baseHandler);
                     }
                     
@@ -343,6 +348,18 @@ public class AjBuildManager implements IOutputClassFileNameProvider,IBinarySourc
         return ret;
     }
     
+    private String stringifyList(List l) {
+      if (l==null) return "";
+    	  StringBuffer sb = new StringBuffer();
+    	  sb.append("{");
+    	  for (Iterator iter = l.iterator(); iter.hasNext();) {
+			Object el = (Object) iter.next();
+			sb.append(el);
+			if (iter.hasNext()) sb.append(",");
+		}
+    	  sb.append("}");
+    	  return sb.toString();
+    }
 
 	private boolean openOutputStream(File outJar)  {
 		try {
diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjState.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjState.java
index 395697840..c450348ab 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjState.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjState.java
@@ -196,22 +196,26 @@ public class AjState {
 		currentBuildTime = System.currentTimeMillis();
 
 		if (!maybeIncremental()) {
+			if (listenerDefined()) getListener().recordDecision("Preparing for build: not going to be incremental because either not in AJDT or incremental deactivated");
 			return false;
 		}
 
 		if (this.batchBuildRequiredThisTime) {
 			this.batchBuildRequiredThisTime = false;
+			if (listenerDefined()) getListener().recordDecision("Preparing for build: not going to be incremental this time because batch build explicitly forced");
 			return false;
 		}
 		
 		if (lastSuccessfulBuildTime == -1 || buildConfig == null) {
 			structuralChangesSinceLastFullBuild.clear();
+			if (listenerDefined()) getListener().recordDecision("Preparing for build: not going to be incremental because no successful previous full build");
 			return false;
 		}
 		
 		// we don't support incremental with an outjar yet
 		if (newBuildConfig.getOutputJar() != null) {
 			structuralChangesSinceLastFullBuild.clear();
+			if (listenerDefined()) getListener().recordDecision("Preparing for build: not going to be incremental because outjar being used");
 			return false;
 		}
 		
@@ -226,6 +230,7 @@ public class AjState {
 		    removeAllResultsOfLastBuild();
 			if (stateListener!=null) stateListener.pathChangeDetected();
 			structuralChangesSinceLastFullBuild.clear();
+			if (listenerDefined()) getListener().recordDecision("Preparing for build: not going to be incremental because path change detected (one of classpath/aspectpath/inpath/injars)");
 		    return false;
 		}
 		
@@ -249,9 +254,13 @@ public class AjState {
 		deletedBinaryFiles.removeAll(newBinaryFiles);
 			
 		boolean couldStillBeIncremental = processDeletedFiles(deletedFiles);
+
+		if (!couldStillBeIncremental) {
+			if (listenerDefined()) getListener().recordDecision("Preparing for build: not going to be incremental because an aspect was deleted");
+			return false;
+		}
 		
-		if (!couldStillBeIncremental) return false;
-		
+		if (listenerDefined()) getListener().recordDecision("Preparing for build: planning to be an incremental build");
 		return true;
 	}
 		
@@ -997,11 +1006,22 @@ public class AjState {
 		return ret;
 	}
 		
-	
+
+    private String stringifyList(List l) {
+    	  StringBuffer sb = new StringBuffer();
+    	  sb.append("{");
+    	  for (Iterator iter = l.iterator(); iter.hasNext();) {
+			Object el = (Object) iter.next();
+			sb.append(el);
+			if (iter.hasNext()) sb.append(",");
+		}
+    	  sb.append("}");
+    	  return sb.toString();
+    }
 	
 	protected void addAffectedSourceFiles(List addTo, List lastTimeSources) {
 		if (qualifiedStrings.isEmpty() && simpleStrings.isEmpty()) return;
-
+		if (listenerDefined()) getListener().recordDecision("Examining whether any other files now need compilation based just compiling: '"+stringifyList(lastTimeSources)+"'");
 		// the qualifiedStrings are of the form 'p1/p2' & the simpleStrings are just 'X'
 		char[][][] qualifiedNames = ReferenceCollection.internQualifiedNames(makeStringSet(qualifiedStrings));
 		// if a well known qualified name was found then we can skip over these
@@ -1022,11 +1042,24 @@ public class AjState {
 				File file = (File)entry.getKey();
 				if (file.exists()) {
 					if (!lastTimeSources.contains(file)) {  //??? O(n**2)
+						if (listenerDefined()) {
+							getListener().recordDecision("Need to recompile '"+file.getName().toString()+"'");
+						}
 						addTo.add(file);
 					}
 				}
 			}
 		}
+		// add in the things we compiled previously - I know that seems crap but otherwise we may pull woven
+		// stuff off disk (since we no longer have UnwovenClassFile objects) in order to satisfy references
+		// in the new files we are about to compile (see pr133532)
+		// XXX Promote addTo to a Set - then we don't need this rubbish? but does it need to be ordered?
+		if (addTo.size()>0) {
+			for (Iterator iter = lastTimeSources.iterator(); iter.hasNext();) {
+				Object element = (Object) iter.next();
+				if (!addTo.contains(element)) addTo.add(element);
+			}
+		}
 		
 		qualifiedStrings.clear();
 		simpleStrings.clear();
@@ -1309,4 +1342,13 @@ public class AjState {
 	public void initializeAspectNamesList() {
 		this.aspectNames = new LinkedList();
 	}
+	
+	// Will allow us to record decisions made during incremental processing, hopefully aid in debugging
+	public boolean listenerDefined() {
+		return stateListener!=null;
+	}
+	
+	public IStateListener getListener() {
+		return stateListener;
+	}
 }
diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/IStateListener.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/IStateListener.java
index f88501a83..9c222293a 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/IStateListener.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/IStateListener.java
@@ -40,5 +40,16 @@ public interface IStateListener {
 	public void detectedAspectDeleted(File f);
 
 	public void buildSuccessful(boolean wasFullBuild);
+	
+	/**
+	 * When a decision is made during compilation (such as needing to recompile some new file, or drop back to batch) this
+	 * method is called with the decision.
+	 */
+	public void recordDecision(String decision);
+	
+	/**
+	 * Provides feedback during compilation on what stage we are at
+	 */
+	public void recordInformation(String info);
 
 }