Community
Participate
Working Groups
In the requestflexer, we check to see if there is a product in the existing install. However this check is only done on the root IUs instead of being done on all the IUs. This means that in situations like the EPP packages where the SDK or Platform products are included, p2 will think that there is no product.
Created attachment 231499 [details] Patch The code fix is in RequestFlexer class. The other changes are to: - add a new test to cater for this use case - enabled disabled product tests - and hook the request flexer test suite to the overall p2 test suite.
(In reply to comment #0) > In the requestflexer, we check to see if there is a product in the existing > install. However this check is only done on the root IUs instead of being > done on all the IUs. > This means that in situations like the EPP packages where the SDK or > Platform products are included, p2 will think that there is no product. I thought the idea was that the EPP Products themselves are 'products' so therefore we didn't need this any lower than the root?
(In reply to comment #2) > (In reply to comment #0) > > In the requestflexer, we check to see if there is a product in the existing > > install. However this check is only done on the root IUs instead of being > > done on all the IUs. > > This means that in situations like the EPP packages where the SDK or > > Platform products are included, p2 will think that there is no product. > > I thought the idea was that the EPP Products themselves are 'products' so > therefore we didn't need this any lower than the root? The EPP are apparently not built as products but they simply include the platform or the SDK as is. This is how they get the executable and a bunch of other files. In the end this is not something that is precluded and is a smart way to go about it. Now from the request flexer point of view, the only thing that he should / could care about is the fact that there is a product in the resulting installation no matter how it gets there. The other thing to note is that before this fix, the logic that checked "if there is already a product installed" is inconsistent with the logic that checks "if there will be a product installed", since the first one checks that there is product installed as root whereas the other one just check that there is an IU in the resulting plan. So all that to say that this fix is good :)
(In reply to comment #2) > I thought the idea was that the EPP Products themselves are 'products'... I have this idea since some years and I thought we could change it this year in EPP. But after EclipseCon I thought that this change is a substantial behaviour change in the EPP packages and could affect many consumers of the packages.
This may be some kind of JUnit configuration difference on my end, but when I run with your patch, the new test suites are not included in the tests. I had to replace this new line from planner.AllTests#suite: suite.addTestSuite(AllRequestFlexerTests.class); With this: suite.addTest(AllRequestFlexerTests.suite()); This is because AllRequestFlexerTests is not actually a test class, it just provides a suite that pulls in a bunch of other test classes. Once I make this change all the new tests run and pass for me (Win 7 64-bit, IBM Java 7).
+1, with the change to the test suite as described in comment #5.
Created attachment 231590 [details] Patch
I'm attaching a new patch. It includes the change mentioned by John but also includes a change that allows p2 remediation to take the obscure lineUp property found on product IU into account when trying to identify if something is a product or not. I've also added a couple new test cases to test this.
The patch looks good. I'm not a fan of adding more code to handle forwards compatibility, but it won't hurt (and it means that people can continue to build with old technology and target newer versions of Eclipse).
(In reply to comment #8) > I'm attaching a new patch. It includes the change mentioned by John but also > includes a change that allows p2 remediation to take the obscure lineUp > property found on product IU into account when trying to identify if > something is a product or not. > I've also added a couple new test cases to test this. So this addresses the EPP case described in bug 409019?
I'd +1 this... but unfortunately I'm not able to run any tests right now.
http://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/?id=8790c9065bb0d12eb6ff26c02b4da43e86a73e2d
Fixed
