Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 327446

Summary:

Launch's wait for build logic does not check for autobuild correctly

Product:

[Eclipse Project] Platform

Reporter:

Martin Oberhuber <mober.at+eclipse>

Component:

Debug

Assignee:

Platform-Debug-Inbox <platform-debug-inbox>

Status:

RESOLVED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

Michael_Rennie, pawel.1.piech, uwe.st

Version:

3.6

Target Milestone:

3.6.2

Hardware:

OS:

Windows XP

Whiteboard:

Attachments:

Description	Flags
Patch fixing the issue for autobuild OFF case	pawel.1.piech: iplog+

Description Martin Oberhuber

2010-10-11 10:01:19 EDT

Build ID: I20100608-0911 (Eclipse 3.6)

CQ: WIND00235406

With the following Preference turned on:

   Run/Debug > Launching > Wait for ongoing build to complete before launching
   == IInternalDebugUIConstants.PREF_WAIT_FOR_BUILD

it can happen that a Launch progress appears in foreground, even though "always launch in background" had been selected for that kind of Launch before. 

This is confusing for our end users and especially annoying, since this can happen even if no single project is in the Workspace and thus it's not expected that any kind of build would ever happen. Even though such a "pseudobuild" is very short, the progress remains visible on top for the entire launch, which may be long-running. It appears that user's Preference is not honored.

The problematic code is in 
   DebugUIPlugin#launchInBackground() line 1160:
   progressService.showInDialog(...)

where the entire job (wait-for-build PLUS launch) is unconditionally shown in foreground, even though background launch had been selected.

Comment 1 Martin Oberhuber

2010-10-12 08:22:04 EDT

I see two possible solutions:

(a) put the entire job into background
    --> In case of an ongoing long-running build, users won't be explicitly 
        notified that their Launch is waiting due to the build (and they cannot
        cancel the Launch). This would be a change in behavior compared to 
        earlier versions of Eclipse.

(b) split the Job into 2 separate Jobs (1 for waiting-for-build, the other
    for acutually performing the Launch). Perform the wait-for-build job with
    progress service like today, but put the launch-job in background. Have
    the launch-job joined to the wait-for-build job.
    --> For short-running builds, no dialog is shown but for long-running ones
        it is shown like today.

I feel like (b) is in line with what end users expect -- no unnecessary notification, but clear notification in case of long-running builds. There might be a small risk that something happens "in-between" the two jobs but I think that this risk is very small and acceptable.

Another option might be putting the entire job into background (like a) but hand-coding a different means of user notification / cancellation for the case that the build runs long.

Comment 2 Martin Oberhuber

2010-11-23 01:11:57 EST

Created attachment 183630 [details]
Patch fixing the issue for autobuild OFF case

Closer investigation shows that the Resources plugin uses the AutoBuildJob not only for building, but any kind of resource change notifications even when autobuilding is off. In fact the AutoBuildJob is responsible for broadcasting PRE_BUILD notifications, which are documented in the IResourceChangeEvent Javadocs as occurring "whenever a build would have occurred" [1].

Therefore, when checking whether a build is currently running, it is incorrect to just check whether a job of FAMILY_AUTO_BUILD is scheduled. Attached patch fixes this by also checking IWorkspace.isAutoBuilding().

This is not a perfect solution, but it should alleviate the problem in our product (which has autobuild turned off by default).

[1] http://help.eclipse.org/helios/topic/org.eclipse.platform.doc.isv/reference/api/org/eclipse/core/resources/IResourceChangeEvent.html

Comment 3 Pawel Piech

2010-11-29 16:11:06 EST

(In reply to comment #2)
> This is not a perfect solution, but it should alleviate the problem in our
> product (which has autobuild turned off by default).
Hi Martin,
Thank you for the patch.  The fix in the patch should be applied just because, as you described, the check for an autobuild is not sufficient.  I committed your fix and renamed this bug to reflect the change.  

If you think that your suggested solution (b) is still needed for correct behavior, please open a new bug for it.

Comment 4 Pawel Piech

2010-11-29 16:12:38 EST

Reviewed and verified behavior.

Comment 5 Martin Oberhuber

2010-11-30 00:29:19 EST

Thanks Pawel. 

I assume target milestone should be 3.7M4. Can you backport to 3.6.2 ?
I can clone a bug for the backporting effort.

Comment 6 Pawel Piech

2010-11-30 00:55:52 EST

(In reply to comment #5)
> Thanks Pawel. 
> 
> I assume target milestone should be 3.7M4. Can you backport to 3.6.2 ?
> I can clone a bug for the backporting effort.

Right, I forgot to ask you whether you wanted this fix in 3.6.2.  It's such a targeted change that I think it's perfectly safe for the maintenance stream.  So as you request, I committed the fix for 3.6.x as well.

Comment 7 Martin Oberhuber

2010-12-01 10:59:58 EST

CQ:WIND00235912 

It looks like DebugUIPlugin v1.309.2.1 did not get tagged, so it is not yet released into today's M20101201-0800. The fix will need to be released into the Mapfile.

Comment 8 Michael Rennie

2010-12-01 11:59:42 EST

(In reply to comment #7)
> CQ:WIND00235912 
> 
> It looks like DebugUIPlugin v1.309.2.1 did not get tagged, so it is not yet
> released into today's M20101201-0800. The fix will need to be released into the
> Mapfile.

I have tagged debug.ui as v20101201_r362