Matthew's fixes for failing weaver unit tests.

ajde/testdata/bug-36071/config.lst

@@ -1,2 +0,0 @@
-src/Main.java
-src/test/TestProperties.java

ajde/testdata/bug-36071/config1.lst

@@ -0,0 +1,2 @@
+src/Main.java
+src/testsrc/TestProperties.java

ajde/testdata/bug-36071/config2.lst

@@ -0,0 +1 @@
+src/aspects/Logging.java

ajde/testsrc/org/aspectj/ajde/NullIdeManager.java

@@ -74,4 +74,13 @@ public class NullIdeManager {
 	public List getCompilationSourceLineTasks() {
 		return taskListManager.getSourceLineTasks();
 	}
+	
+	public NullIdeProperties getProjectProperties() {
+		return projectProperties;
+	}
+
+	public void setProjectProperties(NullIdeProperties properties) {
+		projectProperties = properties;
+	}
+
 }

ajde/testsrc/org/aspectj/ajde/NullIdeTaskListManager.java

@@ -42,9 +42,16 @@ public class NullIdeTaskListManager implements TaskListManager {
     	sourceLineTasks.add(new SourceLineTask(message));
         if (!hasWarning && IMessage.WARNING.isSameOrLessThan(message.getKind())) {
             hasWarning = true;
-        }
-    	System.out.println("NullIde> task: " + message.getMessage() + ", file: " + message.getSourceLocation().getSourceFile().getAbsolutePath()
-    		+ ": " +  message.getSourceLocation().getLine());
+        }        
+		/* Guard against null source locations e.g. JAR file messages */
+		if (null != message.getSourceLocation()) {
+			System.out.println("> added sourceline task: " + message + ", file: " + message.getSourceLocation().getSourceFile().getAbsolutePath()
+				+ ": " +  message.getSourceLocation().getLine());
+		}
+		else {
+			System.out.println("> added sourceline task: " + message);
+		}
+
     }
    
     public void addProjectTask(String message, IMessage.Kind kind) {

ajde/testsrc/org/aspectj/ajde/ResourceCopyTestCase.java

@@ -22,11 +22,11 @@ public class ResourceCopyTestCase extends AjdeTestCase {
 
 	public static final String PROJECT_DIR = "bug-36071"; 
 	public static final String srcDir = PROJECT_DIR + "/src"; 
-	public static final String binDir = PROJECT_DIR + "/bin"; 
+	public static final String binDir = "bin"; 
 
-	public static final String injar1 = "testdata/bug-40943/input1.jar"; 
-	public static final String injar2 = "testdata/bug-40943/input2.jar"; 
-	public static final String outjar = "testdata/bug-40943/output.jar"; 
+	public static final String injar1Name = "input1.jar"; 
+	public static final String injar2Name = "input2.jar"; 
+	public static final String outjarName = "/bin/output.jar"; 
 
 	/**
 	 * Constructor for JarResourceCopyTestCase.
@@ -41,133 +41,132 @@ public class ResourceCopyTestCase extends AjdeTestCase {
 	 */	
 	protected void setUp() throws Exception {
 		super.setUp(PROJECT_DIR);
-		FileUtil.deleteContents(new File(binDir));
+		FileUtil.deleteContents(openFile(binDir));
 	}
-	
+
 	public void testSrcToBin () {
-		doSynchronousBuild("config.lst");
 		assertTrue(!Ajde.getDefault().getTaskListManager().hasWarning());
-		assertTrue(new java.io.File("testdata/bug-36071").getAbsolutePath(), compareDirs("src", "bin"));
+		assertTrue("Build failed",doSynchronousBuild("config1.lst"));
+		compareDirs("src","bin");
+	}
+	
+	public void testInjarsToOutjar () {
+		Set injars = new HashSet();
+		File injar1 = openFile(injar1Name);
+		injars.add(injar1);
+		ideManager.getProjectProperties().setInJars(injars);
+		File outjar = openFile(outjarName);
+		ideManager.getProjectProperties().setOutJar(outjar.getAbsolutePath());
+		assertTrue("Build failed",doSynchronousBuild("config2.lst"));
+		assertFalse("No build warnings",ideManager.getCompilationSourceLineTasks().isEmpty());
+		List msgs = NullIdeManager.getIdeManager().getCompilationSourceLineTasks();
+		assertTrue("Wrong message",((NullIdeTaskListManager.SourceLineTask)msgs.get(0)).message.getMessage().startsWith("manifest not copied: "));
+		compareJars(injar1,"src",outjar);
 	}
 	
-//	public void testInjarsToBin () {
-//		List args = new ArrayList();
-//		args.add("-injars");
-//		args.add(injar1);
-//		
-//		args.add("-d");
-//		args.add(binDir);
-//
-//		args.add("-classpath");
-//		args.add("../runtime/bin");
-//	
-//		args.add("testdata/bug-40943/aspects/Logging.java");
-//	
-//		CommandTestCase.runCompiler(args, CommandTestCase.NO_ERRORS);
-//
-//		assertTrue(new java.io.File("testdata/bug-40943").getAbsolutePath(),compareJarToDirs(injar1,binDir));
-//	}
-//
-//	public void testInjarsToOutjar () {
-//		List args = new ArrayList();
-//		args.add("-injars");
-//		args.add(injar1);
-//		
-//		args.add("-outjar");
-//		args.add(outjar);
-//
-//		args.add("-classpath");
-//		args.add("../runtime/bin");
-//	
-//		args.add("testdata/bug-40943/aspects/Logging.java");
-//	
-//		CommandTestCase.runCompiler(args, CommandTestCase.NO_ERRORS);
-//
-//		assertTrue(new java.io.File("testdata/bug-40943").getAbsolutePath(),compareJars(injar1,outjar));
-//	}
-//	
-//
-//	public void test2InjarsToOutjar () {
-//		System.err.println("? test2InjarsToOutjar()");
-//		List args = new ArrayList();
-//		args.add("-injars");
-//		args.add(injar1 + ";" + injar2);
-//		
-//		args.add("-outjar");
-//		args.add(outjar);
-//
-//		args.add("-classpath");
-//		args.add("../runtime/bin");
-//	
-//		args.add("testdata/bug-40943/aspects/Logging.java");
-//	
-//		CommandTestCase.runCompiler(args, CommandTestCase.NO_ERRORS);
-//
-//		assertTrue(new java.io.File("testdata/bug-40943").getAbsolutePath(),compareJars(injar1,outjar));
-//	}
+	public void testDuplicateResources () {
+		Set injars = new HashSet();
+		File injar1 = openFile(injar1Name);
+		File injar2 = openFile(injar2Name);
+		injars.add(injar1);
+		injars.add(injar2);
+		ideManager.getProjectProperties().setInJars(injars);
+		File outjar = openFile(outjarName);
+		ideManager.getProjectProperties().setOutJar(outjar.getAbsolutePath());
+		assertFalse("Build should have failed",doSynchronousBuild("config2.lst"));
+		assertFalse("No build errors",ideManager.getCompilationSourceLineTasks().isEmpty());
+		List msgs = NullIdeManager.getIdeManager().getCompilationSourceLineTasks();
+		assertTrue("Wrong message",((NullIdeTaskListManager.SourceLineTask)msgs.get(1)).message.getMessage().startsWith("duplicate resource: "));
+	}
+	
+	public void testSrcToOutjar () {
+		File outjar = openFile(outjarName);
+		ideManager.getProjectProperties().setOutJar(outjar.getAbsolutePath());
+		assertTrue("Build failed",doSynchronousBuild("config1.lst"));
+		compareSourceToOutjar("src",outjar);
+	}
+	
+	public void testInjarsToBin () {
+		Set injars = new HashSet();
+		File injar1 = openFile(injar1Name);
+		injars.add(injar1);
+		ideManager.getProjectProperties().setInJars(injars);
+		assertTrue("Build failed",doSynchronousBuild("config2.lst"));
+		assertFalse("No build warnings",ideManager.getCompilationSourceLineTasks().isEmpty());
+		List msgs = NullIdeManager.getIdeManager().getCompilationSourceLineTasks();
+		assertTrue("Wrong message",((NullIdeTaskListManager.SourceLineTask)msgs.get(0)).message.getMessage().startsWith("manifest not copied: "));
+		compareInjarsToBin(injar1,"src","bin");
+	}
 	
 	/*
-	 * Ensure -outjar conatins all non-Java resouces from injars
+	 * Ensure bin contains all non-Java resouces from source and injars
 	 */
-	public boolean compareDirs (String indirName, String outdirName) {
-		File srcBase = openFile(indirName);
+	public void compareDirs (String indirName, String outdirName) {
 		File binBase = openFile(outdirName);
-		File[] fromResources = FileUtil.listFiles(srcBase,aspectjResourceFileFilter);
 		File[] toResources = FileUtil.listFiles(binBase,aspectjResourceFileFilter);
 
-		HashSet resources = new HashSet();		
-		for (int i = 0; i < fromResources.length; i++) {
-			resources.add(FileUtil.normalizedPath(fromResources[i],srcBase));
-		}
+		HashSet resources = new HashSet();
+		listSourceResources(indirName,resources);		
 		
 		for (int i = 0; i < toResources.length; i++) {
 			String fileName = FileUtil.normalizedPath(toResources[i],binBase);
 			boolean b = resources.remove(fileName);
-			assertTrue(fileName,b);
+			assertTrue("Extraneous resoures:" + fileName,b);
 		}
 		
-		assertTrue(resources.toString(), resources.isEmpty());
-		return true;
+		assertTrue("Missing resources:" + resources.toString(), resources.isEmpty());
 	}	
 	
 	/*
-	 * Ensure -outjar conatins all non-Java resouces from injars
+	 * Ensure -outjar contains all non-Java resouces from injars
 	 */
-	public boolean compareJarToDirs (String injarName , String outdirName) {
-		File baseDir = new File(binDir);
-		System.err.println("? compareJarToDirs() baseDir='" + baseDir + "'");
-		File[] files = FileUtil.listFiles(baseDir,aspectjResourceFileFilter);
-		for (int i = 0; i < files.length; i++) {
-			System.err.println("? compareJarToDirs() name='" + files[i] + "'");
-		}
-		
-		return false;
-	}	
-	
-	/*
-	 * Ensure -outjar conatins all non-Java resouces from injars
-	 */
-	public boolean compareJars (String injarName , String outjarName) {
+	public void compareJars (File injarFile, String indirName, File outjarFile) {
 	
 		HashSet resources = new HashSet();
 	
 		try {	
-			File injarFile = new File(injarName);
-			File outjarFile = new File(outjarName);
 			assertTrue("outjar older than injar",(outjarFile.lastModified() > injarFile.lastModified()));
-			
-			ZipInputStream injar = new ZipInputStream(new java.io.FileInputStream(injarFile));
+			byte[] inManifest = listJarResources(injarFile,resources);
+			listSourceResources(indirName,resources);		
+
+			ZipInputStream outjar = new ZipInputStream(new java.io.FileInputStream(outjarFile));
 			ZipEntry entry;
-			while (null != (entry = injar.getNextEntry())) {
+			while (null != (entry = outjar.getNextEntry())) {
 				String fileName = entry.getName();
 				if (!fileName.endsWith(".class")) {
-					resources.add(fileName);
+					
+					/* Ensure we didn't copy any JAR manifests */
+					if (fileName.toLowerCase().startsWith("meta-inf")) {
+						byte[] outManifest = FileUtil.readAsByteArray(outjar);
+//						System.err.println("? compareJars() fileName='" + fileName + "', manifest='" + new String(outManifest) + "'");
+						assertFalse("Manifest has been copied",Arrays.equals(inManifest,outManifest));
+					}
+					
+					boolean b = resources.remove(fileName);
+					assertTrue(fileName,b);
 				}
-				injar.closeEntry();
+				outjar.closeEntry();
 			}
-			injar.close();
+			outjar.close();
+
+			assertTrue(resources.toString(),resources.isEmpty());
+		}
+		catch (IOException ex) {
+			fail(ex.toString());
+		}
+	}
+	
+	/*
+	 * Ensure -outjar conatins all non-Java resouces from source and injars
+	 */
+	public void compareSourceToOutjar (String indirName, File outjarFile) {
+		HashSet resources = new HashSet();		
+		listSourceResources(indirName,resources);		
+	
+		try {	
 
 			ZipInputStream outjar = new ZipInputStream(new java.io.FileInputStream(outjarFile));
+			ZipEntry entry;
 			while (null != (entry = outjar.getNextEntry())) {
 				String fileName = entry.getName();
 				if (!fileName.endsWith(".class")) {
@@ -178,19 +177,92 @@ public class ResourceCopyTestCase extends AjdeTestCase {
 			}
 			outjar.close();
 
-			assertTrue(resources.toString(),resources.isEmpty());
+			assertTrue("Missing resources:" + resources.toString(), resources.isEmpty());
+		}
+		catch (IOException ex) {
+			fail(ex.toString());
+		}
+	}
+	
+	/*
+	 * Ensure bin contains all non-Java resouces from source and injars
+	 */
+	public void compareInjarsToBin(File injarFile, String indirName, String outdirName) {
+	
+		HashSet resources = new HashSet();
+	
+		try {	
+			byte[] inManifest = listJarResources(injarFile,resources);
+			listSourceResources(indirName,resources);		
+			
+			File binBase = openFile(outdirName);
+			File[] toResources = FileUtil.listFiles(binBase,aspectjResourceFileFilter);
+			for (int i = 0; i < toResources.length; i++) {
+				String fileName = FileUtil.normalizedPath(toResources[i],binBase);
+
+				/* Ensure we didn't copy any JAR manifests */
+				if (fileName.toLowerCase().startsWith("meta-inf")) {
+					byte[] outManifest = FileUtil.readAsByteArray(toResources[i]);
+//					System.err.println("? compareJars() fileName='" + fileName + "', manifest='" + new String(outManifest) + "'");
+					assertFalse("Manifest has been copied",Arrays.equals(inManifest,outManifest));
+				}
+				boolean b = resources.remove(fileName);
+				assertTrue("Extraneous resoures:" + fileName,b);
+			}
+
+			assertTrue("Missing resources:" + resources.toString(), resources.isEmpty());
 		}
 		catch (IOException ex) {
 			fail(ex.toString());
 		}
-		
-		return true;
 	}
     
+    private void listSourceResources (String indirName, Set resources) {
+		File srcBase = openFile(indirName);
+		File[] fromResources = FileUtil.listFiles(srcBase,aspectjResourceFileFilter);
+		for (int i = 0; i < fromResources.length; i++) {
+			String name = FileUtil.normalizedPath(fromResources[i],srcBase);
+			if (!name.startsWith("CVS/") && (-1 == name.indexOf("/CVS/")) && !name.endsWith("/CVS")) {
+				resources.add(name);
+			}
+		}
+    }
+    
+    private byte[] listJarResources (File injarFile, Set resources) {
+		byte[] manifest = null;
+	
+		try {
+			ZipInputStream injar = new ZipInputStream(new java.io.FileInputStream(injarFile));
+			ZipEntry entry;
+			while (null != (entry = injar.getNextEntry())) {
+				String fileName = entry.getName();
+				if (!fileName.endsWith(".class")) {
+					
+					/* JAR manifests shouldn't be copied */
+					if (fileName.toLowerCase().startsWith("meta-inf")) {
+						manifest = FileUtil.readAsByteArray(injar);
+//						System.err.println("? compareJars() fileName='" + fileName + "', manifest='" + new String(manifest) + "'");
+					}
+					else {
+						resources.add(fileName);
+					}
+				}
+				injar.closeEntry();
+			}
+			injar.close();
+		}	
+		catch (IOException ex) {
+			fail(ex.toString());
+		}
+		
+		return manifest;
+    }
+    
 	public static final FileFilter aspectjResourceFileFilter = new FileFilter() {
 		public boolean accept(File pathname) {
 			String name = pathname.getName().toLowerCase();
-			return !name.endsWith(".class") && !name.endsWith(".java") && !name.endsWith(".aj");
+			return (!name.endsWith(".class") && !name.endsWith(".java") && !name.endsWith(".aj"));
+
 		}
 	};
 

ajde/testsrc/org/aspectj/ajde/StructureModelTest.java

@@ -128,6 +128,8 @@ public class StructureModelTest extends AjdeTestCase {
 			
 		assertTrue("find result", node != null);	
 			
+		System.err.println("##### " + node);
+			
 		IProgramElement pNode = (IProgramElement)((IProgramElement)node).getParent();
         if (null == pNode) {
             assertTrue("null parent of " + node, false);

weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java

@@ -47,7 +47,7 @@ public class BcelWeaver implements IWeaver {
     private Map  sourceJavaClasses = new HashMap();   /* String -> UnwovenClassFile */
     private List addedClasses      = new ArrayList(); /* List<UnovenClassFile> */
     private List deletedTypenames  = new ArrayList(); /* List<String> */
-    private List resources         = new ArrayList();   /* String -> byte[] */ 
+    private Map  resources         = new HashMap(); /* String -> UnwovenClassFile */ 
     private boolean needToReweaveWorld = false;
 
     private List shadowMungerList = null; // setup by prepareForWeave
@@ -112,6 +112,7 @@ public class BcelWeaver implements IWeaver {
 	/** Adds all class files in the jar
 	 */
 	public void addJarFile(File inFile, File outDir) throws IOException {
+//		System.err.println("? addJarFile(" + inFile + ", " + outDir + ")");
 		needToReweaveWorld = true;
 		ZipInputStream inStream = new ZipInputStream(new FileInputStream(inFile)); //??? buffered
 		
@@ -121,14 +122,22 @@ public class BcelWeaver implements IWeaver {
 			
 			byte[] bytes = FileUtil.readAsByteArray(inStream);
 			String filename = entry.getName();
+			UnwovenClassFile classFile = new UnwovenClassFile(new File(outDir, filename).getAbsolutePath(), bytes);
 
 			if (filename.endsWith(".class")) {
-				UnwovenClassFile classFile = new UnwovenClassFile(new File(outDir, filename).getAbsolutePath(), bytes);
 				this.addClassFile(classFile);
 			}
 			else if (!entry.isDirectory()) {
-				UnwovenClassFile resourceFile = new UnwovenClassFile(filename, bytes);
-				addResource(resourceFile);
+//				System.err.println("? addJarFile() filename='" + filename + "'");
+				
+				/* Don't copy JAR manifests */
+				if (filename.toLowerCase().startsWith("meta-inf")) {
+					world.showMessage(IMessage.WARNING, "manifest not copied: '" + filename + 
+									"' in JAR '" + inFile + "'", null, null);
+				}
+				else {
+					addResource(filename,classFile);
+				}
 			}
 
 			inStream.closeEntry();
@@ -138,12 +147,18 @@ public class BcelWeaver implements IWeaver {
 	}
 
 	public void addResource(String name, File inPath, File outDir) throws IOException {
-		BufferedInputStream inStream = new BufferedInputStream(new FileInputStream(inPath));
-		byte[] bytes = new byte[(int)inPath.length()];
-		inStream.read(bytes);
-		UnwovenClassFile resourceFile = new UnwovenClassFile(new File(outDir, name).getAbsolutePath(), bytes);
-		addResource(resourceFile);
-		inStream.close();
+
+		/* Eliminate CVS files. Relative paths use "/" */
+		if (!name.startsWith("CVS/") && (-1 == name.indexOf("/CVS/")) && !name.endsWith("/CVS")) {
+//			System.err.println("? addResource('" + name + "')");
+//			BufferedInputStream inStream = new BufferedInputStream(new FileInputStream(inPath));
+//			byte[] bytes = new byte[(int)inPath.length()];
+//			inStream.read(bytes);
+//			inStream.close();
+			byte[] bytes = FileUtil.readAsByteArray(inPath);
+			UnwovenClassFile resourceFile = new UnwovenClassFile(new File(outDir, name).getAbsolutePath(), bytes);
+			addResource(name,resourceFile);
+		}
 	}
     
     /** Should be addOrReplace
@@ -163,8 +178,12 @@ public class BcelWeaver implements IWeaver {
     	world.deleteSourceObjectType(TypeX.forName(typename));
     }
 
-	public void addResource (UnwovenClassFile resourceFile) {
-		resources.add(resourceFile);
+	public void addResource (String name, UnwovenClassFile resourceFile) {
+		Object previous = resources.put(name, resourceFile);
+		if (null != previous) {
+			world.showMessage(IMessage.ERROR, "duplicate resource: '" + name + "'",
+				null, null);
+		}
 	}
 
 	// ---- weave preparation
@@ -208,7 +227,7 @@ public class BcelWeaver implements IWeaver {
     	this.zipOutputStream = new ZipOutputStream(os);
     	dumpUnwoven();
 		/* BUG 40943*/
-		dumpResourcesOutJar();
+		dumpResourcesToOutJar();
     	zipOutputStream.close();  //this flushes and closes the acutal file
     }
     
@@ -219,23 +238,25 @@ public class BcelWeaver implements IWeaver {
             UnwovenClassFile classFile = (UnwovenClassFile)i.next();
             dumpUnchanged(classFile);
        	}
-       	dumpResourcesToOutPath();
     }
     
-	private void dumpResourcesToOutPath() throws IOException {
-		Iterator i = resources.iterator();
+	public void dumpResourcesToOutPath() throws IOException {
+//		System.err.println("? dumpResourcesToOutPath() resources=" + resources.keySet());
+		Iterator i = resources.keySet().iterator();
 		while (i.hasNext()) {
-			UnwovenClassFile res = (UnwovenClassFile)i.next();
+			UnwovenClassFile res = (UnwovenClassFile)resources.get(i.next());
 			dumpUnchanged(res);
-		}		
+		}
 	}
 
 	/* BUG #40943 */
-    public void dumpResourcesOutJar() throws IOException {
-		Iterator i = resources.iterator();
+    public void dumpResourcesToOutJar() throws IOException {
+//		System.err.println("? dumpResourcesToOutJar() resources=" + resources.keySet());
+		Iterator i = resources.keySet().iterator();
 		while (i.hasNext()) {
-			UnwovenClassFile res = (UnwovenClassFile)i.next();
-			writeZipEntry(res.getFilename(),res.getBytes());
+			String name = (String)i.next();
+			UnwovenClassFile res = (UnwovenClassFile)resources.get(name);
+			writeZipEntry(name,res.getBytes());
 		}
     }
     
@@ -246,7 +267,7 @@ public class BcelWeaver implements IWeaver {
     	this.zipOutputStream = new ZipOutputStream(os);
     	Collection c = weave();
     	/* BUG 40943*/
-    	dumpResourcesOutJar();
+    	dumpResourcesToOutJar();
     	zipOutputStream.close();  //this flushes and closes the acutal file
     	return c;
     }
@@ -319,8 +340,6 @@ public class BcelWeaver implements IWeaver {
         
         addedClasses = new ArrayList();
     	deletedTypenames = new ArrayList();
-        
-		dumpResourcesToOutPath();
 		
         return wovenClassNames;
     }