Bug 336052 - AbstractCLaunchDelegate{2} fails to launch if the Build throws Exception or build cancels the progressmonitor.

launch/org.eclipse.cdt.launch/src/org/eclipse/cdt/launch/AbstractCLaunchDelegate.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2005, 2010 QNX Software Systems and others.
+ * Copyright (c) 2005, 2011 QNX Software Systems and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -11,6 +11,7 @@
  *     Ken Ryall (Nokia) - bug 178731
  *     Anton Leherbauer (Wind River Systems) - bug 224187
  *     Alex Collins (Broadcom Corp.) - choose build config automatically
+ *     James Blackburn (Broadcom Corp.)
  *******************************************************************************/
 package org.eclipse.cdt.launch;
 
@@ -155,6 +156,8 @@ abstract public class AbstractCLaunchDelegate extends LaunchConfigurationDelegat
 	 * Used in conjunction with build before launch settings in the main tab.
 	 */
 	private boolean workspaceBuildBeforeLaunch;
+	/** Flag set to true if build before launch failed, or was cancelled. */
+	private boolean buildFailed;
 	
 	abstract public void launch(ILaunchConfiguration configuration, String mode, ILaunch launch, IProgressMonitor monitor)
 			throws CoreException;
@@ -600,15 +603,21 @@ abstract public class AbstractCLaunchDelegate extends LaunchConfigurationDelegat
 		try {
 			monitor.beginTask(LaunchMessages.AbstractCLaunchDelegate_building_projects, totalWork); 
 
-			for (Iterator i = orderedProjects.iterator(); i.hasNext();) {
+			try {
-				IProject proj = (IProject)i.next();
+				for (Iterator i = orderedProjects.iterator(); i.hasNext();) {
-				monitor.subTask(LaunchMessages.AbstractCLaunchDelegate_building + proj.getName()); 
+					IProject proj = (IProject)i.next();
-				proj.build(IncrementalProjectBuilder.INCREMENTAL_BUILD, new SubProgressMonitor(monitor, scale));
+					monitor.subTask(LaunchMessages.AbstractCLaunchDelegate_building + proj.getName()); 
-			}
+					proj.build(IncrementalProjectBuilder.INCREMENTAL_BUILD, new LaunchUtils.BuildProgressMonitor(monitor, scale));
+				}
 
-			monitor.subTask(LaunchMessages.AbstractCLaunchDelegate_building + project.getName()); 
+				monitor.subTask(LaunchMessages.AbstractCLaunchDelegate_building + project.getName()); 
-			setBuildConfiguration(configuration, project);
+				setBuildConfiguration(configuration, project);
-			project.build(IncrementalProjectBuilder.INCREMENTAL_BUILD, new SubProgressMonitor(monitor, scale));
+				project.build(IncrementalProjectBuilder.INCREMENTAL_BUILD, new LaunchUtils.BuildProgressMonitor(monitor, scale));
+			} catch (Exception e) {
+				// Catch CoreException or OperationCancelledException possibly thrown by the build contract.
+				// Still allow the user to continue to the launch
+				buildFailed = true;
+			}
 		} finally {
 			monitor.done();
 		}
@@ -671,15 +680,20 @@ abstract public class AbstractCLaunchDelegate extends LaunchConfigurationDelegat
 			if (ICDTLaunchConfigurationConstants.BUILD_BEFORE_LAUNCH_ENABLED == configuration.getAttribute(ICDTLaunchConfigurationConstants.ATTR_BUILD_BEFORE_LAUNCH,
 					ICDTLaunchConfigurationConstants.BUILD_BEFORE_LAUNCH_USE_WORKSPACE_SETTING)) {
 				
-				IProgressMonitor buildMonitor = new SubProgressMonitor(monitor, 10, SubProgressMonitor.PREPEND_MAIN_LABEL_TO_SUBTASK);
+				try {
-				buildMonitor.beginTask(LaunchMessages.AbstractCLaunchDelegate_BuildBeforeLaunch, 10); 	
+					IProgressMonitor buildMonitor = new LaunchUtils.BuildProgressMonitor(monitor, 10, SubProgressMonitor.PREPEND_MAIN_LABEL_TO_SUBTASK);
-				buildMonitor.subTask(LaunchMessages.AbstractCLaunchDelegate_PerformingBuild); 
+					buildMonitor.beginTask(LaunchMessages.AbstractCLaunchDelegate_BuildBeforeLaunch, 10); 	
-				if (buildForLaunch(configuration, mode, new SubProgressMonitor(buildMonitor, 7))) {
+					buildMonitor.subTask(LaunchMessages.AbstractCLaunchDelegate_PerformingBuild); 
-					buildMonitor.subTask(LaunchMessages.AbstractCLaunchDelegate_PerformingIncrementalBuild); 
+					if (buildForLaunch(configuration, mode, new SubProgressMonitor(buildMonitor, 7))) {
-					ResourcesPlugin.getWorkspace().build(IncrementalProjectBuilder.INCREMENTAL_BUILD, new SubProgressMonitor(buildMonitor, 3));				
+						buildMonitor.subTask(LaunchMessages.AbstractCLaunchDelegate_PerformingIncrementalBuild); 
-				}
+						ResourcesPlugin.getWorkspace().build(IncrementalProjectBuilder.INCREMENTAL_BUILD, new SubProgressMonitor(buildMonitor, 3));				
-				else {
+					} else {
-					buildMonitor.worked(3); /* No incremental build required */
+						buildMonitor.worked(3); /* No incremental build required */
+					}
+				} catch (Exception e) {
+					// Catch CoreException or OperationCancelledException possibly thrown by the build contract.
+					// Still allow the user to continue to the launch
+					buildFailed = true;
 				}
 			}
 		}
@@ -697,26 +711,28 @@ abstract public class AbstractCLaunchDelegate extends LaunchConfigurationDelegat
 		int totalWork = (orderedProjects.size() + 1) * scale;
 		try {
 			monitor.beginTask(LaunchMessages.AbstractCLaunchDelegate_searching_for_errors, totalWork); 
-			boolean compileErrorsInProjs = false;
+			boolean compileErrorsInProjs = buildFailed;
-			
+
 			//check prerequisite projects for compile errors.
-			for (Iterator i = orderedProjects.iterator(); i.hasNext();) {
+			if (!compileErrorsInProjs) {
-				IProject proj = (IProject)i.next();
+				for (Iterator i = orderedProjects.iterator(); i.hasNext();) {
-				monitor.subTask(LaunchMessages.AbstractCLaunchDelegate_searching_for_errors_in + proj.getName()); 
+					IProject proj = (IProject)i.next();
-				monitor.worked(scale);
+					monitor.subTask(LaunchMessages.AbstractCLaunchDelegate_searching_for_errors_in + proj.getName()); 
-				compileErrorsInProjs = existsErrors(proj);
+					monitor.worked(scale);
-				if (compileErrorsInProjs) {
+					compileErrorsInProjs = existsErrors(proj);
-					break;
+					if (compileErrorsInProjs) {
+						break;
+					}
 				}
 			}
-			
+
 			//check current project, if prerequite projects were ok
 			if (!compileErrorsInProjs) {
 				monitor.subTask(LaunchMessages.AbstractCLaunchDelegate_searching_for_errors_in + project.getName()); 
 				monitor.worked(scale);
 				compileErrorsInProjs = existsErrors(project);
 			}
-			
+
 			//if compile errors exist, ask the user before continuing.
 			if (compileErrorsInProjs) {
 				IStatusHandler prompter = DebugPlugin.getDefault().getStatusHandler(promptStatus);
@@ -844,8 +860,8 @@ abstract public class AbstractCLaunchDelegate extends LaunchConfigurationDelegat
 	 */
 	protected Properties getEnvironmentAsProperty(ILaunchConfiguration config) throws CoreException {
 		String[] envp = getEnvironment(config);
-		Properties p = new Properties();
+		Properties p = new Properties( );
-		for(int i = 0; i < envp.length; i++) {
+		for( int i = 0; i < envp.length; i++ ) {
 			int idx = envp[i].indexOf('=');
 			if (idx != -1) {
 				String key = envp[i].substring(0, idx);

launch/org.eclipse.cdt.launch/src/org/eclipse/cdt/launch/AbstractCLaunchDelegate2.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2010 Nokia and others.
+ * Copyright (c) 2010, 2011 Nokia and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -7,6 +7,7 @@
  *
  * Contributors:
  *     Ken Ryall (Nokia)
+ *     James Blackburn (Broadcom Corp.)
  *******************************************************************************/
 package org.eclipse.cdt.launch;
 
@@ -65,6 +66,8 @@ import org.eclipse.osgi.util.NLS;
 public abstract class AbstractCLaunchDelegate2 extends LaunchConfigurationDelegate {
 
 	private boolean workspaceBuildBeforeLaunch;
+	/** Flag set to true if build before launch failed, or was cancelled. */
+	private boolean buildFailed;
 	private boolean requireCProject;
 
 	public AbstractCLaunchDelegate2() {
@@ -302,13 +305,13 @@ public abstract class AbstractCLaunchDelegate2 extends LaunchConfigurationDelega
 	 * @throws CoreException
 	 */
 	protected void buildProject(final IProject project, final String buildConfigID, IProgressMonitor monitor) throws CoreException {
+		final int TOTAL_TICKS = 1000;
 
 		// Some day, this will hopefully be a simple pass-thru to a cdt.core
 		// utility. See bug 313927
-		
+
 		IWorkspaceRunnable build = new IWorkspaceRunnable(){
 			public void run(IProgressMonitor pm) throws CoreException {
-				final int TOTAL_TICKS = 1000;
 				SubMonitor localmonitor = SubMonitor.convert(pm, "", TOTAL_TICKS); //$NON-NLS-1$
 
 				try {
@@ -357,7 +360,11 @@ public abstract class AbstractCLaunchDelegate2 extends LaunchConfigurationDelega
 				}
 			}
 		};
-		ResourcesPlugin.getWorkspace().run(build, monitor);
+		try {
+			ResourcesPlugin.getWorkspace().run(build, new LaunchUtils.BuildProgressMonitor(monitor, TOTAL_TICKS));
+		} catch (Exception e) {
+			buildFailed = true;
+		}
 	}
 
 	/** TODO: Temporarily duplicated from BuilderFactory. Remove when 313927 is addressed */
@@ -455,7 +462,7 @@ public abstract class AbstractCLaunchDelegate2 extends LaunchConfigurationDelega
 	public boolean finalLaunchCheck(ILaunchConfiguration configuration, String mode, IProgressMonitor monitor) throws CoreException {
 		try {
 			SubMonitor localMonitor = SubMonitor.convert(monitor, LaunchMessages.AbstractCLaunchDelegate_BuildBeforeLaunch, 10); 
-	
+			
 			if (!workspaceBuildBeforeLaunch) {
 				// buildForLaunch was not called which means that the workspace pref is disabled.  see if the user enabled the
 				// launch specific setting in the main tab.  if so, we do call buildBeforeLaunch here.
@@ -469,7 +476,6 @@ public abstract class AbstractCLaunchDelegate2 extends LaunchConfigurationDelega
 					}
 				}
 			}
-
 			// We can just call our super's implementation to have it check for
 			// build errors, but it is too generic. It doesn't know the concept
 			// of a CDT build configuration, and the fact that we requested the
@@ -481,7 +487,7 @@ public abstract class AbstractCLaunchDelegate2 extends LaunchConfigurationDelega
 			if (cproject != null) {
 				IProject project = cproject.getProject();
 				localMonitor.subTask(DebugCoreMessages.LaunchConfigurationDelegate_6);
-				if (existsProblems(project)) {
+				if (buildFailed || existsProblems(project)) {
 					// There's a build error in the main project
 					
 					// Put up the error dialog. 
@@ -534,7 +540,7 @@ public abstract class AbstractCLaunchDelegate2 extends LaunchConfigurationDelega
 				monitor.done();
 			}
 		}
-	}
+}
 
 	protected ICProject verifyCProject(ILaunchConfiguration config) throws CoreException {
 		String name = CDebugUtils.getProjectName(config);

launch/org.eclipse.cdt.launch/src/org/eclipse/cdt/launch/LaunchUtils.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2004, 2010 QNX Software Systems and others.
+ * Copyright (c) 2004, 2011 QNX Software Systems and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -8,6 +8,7 @@
  * Contributors:
  * QNX Software Systems - Initial API and implementation
  * Alex Collins (Broadcom Corp.) - choose build config automatically
+ * James Blackburn (Broadcom Corp.)
  *******************************************************************************/
 package org.eclipse.cdt.launch;
 
@@ -34,7 +35,9 @@ import org.eclipse.core.resources.IProject;
 import org.eclipse.core.resources.ResourcesPlugin;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IPath;
+import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.Path;
+import org.eclipse.core.runtime.SubProgressMonitor;
 import org.eclipse.core.variables.IStringVariableManager;
 import org.eclipse.core.variables.VariablesPlugin;
 import org.eclipse.debug.core.ILaunchConfiguration;
@@ -47,6 +50,35 @@ import org.eclipse.ui.activities.IWorkbenchActivitySupport;
  */
 public class LaunchUtils {
 
+	/**
+	 * A specialised WrapperProgressMonitor which doesn't let cancellation of the
+	 * child build task cause cancellation of our top-level launch task.
+	 */
+	static class BuildProgressMonitor extends SubProgressMonitor {
+		private boolean cancelled;
+
+		public BuildProgressMonitor(IProgressMonitor monitor, int ticks, int style) {
+			super(monitor, ticks, style);
+		}
+		
+		public BuildProgressMonitor(IProgressMonitor monitor, int ticks) {
+			this(monitor, ticks, 0);
+		}
+
+		@Override
+		public void setCanceled(boolean b) {
+			// Only cancel this operation, not the top-level launch.
+			cancelled = b;
+		}
+
+		@Override
+		public boolean isCanceled() {
+			// Canceled if this monitor has been explicitly canceled
+			//  || parent has been canceled.
+			return cancelled || super.isCanceled();
+		}
+	}
+
 	/**
 	 * For given launch configuration returns the program arguments as 
 	 * an array of individual arguments.