Bug 430339 - Should restrict "feature patches" to apply to only intended version of patches.
Summary: Should restrict "feature patches" to apply to only intended version of patches.
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Releng (show other bugs)
Version: 4.4   Edit
Hardware: PC Linux
: P2 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Platform-Releng-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-14 01:05 EDT by David Williams CLA
Modified: 2016-08-11 15:26 EDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Williams CLA 2014-03-14 01:05:01 EDT
Based on comments in but 380190 (starting around comment 58) I decided to try and install "the patch" on to the JEE EPP package from Luna M6. The PDE patch was not installed (as implied by discussion in bug 380190) but the JDT patch was. 

BUT, it did this by essentially "down leveling" some bundles. For example, when freshly installed, JEE package had jdt.core bundle of 3.10.0.v20140303-0628 but after the patch installed, the jdt.core was at version 3.9.2.v20140313-0432. 

So, 1) that's not right (*),  and 2) is partially a fluke, I suspect, of the way we still have "old stuff" in some of our repos?, and 3) I think the Tycho process of producing a patch (at least its "metadata") is different from the way PDE used to do it. 

* The reason I say "it's not right" is that 1) in theory, people could lose function or bug fixes, and 2) it would be sort of a maintenance nightmare, when people report bugs, it'd be hard to know exactly what they were running or running with.
Comment 1 David Williams CLA 2014-03-14 01:25:02 EDT
So in looking for a solution, I started by looking at the content.jar/xml file for a recent "patch build" and for the jdt patch, I see the following: 

Peeking at the "content.jar/xml" file, I see for the patch feature, it says: 

<unit id='org.eclipse.jdt.java8patch.feature.group' version='1.0.0.v20140313-1842' singleton='false'>
  <patchScope>
    <scope>
      <requires size='1'>
        <required namespace='org.eclipse.equinox.p2.iu' name='org.eclipse.jdt.feature.group' range='0.0.0'/>
      </requires>
    </scope>
  </patchScope>

If I'm reading this right ... it places no restrictions at all, at the feature level ... similar for bundle level, I believe. 

I'm guessing PDE can't be installed because some prereq is completely missing from repositories ... but not sure? (I did my test with all "repositories enabled"). 

In our jdt patch feature.xml, we do say 

  <requires>
      <import feature="org.eclipse.jdt" version="3.9.2.v20140212-0800" patch="true"/>
   </requires>

PDE used to translate that in p2 metadata, as 
"this (patch) feature requires exactly org.eclpse.jdt feature versioned 3.9.2.v20140212-0800

But it is version org.eclipse.jdt_3.10.0.v20140306-1200 that in Luna M6. 

So, I'm thinking it might be worth trying to write our feature.xml as 

  <requires>
      <import feature="org.eclipse.jdt" version="[3.9.2.v20140212-0800]" patch="true"/>
   </requires>

But I have a feeling Tycho may ignore that as well. I suspect root problem is bug 389698 where Tycho does not use "the most recent" ways of producing repositories for feature patches, but some old fashioned ""publish and convert update site" method, or something. 

So, if specifying exact range has no effect as expected, we may have to include some custom p2.inf files? 

Or ... if that doesn't help, run it through some "post process" (which, would probably take longer than "a day or two" to figure out). 

We can (and should anyway) add some words in description etc., that the patch is for Eclipse 4.3.2 (Kepler SR2) distributions.
Comment 2 David Williams CLA 2014-03-14 02:38:20 EDT
First attempt: 

http://git.eclipse.org/c/platform/eclipse.platform.releng.aggregator.git/commit/?h=R4_3_maintenance_Java8&id=5d311d3d3e06e5b67e5bec0f7d402e72e4b8165c

I noticed too that our versions were slightly wrong ... perhaps because we ended up doing that "RC4a" build in the last week of Kepler SR2. In an ideal world, maybe Tycho was "just being nice" and thinking since it didn't find an exact match, it just filled in "0.0.0"? Guess I could check some older Patch repos?
Comment 3 Paul Webster CLA 2014-03-14 06:00:01 EDT
Are you already using a p2.inf to broaden the range of the feature patch?

PW
Comment 4 David Williams CLA 2014-03-14 08:13:38 EDT
(In reply to Paul Webster from comment #3)
> Are you already using a p2.inf to broaden the range of the feature patch?
> 
> PW

No.
Comment 5 David Williams CLA 2014-03-14 17:12:25 EDT
Just to give status, the brackets didn't help. Tycho reports "invalid syntax", which it probably is. And, using the correct, exact values didn't change anything. 

Not sure there's time to try all three of these experiments, but, I think options are: 

1. use p2.inf files to restrict. Not optimistic, since that's the current problem (bug 389698) ... creating repositories with Tycho must use old "update site" methods with the "update site converter" which doesn't know much about p2 metadata. 

2. experiment with b3 aggregator, and see if its any smarter about "converting" an old style update site. (unlikely). 

3. Another option, once mentioned as a work around for bug 389698, is to do a second run, basically telling Tycho to "create a product" which would then create a proper p2 repository. At the time, I thought this meant to create a "whole product" (such as a 4.3.3 product, with feature patches applied) but, there might be a way to do it to create a "dummy product" ... just to get the repository for just the feature patch (and not actually materialize any products) -- just a re-write of metadata, basically. 

4. Plus, I'm going to use PDE Build from my workbench and better understand what p2 meta data "should" be being created ... while I hate the idea of "editing metadata files", maybe this is a case where its necessary and not that much of an edit? 

We'll see. Not optimistic. We may just have to rely on good names and descriptions. and let "user beware". 

[Side note: I thought of another reason to add in comment 0 about dangers of applying patch which "downgrades" some bundles ... it may or may not apply in this case ... but, I suspect doing things like that it'd be easy to get into a state where the next "regular update" (such as from M6 to M7) might not work correctly if things were not completely "well formed". It'd be a bug for p2 to let you get into that state ... but, I'm sure it is not a well tested scenario.]
Comment 6 David Williams CLA 2014-03-17 16:43:39 EDT
untargetting, as obviously won't be done for the patch being built now (for GA tomorrow). 

The only "new" thing I learned, was interesting but not very helpful. 

I created a "feature patch" with PDE, directly from workbench (using Kepler SR2). 
The content.jar/xml file did seem to have some differences from the one produced by Tycho, but was hard to see what was significant ... since so much "changes order", etc., it is hard to compare. 

But, I did try applying that to Luna M6 JEE package, and the interesting thing was, was that it did install ... BUT ... it did not "take effect". That is, while both levels of bundles were in the /plugins directory, when I started up JEE, it only showed the most recent ones, as being the "active", in use ones, unlike after I installed they Tycho built patch. So ... something was different ... or there's some "random" aspect to p2 ... and we all know that's not the case :) 

I am going to leave this bug open, and change to enhancement, and in my free time, would like to experiment with how-to accomplish the 'lock down' that feature patches should have. But, if we get lots of complaints from community, it'd be fair to move back to bug.
Comment 7 David Williams CLA 2014-03-17 17:16:49 EDT
Changed wording to apply to the general case, no longer "BETA_JAVA8" and Kepler SR2.
Comment 8 David Williams CLA 2016-08-11 15:26:47 EDT
Doing a mass "reset to default assignee" of 52 bugs to help make clear it will (very likely) not be me working on things I had previously planned to work on. I hope this will help prevent the bugs from "getting lost" in other people's queries. Feel free to "take" a bug if appropriate.