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

Bug 339703

Summary:

IBuildRunner should be an abstract base class

Product:

[Tools] CDT

Reporter:

James Blackburn <jamesblackburn+eclipse>

Component:

cdt-build

Assignee:

James Blackburn <jamesblackburn+eclipse>

Status:

RESOLVED FIXED

QA Contact:

Andrew Gvozdev <angvoz.dev>

Severity:

normal

Priority:

CC:

cdtdoug

Version:

8.0

Target Milestone:

8.0

Hardware:

OS:

All

Whiteboard:

Attachments:

Description	Flags
patch 1	jamesblackburn+eclipse: iplog-

Description James Blackburn

2011-03-11 10:04:31 EST

IBuildRunner was added in bug 322458.  Unfortunately as an interface, it's  tough to extend.  For example passing the platform build configurations to an interested builder is not possible without breaking API.

I think this API should be like the platform IncProjBuilder API. Something like:
  

  - InternalBuildRunner  (internal)
    + AbstractBuildRunner  (public API)
       |
       +- Extenders BuildRunner which can use the API


I'd like to do this for CDT8 as this is brand new API and I already need to change it...

Comment 1 James Blackburn

2011-03-11 10:06:13 EST

BTW I'll do this without breaking the existing contract of #invokeBuild so anyone already using this (Doug?) won't have to change code.

Comment 2 Doug Schaefer

2011-03-11 10:13:00 EST

Cool. +1 from me.

Comment 3 James Blackburn

2011-03-11 10:15:47 EST

Created attachment 190990 [details]
patch 1

Simple patch to change interface IBuildRunner to an abstract base class AbstractBuildRunner.

Doug would you be OK with this change?  If we don't do this maitaining API compatibility in the next release will be hard :(

Comment 4 James Blackburn

2011-03-11 10:18:00 EST

(In reply to comment #2)
> Cool. +1 from me.

So to be clear comment 1 is a white lie.  You'll have to change 'implements IBuildRunner' to 'extends AbstractBuildRunner'. Is this OK with you?

We could maintain the interface, but no one should implement it -- as it's new in 8.0 it doesn't seem to make sense to keep it.

Comment 5 James Blackburn

2011-03-11 10:36:38 EST

Fixed in HEAD

Comment 6 CDT Genie

2011-03-11 11:23:43 EST

*** cdt cvs genie on behalf of jblackburn ***
Bug 339703 IBuildRunner should be an abstract base class

[-] IBuildRunner.java  http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/IBuildRunner.java?root=Tools_Project&view=markup
[*] IBuilder.java 1.21 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/IBuilder.java?root=Tools_Project&r1=1.20&r2=1.21
[*] InternalBuildRunner.java 1.2 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/InternalBuildRunner.java?root=Tools_Project&r1=1.1&r2=1.2
[*] ExternalBuildRunner.java 1.6 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/ExternalBuildRunner.java?root=Tools_Project&r1=1.5&r2=1.6
[+] AbstractBuildRunner.java  http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/AbstractBuildRunner.java?root=Tools_Project&revision=1.1&view=markup

[*] buildDefinitions.exsd 1.49 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.managedbuilder.core/schema/buildDefinitions.exsd?root=Tools_Project&r1=1.48&r2=1.49

[*] Builder.java 1.56 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/Builder.java?root=Tools_Project&r1=1.55&r2=1.56