From 6a815d839a22dddeb6ce30f8009f0e28547ebb15 Mon Sep 17 00:00:00 2001 From: James Blackburn Date: Wed, 2 Feb 2011 15:41:28 +0000 Subject: [PATCH] Bug 336052 - AbstractCLaunchDelegate{2} fails to launch if the Build throws Exception or build cancels the progressmonitor. --- .../cdt/launch/AbstractCLaunchDelegate.java | 78 +++++++++++-------- .../cdt/launch/AbstractCLaunchDelegate2.java | 22 ++++-- .../org/eclipse/cdt/launch/LaunchUtils.java | 34 +++++++- 3 files changed, 94 insertions(+), 40 deletions(-) diff --git a/launch/org.eclipse.cdt.launch/src/org/eclipse/cdt/launch/AbstractCLaunchDelegate.java b/launch/org.eclipse.cdt.launch/src/org/eclipse/cdt/launch/AbstractCLaunchDelegate.java index 0ec4090f35c..6da9cfb5296 100644 --- a/launch/org.eclipse.cdt.launch/src/org/eclipse/cdt/launch/AbstractCLaunchDelegate.java +++ b/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();) { - IProject proj = (IProject)i.next(); - monitor.subTask(LaunchMessages.AbstractCLaunchDelegate_building + proj.getName()); - proj.build(IncrementalProjectBuilder.INCREMENTAL_BUILD, new SubProgressMonitor(monitor, scale)); - } + try { + for (Iterator i = orderedProjects.iterator(); i.hasNext();) { + IProject proj = (IProject)i.next(); + monitor.subTask(LaunchMessages.AbstractCLaunchDelegate_building + proj.getName()); + proj.build(IncrementalProjectBuilder.INCREMENTAL_BUILD, new LaunchUtils.BuildProgressMonitor(monitor, scale)); + } - monitor.subTask(LaunchMessages.AbstractCLaunchDelegate_building + project.getName()); - setBuildConfiguration(configuration, project); - project.build(IncrementalProjectBuilder.INCREMENTAL_BUILD, new SubProgressMonitor(monitor, scale)); + monitor.subTask(LaunchMessages.AbstractCLaunchDelegate_building + project.getName()); + setBuildConfiguration(configuration, project); + 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); - buildMonitor.beginTask(LaunchMessages.AbstractCLaunchDelegate_BuildBeforeLaunch, 10); - buildMonitor.subTask(LaunchMessages.AbstractCLaunchDelegate_PerformingBuild); - if (buildForLaunch(configuration, mode, new SubProgressMonitor(buildMonitor, 7))) { - buildMonitor.subTask(LaunchMessages.AbstractCLaunchDelegate_PerformingIncrementalBuild); - ResourcesPlugin.getWorkspace().build(IncrementalProjectBuilder.INCREMENTAL_BUILD, new SubProgressMonitor(buildMonitor, 3)); - } - else { - buildMonitor.worked(3); /* No incremental build required */ + try { + IProgressMonitor buildMonitor = new LaunchUtils.BuildProgressMonitor(monitor, 10, SubProgressMonitor.PREPEND_MAIN_LABEL_TO_SUBTASK); + buildMonitor.beginTask(LaunchMessages.AbstractCLaunchDelegate_BuildBeforeLaunch, 10); + buildMonitor.subTask(LaunchMessages.AbstractCLaunchDelegate_PerformingBuild); + if (buildForLaunch(configuration, mode, new SubProgressMonitor(buildMonitor, 7))) { + buildMonitor.subTask(LaunchMessages.AbstractCLaunchDelegate_PerformingIncrementalBuild); + ResourcesPlugin.getWorkspace().build(IncrementalProjectBuilder.INCREMENTAL_BUILD, new SubProgressMonitor(buildMonitor, 3)); + } else { + 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();) { - IProject proj = (IProject)i.next(); - monitor.subTask(LaunchMessages.AbstractCLaunchDelegate_searching_for_errors_in + proj.getName()); - monitor.worked(scale); - compileErrorsInProjs = existsErrors(proj); - if (compileErrorsInProjs) { - break; + if (!compileErrorsInProjs) { + for (Iterator i = orderedProjects.iterator(); i.hasNext();) { + IProject proj = (IProject)i.next(); + monitor.subTask(LaunchMessages.AbstractCLaunchDelegate_searching_for_errors_in + proj.getName()); + monitor.worked(scale); + compileErrorsInProjs = existsErrors(proj); + 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(); - for(int i = 0; i < envp.length; i++) { + Properties p = new Properties( ); + for( int i = 0; i < envp.length; i++ ) { int idx = envp[i].indexOf('='); if (idx != -1) { String key = envp[i].substring(0, idx); diff --git a/launch/org.eclipse.cdt.launch/src/org/eclipse/cdt/launch/AbstractCLaunchDelegate2.java b/launch/org.eclipse.cdt.launch/src/org/eclipse/cdt/launch/AbstractCLaunchDelegate2.java index 8e3fe10138c..22f5e014f02 100644 --- a/launch/org.eclipse.cdt.launch/src/org/eclipse/cdt/launch/AbstractCLaunchDelegate2.java +++ b/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); diff --git a/launch/org.eclipse.cdt.launch/src/org/eclipse/cdt/launch/LaunchUtils.java b/launch/org.eclipse.cdt.launch/src/org/eclipse/cdt/launch/LaunchUtils.java index 8569df84fd6..b0bc9664b53 100644 --- a/launch/org.eclipse.cdt.launch/src/org/eclipse/cdt/launch/LaunchUtils.java +++ b/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.