Community
Participate
Working Groups
- Create a Java project. - Add aspectjrt.jar (as ASPECTJRT_LIB variable or from Spring's distribution etc.) - Convert to AspectJ project. - Remove AspectJ nature. This removes the user-added aspectjrt.jar leading to errors in all @AspectJ aspects. I use this style to convert add AspectJ nature for debugging and quick feedback through advice markers etc. for project that will be using LTW eventually. Every time, I remove that nature to test under near-production environment, I face this problem. I think at the minimum, removing the AspectJ nature should not remove aspectjrt.jar unless it was added due to conversion to AspectJ project. Perhaps, this can be done by checking if aspectjrt.jar is the same as the ASPECTJRT_LIB variable. Definitely, it shouldn't remove aspectjrt.jar added from, say, Spring distribution. I think the best solution may be to simply prompt use if aspectjrt.jar should be removed (and even then, I think, it should do so only for the one matching ASPECTJRT_LIB)
easy to implement?
Created attachment 110821 [details] This patch adds a user prompt when removing aj nature for non-pde projects. Includes updates to two tests. The prompt asks whether the user wants to remove the AJRT container from the classpath when removing aj natures from non-pde projects. There already is a prompt for pde projects. I am now using the same functionality for both.
Comment on attachment 110821 [details] This patch adds a user prompt when removing aj nature for non-pde projects. Includes updates to two tests. >### Eclipse Workspace Patch 1.0 >#P org.eclipse.ajdt.ui >Index: src/org/eclipse/ajdt/internal/utils/AJDTUtils.java >=================================================================== >RCS file: /cvsroot/tools/org.eclipse.ajdt/AJDT_src/org.eclipse.ajdt.ui/src/org/eclipse/ajdt/internal/utils/AJDTUtils.java,v >retrieving revision 1.36 >diff -u -r1.36 AJDTUtils.java >--- src/org/eclipse/ajdt/internal/utils/AJDTUtils.java 19 Jul 2008 00:20:32 -0000 1.36 >+++ src/org/eclipse/ajdt/internal/utils/AJDTUtils.java 25 Aug 2008 16:59:19 -0000 >@@ -68,6 +68,7 @@ > import org.eclipse.jdt.core.JavaModelException; > import org.eclipse.jdt.internal.ui.packageview.PackageExplorerPart; > import org.eclipse.jdt.ui.JavaElementImageDescriptor; >+import org.eclipse.jface.dialogs.Dialog; > import org.eclipse.jface.dialogs.MessageDialog; > import org.eclipse.jface.resource.ImageDescriptor; > import org.eclipse.pde.core.plugin.IPluginImport; >@@ -653,8 +661,14 @@ > > } > } else { >- // Update the build classpath to try and remove the aspectjrt.jar >- AspectJUIPlugin.removeAjrtFromBuildPath(project); >+ IWorkbenchWindow window = AspectJUIPlugin.getDefault().getWorkbench() >+ .getActiveWorkbenchWindow(); >+ if ((AspectJPreferences.askPDEAutoRemoveImport() && confirmAutoRemoveImport(window, false)) >+ || (AspectJPreferences.doPDEAutoRemoveImport())) { >+ >+ // Update the build classpath to try and remove the aspectjrt.jar >+ AspectJUIPlugin.removeAjrtFromBuildPath(project); >+ } > } > > AspectJPlugin.getDefault().getCompilerFactory().removeCompilerForProject(project); >@@ -814,7 +828,7 @@ > private static void removeAJPluginDependency(ManifestEditor manEd) { > IWorkbenchWindow window = AspectJUIPlugin.getDefault().getWorkbench() > .getActiveWorkbenchWindow(); >- if ((AspectJPreferences.askPDEAutoRemoveImport() && confirmPDEAutoRemoveImport(window)) >+ if ((AspectJPreferences.askPDEAutoRemoveImport() && confirmAutoRemoveImport(window, true)) > || (AspectJPreferences.doPDEAutoRemoveImport())) { > > >@@ -954,16 +968,21 @@ > * Prompts the user for whether to automatically remove the AspectJ runtime plug-in > * dependency when removing AspectJ nature from a PDE project. > * >+ * @param window the workbench windo that the doalog box will be modal on >+ * @param pde whether or not to use pde related messages (bug 170043) >+ * > * @return <code>true</code> if it's OK to import, <code>false</code> > * otherwise > */ >- private static boolean confirmPDEAutoRemoveImport(IWorkbenchWindow window) { >+ private static boolean confirmAutoRemoveImport(IWorkbenchWindow window, boolean pde) { > > MessageDialogWithToggle dialog = MessageDialogWithToggle > .openQuestion( > window.getShell(), >- UIMessages.PluginImportDialog_removeImportConfirmTitle, >- UIMessages.PluginImportDialog_removeImportConfirmMsg, >+ pde ? UIMessages.PluginImportDialog_removeImportConfirmTitle : >+ UIMessages.PluginImportDialog_removeNonPluginImportConfirmTitle, >+ pde ? UIMessages.PluginImportDialog_removeImportConfirmMsg : >+ UIMessages.PluginImportDialog_removeNonPluginImportConfirmMsg, > UIMessages.PluginImportDialog_removeImportConfirmToggleMsg, > false); // toggle is initially unchecked > >Index: src/org/eclipse/ajdt/internal/ui/text/UIMessages.java >=================================================================== >RCS file: /cvsroot/tools/org.eclipse.ajdt/AJDT_src/org.eclipse.ajdt.ui/src/org/eclipse/ajdt/internal/ui/text/UIMessages.java,v >retrieving revision 1.54 >diff -u -r1.54 UIMessages.java >--- src/org/eclipse/ajdt/internal/ui/text/UIMessages.java 19 Jul 2008 00:20:33 -0000 1.54 >+++ src/org/eclipse/ajdt/internal/ui/text/UIMessages.java 25 Aug 2008 16:59:19 -0000 >@@ -74,6 +74,8 @@ > public static String AutoPluginRemoveErrorDialog_message; > public static String PluginImportDialog_removeImportConfirmTitle; > public static String PluginImportDialog_removeImportConfirmMsg; >+ public static String PluginImportDialog_removeNonPluginImportConfirmTitle; >+ public static String PluginImportDialog_removeNonPluginImportConfirmMsg; > public static String PluginImportDialog_removeImportConfirmToggleMsg; > public static String NewAspectJProject_CreateAnAspectJProject; > public static String NewAspectJProject_CreateAnAspectJProjectDescription; >Index: src/org/eclipse/ajdt/internal/ui/text/UIMessages.properties >=================================================================== >RCS file: /cvsroot/tools/org.eclipse.ajdt/AJDT_src/org.eclipse.ajdt.ui/src/org/eclipse/ajdt/internal/ui/text/UIMessages.properties,v >retrieving revision 1.63 >diff -u -r1.63 UIMessages.properties >--- src/org/eclipse/ajdt/internal/ui/text/UIMessages.properties 19 Jul 2008 00:20:33 -0000 1.63 >+++ src/org/eclipse/ajdt/internal/ui/text/UIMessages.properties 25 Aug 2008 16:59:19 -0000 >@@ -92,6 +92,11 @@ > Do you want to remove this now? > PluginImportDialog_removeImportConfirmToggleMsg= &Do not show this dialog again > >+PluginImportDialog_removeNonPluginImportConfirmTitle=Confirm Dependency Removal >+PluginImportDialog_removeNonPluginImportConfirmMsg=Your project has a dependency on the org.aspectj.runtime jar. \ >+Do you want to remove this now? >+ >+ > NewAspectJProject_CreateAnAspectJProject=Create an AspectJ Project > NewAspectJProject_CreateAnAspectJProjectDescription=Create an AspectJ Project in the workspace or in an external location >
Comment on attachment 110821 [details] This patch adds a user prompt when removing aj nature for non-pde projects. Includes updates to two tests. ### Eclipse Workspace Patch 1.0 #P org.eclipse.ajdt.ui Index: src/org/eclipse/ajdt/internal/utils/AJDTUtils.java =================================================================== RCS file: /cvsroot/tools/org.eclipse.ajdt/AJDT_src/org.eclipse.ajdt.ui/src/org/eclipse/ajdt/internal/utils/AJDTUtils.java,v retrieving revision 1.36 diff -u -r1.36 AJDTUtils.java --- src/org/eclipse/ajdt/internal/utils/AJDTUtils.java 19 Jul 2008 00:20:32 -0000 1.36 +++ src/org/eclipse/ajdt/internal/utils/AJDTUtils.java 25 Aug 2008 17:54:57 -0000 @@ -68,6 +68,7 @@ import org.eclipse.jdt.core.JavaModelException; import org.eclipse.jdt.internal.ui.packageview.PackageExplorerPart; import org.eclipse.jdt.ui.JavaElementImageDescriptor; +import org.eclipse.jface.dialogs.Dialog; import org.eclipse.jface.dialogs.MessageDialog; import org.eclipse.jface.resource.ImageDescriptor; import org.eclipse.pde.core.plugin.IPluginImport; @@ -629,7 +630,7 @@ }// end for description.setNatureIds(newNatures); project.setDescription(description, null); - + // Bugzilla 62625 // Bugzilla 93532 - just remove plugin dependency if there is a plugin.xml file // Bugzilla 137922 - also consider bundles without a plugin.xml @@ -653,8 +654,14 @@ } } else { - // Update the build classpath to try and remove the aspectjrt.jar - AspectJUIPlugin.removeAjrtFromBuildPath(project); + IWorkbenchWindow window = AspectJUIPlugin.getDefault().getWorkbench() + .getActiveWorkbenchWindow(); + if ((AspectJPreferences.askPDEAutoRemoveImport() && confirmAutoRemoveImport(window, false)) + || (AspectJPreferences.doPDEAutoRemoveImport())) { + + // Update the build classpath to try and remove the aspectjrt.jar + AspectJUIPlugin.removeAjrtFromBuildPath(project); + } } AspectJPlugin.getDefault().getCompilerFactory().removeCompilerForProject(project); @@ -814,7 +821,7 @@ private static void removeAJPluginDependency(ManifestEditor manEd) { IWorkbenchWindow window = AspectJUIPlugin.getDefault().getWorkbench() .getActiveWorkbenchWindow(); - if ((AspectJPreferences.askPDEAutoRemoveImport() && confirmPDEAutoRemoveImport(window)) + if ((AspectJPreferences.askPDEAutoRemoveImport() && confirmAutoRemoveImport(window, true)) || (AspectJPreferences.doPDEAutoRemoveImport())) { @@ -954,16 +961,21 @@ * Prompts the user for whether to automatically remove the AspectJ runtime plug-in * dependency when removing AspectJ nature from a PDE project. * + * @param window the workbench windo that the doalog box will be modal on + * @param pde whether or not to use pde related messages (bug 170043) + * * @return <code>true</code> if it's OK to import, <code>false</code> * otherwise */ - private static boolean confirmPDEAutoRemoveImport(IWorkbenchWindow window) { + private static boolean confirmAutoRemoveImport(IWorkbenchWindow window, boolean pde) { MessageDialogWithToggle dialog = MessageDialogWithToggle .openQuestion( window.getShell(), - UIMessages.PluginImportDialog_removeImportConfirmTitle, - UIMessages.PluginImportDialog_removeImportConfirmMsg, + pde ? UIMessages.PluginImportDialog_removeImportConfirmTitle : + UIMessages.PluginImportDialog_removeNonPluginImportConfirmTitle, + pde ? UIMessages.PluginImportDialog_removeImportConfirmMsg : + UIMessages.PluginImportDialog_removeNonPluginImportConfirmMsg, UIMessages.PluginImportDialog_removeImportConfirmToggleMsg, false); // toggle is initially unchecked Index: src/org/eclipse/ajdt/internal/ui/text/UIMessages.java =================================================================== RCS file: /cvsroot/tools/org.eclipse.ajdt/AJDT_src/org.eclipse.ajdt.ui/src/org/eclipse/ajdt/internal/ui/text/UIMessages.java,v retrieving revision 1.54 diff -u -r1.54 UIMessages.java --- src/org/eclipse/ajdt/internal/ui/text/UIMessages.java 19 Jul 2008 00:20:33 -0000 1.54 +++ src/org/eclipse/ajdt/internal/ui/text/UIMessages.java 25 Aug 2008 17:54:56 -0000 @@ -74,6 +74,8 @@ public static String AutoPluginRemoveErrorDialog_message; public static String PluginImportDialog_removeImportConfirmTitle; public static String PluginImportDialog_removeImportConfirmMsg; + public static String PluginImportDialog_removeNonPluginImportConfirmTitle; + public static String PluginImportDialog_removeNonPluginImportConfirmMsg; public static String PluginImportDialog_removeImportConfirmToggleMsg; public static String NewAspectJProject_CreateAnAspectJProject; public static String NewAspectJProject_CreateAnAspectJProjectDescription; Index: src/org/eclipse/ajdt/internal/ui/text/UIMessages.properties =================================================================== RCS file: /cvsroot/tools/org.eclipse.ajdt/AJDT_src/org.eclipse.ajdt.ui/src/org/eclipse/ajdt/internal/ui/text/UIMessages.properties,v retrieving revision 1.63 diff -u -r1.63 UIMessages.properties --- src/org/eclipse/ajdt/internal/ui/text/UIMessages.properties 19 Jul 2008 00:20:33 -0000 1.63 +++ src/org/eclipse/ajdt/internal/ui/text/UIMessages.properties 25 Aug 2008 17:54:56 -0000 @@ -92,6 +92,11 @@ Do you want to remove this now? PluginImportDialog_removeImportConfirmToggleMsg= &Do not show this dialog again +PluginImportDialog_removeNonPluginImportConfirmTitle=Confirm Dependency Removal +PluginImportDialog_removeNonPluginImportConfirmMsg=Your project has a dependency on the org.aspectj.runtime jar. \ +Do you want to remove this now? + + NewAspectJProject_CreateAnAspectJProject=Create an AspectJ Project NewAspectJProject_CreateAnAspectJProjectDescription=Create an AspectJ Project in the workspace or in an external location #P org.eclipse.ajdt.ui.tests Index: src/org/eclipse/ajdt/ui/tests/actions/AddAJNatureActionTest.java =================================================================== RCS file: /cvsroot/tools/org.eclipse.ajdt/AJDT_src/org.eclipse.ajdt.ui.tests/src/org/eclipse/ajdt/ui/tests/actions/AddAJNatureActionTest.java,v retrieving revision 1.5 diff -u -r1.5 AddAJNatureActionTest.java --- src/org/eclipse/ajdt/ui/tests/actions/AddAJNatureActionTest.java 8 Jun 2006 10:57:01 -0000 1.5 +++ src/org/eclipse/ajdt/ui/tests/actions/AddAJNatureActionTest.java 25 Aug 2008 17:54:58 -0000 @@ -57,5 +57,6 @@ // Attempt to add the nature aja.run(action); assertTrue(AspectJPlugin.isAJProject(testProject)); + assertTrue("Should have aspectjruntime.jar on the classpath", RemoveAJNatureActionTest.hasAjrtOnBuildPath(testProject)); //$NON-NLS-1$ } } Index: src/org/eclipse/ajdt/ui/tests/actions/RemoveAJNatureActionTest.java =================================================================== RCS file: /cvsroot/tools/org.eclipse.ajdt/AJDT_src/org.eclipse.ajdt.ui.tests/src/org/eclipse/ajdt/ui/tests/actions/RemoveAJNatureActionTest.java,v retrieving revision 1.6 diff -u -r1.6 RemoveAJNatureActionTest.java --- src/org/eclipse/ajdt/ui/tests/actions/RemoveAJNatureActionTest.java 8 Jun 2006 10:57:01 -0000 1.6 +++ src/org/eclipse/ajdt/ui/tests/actions/RemoveAJNatureActionTest.java 25 Aug 2008 17:54:58 -0000 @@ -11,12 +11,22 @@ *******************************************************************************/ package org.eclipse.ajdt.ui.tests.actions; +import java.util.ArrayList; + +import org.eclipse.ajdt.core.AspectJCorePreferences; import org.eclipse.ajdt.core.AspectJPlugin; import org.eclipse.ajdt.internal.ui.actions.RemoveAJNatureAction; +import org.eclipse.ajdt.internal.ui.preferences.AspectJPreferences; import org.eclipse.ajdt.ui.AspectJUIPlugin; import org.eclipse.ajdt.ui.tests.UITestCase; import org.eclipse.core.resources.IProject; import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.IPath; +import org.eclipse.core.runtime.NullProgressMonitor; +import org.eclipse.jdt.core.IClasspathEntry; +import org.eclipse.jdt.core.IJavaProject; +import org.eclipse.jdt.core.JavaCore; +import org.eclipse.jdt.core.JavaModelException; import org.eclipse.jface.action.Action; import org.eclipse.jface.action.IAction; import org.eclipse.jface.viewers.ISelection; @@ -27,34 +37,92 @@ */ public class RemoveAJNatureActionTest extends UITestCase { - private IProject testProject = null; - - protected void setUp() throws Exception { - super.setUp(); - testProject = createPredefinedProject("project.java.Y"); //$NON-NLS-1$ - waitForJobsToComplete(); - AspectJUIPlugin.convertToAspectJProject(testProject); - waitForJobsToComplete(); - } - - public void testRemovesAJNature() throws CoreException { - // Ensure that we are starting with a project that has an AspectJ - // nature. - assertTrue(AspectJPlugin.isAJProject(testProject)); - - // Next, create the necessary arguments for the nature addition. - ISelection sel = new StructuredSelection(testProject); - IAction action = new Action() { - public void run() { - // NO OP - } - }; - RemoveAJNatureAction rna = new RemoveAJNatureAction(); - rna.selectionChanged(action, sel); - - // Remove the nature - rna.run(action); - assertFalse(AspectJPlugin.isAJProject(testProject)); - } + private IProject testProject = null; + protected void setUp() throws Exception { + super.setUp(); + testProject = createPredefinedProject("project.java.Y"); //$NON-NLS-1$ + waitForJobsToComplete(); + AspectJUIPlugin.convertToAspectJProject(testProject); + waitForJobsToComplete(); + } + + public void testRemovesAJNature() throws CoreException { + // Ensure that we are starting with a project that has an AspectJ + // nature. + assertTrue(AspectJPlugin.isAJProject(testProject)); + + // Next, create the necessary arguments for the nature addition. + ISelection sel = new StructuredSelection(testProject); + IAction action = new Action() { + public void run() { + // NO OP + } + }; + + // avoid prompting for depency removal. + AspectJPreferences.setAskPDEAutoRemoveImport(false); + // automatically remove import from classpath + AspectJPreferences.setDoPDEAutoRemoveImport(true); + + RemoveAJNatureAction rna = new RemoveAJNatureAction(); + rna.selectionChanged(action, sel); + + // Remove the nature + rna.run(action); + assertFalse(AspectJPlugin.isAJProject(testProject)); + assertFalse( + "Should not have aspectjruntime.jar on the classpath", hasAjrtOnBuildPath(testProject)); //$NON-NLS-1$ + } + + public void testRemovesAJNatureKeepClasspath() throws CoreException { + // Ensure that we are starting with a project that has an AspectJ + // nature. + assertTrue(AspectJPlugin.isAJProject(testProject)); + + // Next, create the necessary arguments for the nature addition. + ISelection sel = new StructuredSelection(testProject); + IAction action = new Action() { + public void run() { + // NO OP + } + }; + + // avoid prompting for depency removal. + AspectJPreferences.setAskPDEAutoRemoveImport(false); + // keep the dependency on the classpath + AspectJPreferences.setDoPDEAutoRemoveImport(false); + + RemoveAJNatureAction rna = new RemoveAJNatureAction(); + rna.selectionChanged(action, sel); + + // Remove the nature + rna.run(action); + assertFalse(AspectJPlugin.isAJProject(testProject)); + assertTrue( + "Should have aspectjruntime.jar on the classpath", hasAjrtOnBuildPath(testProject)); //$NON-NLS-1$ + } + + public static boolean hasAjrtOnBuildPath(IProject project) + throws CoreException { + IJavaProject javaProject = JavaCore.create(project); + IClasspathEntry[] originalCP = javaProject.getRawClasspath(); + + // Go through each current classpath entry one at a time. If it + // is not a reference to the aspectjrt.jar then do not add it + // to the collection of new classpath entries. + for (int i = 0; i < originalCP.length; i++) { + IPath path = originalCP[i].getPath(); + if (path.toOSString().endsWith("ASPECTJRT_LIB") //$NON-NLS-1$ + || path.toOSString().endsWith("aspectjrt.jar")) { //$NON-NLS-1$ + return true; + } + if (originalCP[i].getEntryKind() == IClasspathEntry.CPE_CONTAINER) { + if (path.segment(0).equals(AspectJPlugin.ASPECTJRT_CONTAINER)) { + return true; + } + } + }// end for + return false; + } } \ No newline at end of file
Created attachment 110831 [details] Patch to confirm removal of dependency with tests Oops. Thought I could edit the patch directly, but turns out it just converts the patch to a comment. Here is the final version of the patch with the appropriate test cases changed. This makes the previous patch obsolete.
comment 5 patches are in HEAD - may need it in AJDT1.5. iplog
