Comment 1 John Arthorne 2012-05-22 10:39:26 EDT

I don't understand any of these changes, and they have the potential to impact our existing build system. Adding pom files is low risk, but I don't think we should be making changes like this in RC2.

Comment 2 Paul Webster 2012-05-22 10:44:28 EDT

(In reply to comment #0)
> This is a pull request for reviewing the CBI non-pom modifications in Equinox
> 
> 4c549ca1286392a7b1082aecf782470467512cac
> http://git.eclipse.org/c/cbi/rt.equinox.framework.git/commit/?h=cbinopom&id=4c549ca1286392a7b1082aecf782470467512cac

Certainly this change removes many plugins from the org.eclipse.equinox.executable feature and they have to be there (both for our PDE build and our published p2 repo)

PW

Comment 3 Thomas Watson 2012-05-22 11:52:01 EDT

(In reply to comment #1)
> I don't understand any of these changes, and they have the potential to impact
> our existing build system. Adding pom files is low risk, but I don't think we
> should be making changes like this in RC2.

I agree with john, here are some comments on specifics:

(In reply to comment #0)

> rt.equinox.bundles:

> http://git.eclipse.org/c/cbi/rt.equinox.bundles.git/commit/?h=cbinopom&id=88edf8be975018dd1ac1bffdb5be1ecd36d23813

This may be safe to do, but it raises a concern that CBI does not compile against the osgi minimum execution environment like our current build does.


> http://git.eclipse.org/c/cbi/rt.equinox.bundles.git/commit/?h=cbinopom&id=6e10e477c66298d2c49e2b3a4ebb6541f2b09efd

This seems risky (wrong?) since it appears to remove the executable from our starter kits.

> http://git.eclipse.org/c/cbi/rt.equinox.bundles.git/commit/?h=cbinopom&id=0937adbcd2e8b869932cabf5ca67bfa965608af7

org.eclipse.equinox.resolver and org.eclipse.equinox.resolver.tests are not built and that folder should be deleted from our repository.  It was simply an experiment project.

> 
> 
> rt.equinox.framework:

> http://git.eclipse.org/c/cbi/rt.equinox.framework.git/commit/?h=cbinopom&id=9a1dcab907cbe2ecdf453a60198a8f44510733de

I don't understand why the test bundle needs to change to not have its test code contained in an inner jar.  This should not cause build issues in CBI.

> http://git.eclipse.org/c/cbi/rt.equinox.framework.git/commit/?h=cbinopom&id=a0aba01b89ed9ef1a7f9d3937f01b8bba04269db

This appears to be a new feature project (rt.equinox.framework.site), no idea what it is for or needed by CBI.

> http://git.eclipse.org/c/cbi/rt.equinox.framework.git/commit/?h=cbinopom&id=9f83dc73b75a04017f939bdc3861f2a3bbc93517

This is related to the first commit against rt.equinox.framework to not have the test classes contained in an inner jar.  Again, not sure why it is needed, but it MUST be released with the following if it is to be released:

http://git.eclipse.org/c/cbi/rt.equinox.framework.git/commit/?h=cbinopom&id=9a1dcab907cbe2ecdf453a60198a8f44510733de


> http://git.eclipse.org/c/cbi/rt.equinox.framework.git/commit/?h=cbinopom&id=4c549ca1286392a7b1082aecf782470467512cac

As Paul stated this seems to remove executables that we must have there.

Comment 4 Thanh Ha 2012-05-22 12:08:00 EDT

(In reply to comment #3)
> (In reply to comment #1)
> > http://git.eclipse.org/c/cbi/rt.equinox.bundles.git/commit/?h=cbinopom&id=0937adbcd2e8b869932cabf5ca67bfa965608af7
> 
> org.eclipse.equinox.resolver and org.eclipse.equinox.resolver.tests are not
> built and that folder should be deleted from our repository.  It was simply an
> experiment project.

You're right we actually also removed this from CBI's poms in a previous commit so CBI isn't building these 2 modules either. This commit should be ignored, sorry for any confusion.

Comment 5 Dani Megert 2012-05-23 10:58:20 EDT

We discussed this in the PMC call and decided that no changes except pom.xml addition should made for Juno at this point.

Comment 6 Igor Fedorenko 2012-05-25 12:44:06 EDT

(In reply to comment #3)
> > rt.equinox.bundles:
> 
> > http://git.eclipse.org/c/cbi/rt.equinox.bundles.git/commit/?h=cbinopom&id=88edf8be975018dd1ac1bffdb5be1ecd36d23813
> 
> This may be safe to do, but it raises a concern that CBI does not compile
> against the osgi minimum execution environment like our current build does.
> 
> 

Tycho does support building against specific JRE libraries, but this requires additional build environment configuration, i.e. installing required JREs and registering them with Tycho. This means that CBI build won't work "out of the box" on all platforms and won't work at all on platforms that do not have 1.3 JDK. This is obviously up to the Equinox developers to decide, but my recommendation is to accept this change such that the build works for everyone out of the box. At the same time, configure 1.3 and other additional JREs on build.eclipse.org (or whatever the main CBI build server is going to be) so real incompatibilities with target JREs can be detected there.



> > http://git.eclipse.org/c/cbi/rt.equinox.bundles.git/commit/?h=cbinopom&id=6e10e477c66298d2c49e2b3a4ebb6541f2b09efd
> 
> This seems risky (wrong?) since it appears to remove the executable from our
> starter kits.
> 
> 

I've opened bug 380695 to track this problem.

> > 
> > 
> > rt.equinox.framework:
> 
> > http://git.eclipse.org/c/cbi/rt.equinox.framework.git/commit/?h=cbinopom&id=9a1dcab907cbe2ecdf453a60198a8f44510733de
> 
> I don't understand why the test bundle needs to change to not have its test
> code contained in an inner jar.  This should not cause build issues in CBI.
>
> > http://git.eclipse.org/c/cbi/rt.equinox.framework.git/commit/?h=cbinopom&id=9f83dc73b75a04017f939bdc3861f2a3bbc93517
> 
> This is related to the first commit against rt.equinox.framework to not have
> the test classes contained in an inner jar.  Again, not sure why it is needed,
> but it MUST be released with the following if it is to be released:
> 
> http://git.eclipse.org/c/cbi/rt.equinox.framework.git/commit/?h=cbinopom&id=9a1dcab907cbe2ecdf453a60198a8f44510733de


I've reverted both 9a1dcab and 9f83dc7. These commits were part of original Eclipse Platform tycho build prototype I did last summer and were meant to allow execution of unit tests as part of the build. Since all tests are executed as part of integration tests suite, these commits are not needed (and likely aren't needed for running unit tests as part of Tycho build either).


> > http://git.eclipse.org/c/cbi/rt.equinox.framework.git/commit/?h=cbinopom&id=a0aba01b89ed9ef1a7f9d3937f01b8bba04269db
> 
> This appears to be a new feature project (rt.equinox.framework.site), no idea
> what it is for or needed by CBI.
> 

I reverted this commit. It was part of "modular build" prototype and was left in the code by mistake.

> 
> 
> > http://git.eclipse.org/c/cbi/rt.equinox.framework.git/commit/?h=cbinopom&id=4c549ca1286392a7b1082aecf782470467512cac
> 
> As Paul stated this seems to remove executables that we must have there.

I've opened bug 380701 to track this.

Comment 7 Thomas Watson 2012-09-05 10:39:36 EDT

Moving to SR2 for any additional requests and remaining work.  Can someone please summarize what other items need attention before equinox can be considered CBI ready?

Comment 8 Thanh Ha 2012-09-05 11:20:42 EDT

(In reply to comment #7)
> Moving to SR2 for any additional requests and remaining work.  Can someone
> please summarize what other items need attention before equinox can be
> considered CBI ready?

In case it helps I made a list below of non-pom related commits in the current state of CBI equinox repos that need to be looked at.


rt.equinox.bundles:

Fix compilation errors on OSX
http://git.eclipse.org/c/cbi/rt.equinox.bundles.git/commit/?h=cbinopom&id=88edf8be975018dd1ac1bffdb5be1ecd36d23813



rt.equinox.framework:

Bug 370704 - equinox launchers build
http://git.eclipse.org/c/cbi/rt.equinox.framework.git/commit/?h=cbinopom&id=6ee0c16688d8c4695af37311baa4570f4d408f5d



rt.equinox.p2:

Bug 380035 download pointing to non-existing url
http://git.eclipse.org/c/cbi/rt.equinox.p2.git/commit/?h=JunoSR1_RC1&id=6c7d3a22a13153b09ec4bfe412808e212c50f8cd



Also Bug 380695 still needs to be looked at on the CBI side related to root.* files which are being commented out in the CBI platform build.

Comment 9 Thomas Watson 2013-01-02 11:06:58 EST

Can this bug be closed?  Please attach any additional patches required.

Comment 10 John Arthorne 2013-01-02 15:19:11 EST

Marking fixed. Thanh please open new bugs if there are further patches here.