Bug 403589 - In M6 common repo, eclipse.inf is added to some jars when unpacked.
Summary: In M6 common repo, eclipse.inf is added to some jars when unpacked.
Status: RESOLVED FIXED
Alias: None
Product: Community
Classification: Eclipse Foundation
Component: Cross-Project (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: David Williams CLA Friend
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 408213
  Show dependency tree
 
Reported: 2013-03-17 18:57 EDT by Martin Oberhuber CLA Friend
Modified: 2013-05-16 04:40 EDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Oberhuber CLA Friend 2013-03-17 18:57:51 EDT
Noticed when comparing the 4.3m6 Platform SDK against 4.3m6 EPP Packages, the META-INF/eclipse.inf file is missing in 4.3m6 Platform SDK bundles. I tested plugins/org.eclipse.compare.core*.jar on win32.

The file had been there in 4.3m5 so it looks like a regression due to CBI/Tycho based builds to me.

See also https://bugs.eclipse.org/bugs/show_bug.cgi?id=397896#c28
Comment 1 David Williams CLA Friend 2013-03-17 19:05:11 EDT
Yes, the are purposely not published by Tycho/Maven builds. I forget the exact reason why, but CCing Igor who I'm sure will know off the top of his head. 

Are you saying you need them in creating an EPP Package? Or, just that you happened to notice? (I would not think you'd need them, but ... wouldn't be the first time I'd miss the reasons for something).
Comment 2 Martin Oberhuber CLA Friend 2013-03-17 19:22:03 EDT
I'm not sure if the eclipse.inf files are strictly needed ; but I think it is bad if the plugin.jar files binary differ between getting something from an SDK Download or an EPP Download or a Kepler Install. So if the policy is changed here, the change should be coordinated with the Release Train IMO.

I had thought that the eclipse.inf files capture the fact that bundles are already pack200 conditioned, so downstream re-packagers won't try to condition them again; and, for problematic bundles which can't be pack200'd for some reason like nested jars, they capture the instruction to not pack200.

So, I personally find them useful and if there's a reason for no longer shipping them I'd like to know why.
Comment 3 Igor Fedorenko CLA Friend 2013-03-17 21:25:30 EDT
Bug 387557 describes the root cause, but in short, exact format of eclipse.inf files is not documented, p2 jarprocessor unnecessarily regenerates eclipse.inf at install time, which results in InvalidContentException. There was a discussion about possible ways to mitigate the problem on cbi-dev list or one of cbi bugzillas, but I don't have links handy.

Note that the problem only affects signed and packed bundles and I believe eclipse.inf still can be/is used to disable signing and/or packing of problematic bundles.
Comment 4 David Williams CLA Friend 2013-03-19 21:19:30 EDT
This might turn out to be a non-issue (or, side issue) but I think Martin's point about the bundle being "self documented" on what kind of processing it has had can be important "downstream". I was thinking this might even be important to our CBI builds, and was hoping, Igor, your expertise of "how it works" might provide some insight ... rather then me just using "trial and error". 

As one approach to fixing bug 403805 (also discussed some in bug 402708) I was thinking of turning "off" pack.gz process for "the main" build. And then at the very end, for the part we want to deploy to the web, use the p2.process.artifacts task, with "pack=true"). I believe this will also work around the lack of MD5 checksums, until that's fixed in Tycho (bug 357513). 

A side advantage of this approach might be that "the main build" would be faster, not having to do the final pack200 processing (which "local builds" would profit from) but during production builds where the pack.gz files are more important, the extra processing is at the end. [But, the pack processing would still occur once, since that's required to do before signing]. 

But, at a minimum, I think this "post processing" would give different results than doing it during the main build. (e.g. I think Tycho does not pack200 features and other non-java jars, which is good), but at worse, it might give different/broken results, such as if something gets "packed twice" or conditioned when it should not be. I think since some of the eclipse.inf files are left, and only "pure defaults" case are not left things might work ok, but, hard for me to intuit.  

Any thoughts?
Comment 5 Igor Fedorenko CLA Friend 2013-03-19 22:42:44 EDT
I am not sure what you are asking exactly. There is some information that explains how to configure pack200 and signing in pom.xml in [1]. It is possible to do pack200 normalization and signing and not do packing. Tycho kinda respects eclipse.inf jarprocessor.excludes and pack200.normalized directives, but Tycho does not support eclipse.inf files that have both packing and signing enabled. The latter means you can't capture 'pack200.normalized=true' as part of tycho build and use this as part of final pack200 processing. 

[1] http://wiki.eclipse.org/Tycho/Pack200
Comment 6 David Williams CLA Friend 2013-03-23 12:54:45 EDT
(In reply to comment #5)
> I am not sure what you are asking exactly. There is some information that
> explains how to configure pack200 and signing in pom.xml in [1]. It is
> possible to do pack200 normalization and signing and not do packing. Tycho
> kinda respects eclipse.inf jarprocessor.excludes and pack200.normalized
> directives, but Tycho does not support eclipse.inf files that have both
> packing and signing enabled. The latter means you can't capture
> 'pack200.normalized=true' as part of tycho build and use this as part of
> final pack200 processing. 
> 
> [1] http://wiki.eclipse.org/Tycho/Pack200

Yes, lack of pack200.normalized=true was/is my concern. Turns out we can fix our "pack final repo but not others" problem (bug 403805) without worrying about it it, but still concerned about others using different technologies. In a sense, this has become a "defacto standard" since done for 10 years ... and to be perfectly correct, those using other technologies would have to figure out some other way to know which have already been conditioned, and which haven't, so they will know to exclude them explicitly, such as if using one large zip file with jar processor, they can be excluded by putting in "pack.properties" file .... but, that isn't always easy. 

So, still a concern, but not sure of severity of impact.
Comment 7 Igor Fedorenko CLA Friend 2013-03-23 14:54:43 EDT
If we change tycho to include eclipse.inf in packed and signed bundles this will make these bundles incopmatible with Juno and earlier versions of p2. I do not believe this is an option.
Comment 8 David Williams CLA Friend 2013-03-24 01:49:13 EDT
(In reply to comment #7)
> If we change tycho to include eclipse.inf in packed and signed bundles this
> will make these bundles incopmatible with Juno and earlier versions of p2. I
> do not believe this is an option.

That is a good point. Mind explaining a little more? 

Is it just a matter of you getting a specification of the exact form to write so that's it compatible with those older versions? (that is, even if mistakenly re-written). If that's all it is, then I suspect we can figure that out (and if not, we can figure out who to ask).
Comment 9 Igor Fedorenko CLA Friend 2013-03-24 10:41:52 EDT
I don't think it is a good idea to workaround this p2 bug in tycho, now I think it is actually feasible

The root cause of the problem is p2 that compares eclipse.inf generated at install time with eclipse.inf signature calculated during the build. I don't think we can just agree on specification of eclipse.inf format here, we actually have to match eclipse.inf produced by all existing p2 versions byte-to-byte, which in practice means copy&paste p2 eclipse.inf generation logic to Tycho and having to maintain this code in Tycho from now on. This assumes that eclipse.inf generation code stayed the same in p2 in the past and will stay the same in the future. I believe this puts unreasonable burden on Tycho developers.

There is also more fundamental problem with pack200 of signed jars. JDK documentation is quite clear that both pack200 normalization and packing have to use exactly the same configuration for this to work. There is also no guarantee that different versions of JDK will produce exactly the same pack200 files (actually, I think exactly the opposite stated somewhere in the docs but I don't have url handy). I don't see how to make this work reliably if you want to one set of tools to normalize and to pack artifacts.

All in all, I am rather strongly against polluting Tycho code with a workaround for this p2 bug. I think it is okay to introduce new marker file (.pack200normalized for example), this is trivial to do in Tycho, but like I said above, I am not sure there is way to use this reliably.
Comment 10 David Williams CLA Friend 2013-03-26 11:03:59 EDT
(In reply to comment #9)

Just a few comments to clarify my comments. I wasn't implying that the fix/workaround would have to be done forever. I believe the p2 bug has now been fixed in Kepler (John said "he thought he did, he'd check" ... so, you might remind him if you see him :) So, the suggestion is, leave in place for a release or two, and at some point it's quite reasonable to say something like "Luna +1 bundles" can not necessarily be installed into Kepler, or Juno due to workaround for bug xyz being removed" (what ever limit seems reasonable). 

Second clarification, I think "tool compatibility" is important to the Eclipse community as a whole ... they don't care where or how the bug originated. Its simply the curse of providing "core components" and several teams have had to live with that extra burden though they'd wish they didn't have to. 

Last clarification, I'm still unclear on how severe the current problem is. It might not be that bad ... or, might be fatal in some cases. I just don't know, and not sure I know enough how to tell. 

And, one additional point. If there is a change (even a change of advice, based on our own "improved understanding") that requires downstream projects to work around something, then that should be clearly documented and mentioned in "migration guide". For example, an outline of such a write-up in this case might be similar to the following: 

"As has always been the case, if a bundle has already been conditioned with pack200, it should not be re-conditioned. Conditioned or not used to be encoded in the eclipse.inf file but now no longer is, so projects must use "external" means to limit what's conditioned, so as to exclude bundles already conditioned". And maybe mention something about not doing pack200 on conditioned jar, without using exact same VM and exact same parameters as used in the initial conditioning. 

Something to that effect. Filled in and made accurate by those of you who understand it well. 

Hope these comments read as constructive as intended.
Comment 11 David Williams CLA Friend 2013-03-28 12:15:06 EDT
Adding Markus and Thomas to CC list in case they'd like to comment. 

Are any problems anticipated using bundles without eclipse.inf file in Buckminster builds? Can EPP avoid "re-processing" bundles that don't have it (if it doesn't already)?
Comment 12 Markus Knauer CLA Friend 2013-04-05 09:44:10 EDT
EPP doesn't condition/pack200/sign any bundles, it just consumes what's already there with the p2 director. Therefore I don't see any problems here.
Comment 13 David Williams CLA Friend 2013-04-05 12:26:11 EDT
I'm going to close this as "won't fix" since, so far, no one has come up with a concrete problem that it causes. 

Feel free to re-open if someone does know of something, or if I am mis-reading something.
Comment 14 Martin Oberhuber CLA Friend 2013-04-05 12:53:06 EDT
So you think it is acceptable that the identical bundle obtained from the 
original Eclipse SDK or platform is binary different than the same bundle
obtained from an EPP package or from the Simrel site ?
Comment 15 David Williams CLA Friend 2013-04-05 12:57:51 EDT
(In reply to comment #14)
> So you think it is acceptable that the identical bundle obtained from the 
> original Eclipse SDK or platform is binary different than the same bundle
> obtained from an EPP package or from the Simrel site ?

I'm not sure why it'd be different? If Markus says he doesn't "reprocess" them, then ... who is?
Comment 16 Martin Oberhuber CLA Friend 2013-04-05 13:01:58 EDT
I don't know, but as I have mentioned in comment 0 they _are_ different when they download them (well I didn't test today but they _were_ different when I last tried).
Comment 17 David Williams CLA Friend 2013-04-05 13:35:02 EDT
(In reply to comment #16)
> I don't know, but as I have mentioned in comment 0 they _are_ different when
> they download them (well I didn't test today but they _were_ different when
> I last tried).

To answer your question more directly, Yes, it is a problem if they are different. Either EPP is (somehow) reconditioning them (perhaps Buckminster does it "under the covers" -- I will remind Markus he at least signs the "EPP specific product bundles ... at least, I think so) or someone _else_ is "re-contributing" them to common repo? But, I don't think this is an Eclipse Releng bug. We are doing what we can and should ... either Tycho needs to fix/change their policy ... or, those that are re-processing platform bundles needs to be tracked down and told to stop doing that. :) 

But again, I don't mean to "cut off" this important topic ... just not sure the discussion belongs where in Platform Releng ... unless ... you are suggesting we go back to PDE based builds? :)
Comment 18 Martin Oberhuber CLA Friend 2013-04-05 13:50:38 EDT
So who's the next actor ?

Bug against EPP that the Platform bundles in EPP _do_ contain the eclipse.inf while the original Eclipse project ones do not ?
Comment 19 Markus Knauer CLA Friend 2013-04-05 14:19:55 EDT
.../downloads/releases/kepler/201303220900/plugins/org.eclipse.compare_3.5.400.v20130228-0954.jar contains META-INF/eclipse.inf 

The one from the eclipse-SDK-4.3M6-win32.zip and eclipse/updates/4.3milestones/S-4.3M6-201303141330/plugins/org.eclipse.compare_3.5.400.v20130228-0954.jar do not contain any eclipse.inf

EPP just takes what's available in /releases/kepler (or better: /releases/staging). Now we need to find out where we got this bundle from. Unfortunately I don't see the 'source' in the b3 aggregator run:

     [exec] - mirroring artifact osgi.bundle,org.eclipse.compare,3.5.400.v20130228-0954
     [exec]     doing copy of optimized artifact
     [exec]     unpacking optimized artifact


(Just in case someone starts to suspect RAP (because we are re-distributing a binary identical copy of some platform bundles...) I checked this and there's no org.eclipse.compare bundle that we are sharing with our users.)
Comment 20 David Williams CLA Friend 2013-04-05 14:50:16 EDT
(In reply to comment #19)
> .../downloads/releases/kepler/201303220900/plugins/org.eclipse.compare_3.5.
> 400.v20130228-0954.jar contains META-INF/eclipse.inf 
> 
> The one from the eclipse-SDK-4.3M6-win32.zip and
> eclipse/updates/4.3milestones/S-4.3M6-201303141330/plugins/org.eclipse.
> compare_3.5.400.v20130228-0954.jar do not contain any eclipse.inf
> 

Was this literally the only bundle that was different? Or just an example? 

I guess "cross-project" bugzilla would be a good place to get the right visibility, that project's have to use care not to "reprocess" bundles that have already been processed, and that you can no longer count on the file having "conditioned" flagged in the eclipse.inf.
Comment 21 David Williams CLA Friend 2013-04-05 14:58:06 EDT
(In reply to comment #19)
> .../downloads/releases/kepler/201303220900/plugins/org.eclipse.compare_3.5.
> 400.v20130228-0954.jar contains META-INF/eclipse.inf 
> 
> The one from the eclipse-SDK-4.3M6-win32.zip and
> eclipse/updates/4.3milestones/S-4.3M6-201303141330/plugins/org.eclipse.
> compare_3.5.400.v20130228-0954.jar do not contain any eclipse.inf
> 

Also, to be perfectly accurate, the one to check is 
... compare_3.5.400.v20130228-0954.jar.pack.gz 

If THAT produces a different jar, then ... something is weird! (And, wrong).
Comment 22 David Williams CLA Friend 2013-04-05 15:20:35 EDT
(In reply to comment #21)
> (In reply to comment #19)
> > .../downloads/releases/kepler/201303220900/plugins/org.eclipse.compare_3.5.
> > 400.v20130228-0954.jar contains META-INF/eclipse.inf 
> > 
> > The one from the eclipse-SDK-4.3M6-win32.zip and
> > eclipse/updates/4.3milestones/S-4.3M6-201303141330/plugins/org.eclipse.
> > compare_3.5.400.v20130228-0954.jar do not contain any eclipse.inf
> > 
> 
> Also, to be perfectly accurate, the one to check is 
> ... compare_3.5.400.v20130228-0954.jar.pack.gz 
> 
> If THAT produces a different jar, then ... something is weird! (And, wrong).

I did check /home/data/httpd/download.eclipse.org/eclipse/updates/4.3milestones/S-4.3M6-201303141330/plugins/compare_3.5.400.v20130228-0954.jar.pack.gz

using unpack200, and the resulting jar did NOT contain eclipse.inf ... so, the mystery deepens.
Comment 23 David Williams CLA Friend 2013-04-05 15:44:46 EDT
(In reply to comment #22)
> (In reply to comment #21)
> > (In reply to comment #19)
> > > .../downloads/releases/kepler/201303220900/plugins/org.eclipse.compare_3.5.
> > > 400.v20130228-0954.jar contains META-INF/eclipse.inf 
> > > 
> > > The one from the eclipse-SDK-4.3M6-win32.zip and
> > > eclipse/updates/4.3milestones/S-4.3M6-201303141330/plugins/org.eclipse.
> > > compare_3.5.400.v20130228-0954.jar do not contain any eclipse.inf
> > > 
> > 
> > Also, to be perfectly accurate, the one to check is 
> > ... compare_3.5.400.v20130228-0954.jar.pack.gz 
> > 
> > If THAT produces a different jar, then ... something is weird! (And, wrong).
> 
> I did check
> /home/data/httpd/download.eclipse.org/eclipse/updates/4.3milestones/S-4.3M6-
> 201303141330/plugins/compare_3.5.400.v20130228-0954.jar.pack.gz
> 
> using unpack200, and the resulting jar did NOT contain eclipse.inf ... so,
> the mystery deepens.

Using the b3 aggregator editor to "browse" contributions (and its "find" function) there is an older version of org.eclipse.compare "contributed" by the koneki project, but it is an older version, 3.5.300.v20120522-1148, and I do not think it is being "pulled in" in any way, since its not listed in the "multiple versions" report, at http://build.eclipse.org/simrel/kepler/reporeports/reports/nonUniqueVersions.txt so I don't think that is in anyway related. That was the only other "org.eclipse.compare" I saw via aggregator.
Comment 24 David Williams CLA Friend 2013-04-05 15:59:03 EDT
(In reply to comment #23)
> (In reply to comment #22)
> > (In reply to comment #21)
> > > (In reply to comment #19)

Next, I checked the jar file on kepler site, and confirmed what Markus said. 

 .../downloads/releases/kepler/201303220900/plugins/org.eclipse.compare_3.5.400.v20130228-0954.jar contains META-INF/eclipse.inf 

BUT, I checked the pack.gz version on kepler site and it does NOT contain an eclipse.inf. 

Thomas, any ideas how the aggregator can get a pack.gz file without eclipse.inf, but end up with one in the jar file? I would assume the aggregator just calls unpack200 ... but ... is there something else it does? Any other ideas how to debug this?
Comment 25 Thomas Hallgren CLA Friend 2013-04-05 17:34:39 EDT
AFAIK, the b3 aggregator doesn't even call unpack/pack. It relies solely on underlying p2 functionality.
Comment 26 David Williams CLA Friend 2013-04-05 18:40:55 EDT
(In reply to comment #25)
> AFAIK, the b3 aggregator doesn't even call unpack/pack. It relies solely on
> underlying p2 functionality.

Thanks Thomas, I think we are narrowing this down. We use 4.2.2 to "run the aggregator" for Kepler, so ... assuming the "4.2" aggregator can run on Kepler M6, I can try that, and see if the p2 fix for bug 387557 fixes this issue. 

But, reading bug 387557 I'm not sure the eclipse.inf is NEVER written during unpack, or if it was only for cases "when not needed". Adding Ian to CC in case he knows off the top of his head. If p2 unpack does still sometimes add it, then, it's back to square one as an issue for Tycho and p2 teams to resolve. 

But, I'll try kepler M6 first, unless, Thomas, you know of a reason the aggregator won't work with Kepler versions.
Comment 27 David Williams CLA Friend 2013-04-06 12:13:55 EDT
I was not able to use M6 and the b3 aggregator (see bug 405057) but was able to use M5 with the current b3 aggregator, and that does indeed fix the issue. 

At least, for the "o.e.compare" bundle ... I did not check all bundles (but if anyone has a way to do that, feel free). I promoted the build to "staging" so next EPP packages produced should be a better match (which is how Martin found the bug, to begin with). 

Compliments to Thomas and his use of p2 API so we get "standard behavior" automatically. 

I plan to continue to use Kepler M5 ... depending on the outcome of bug 405057. If b3 aggregator is upgraded to "Kepler Stream", I will "move up" as feasible.