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

Bug 313789

Summary: [comparator] False Positive comparing with different inner class info ordering
Product: [Eclipse Project] Equinox Reporter: Kim Moir <kim.moir>
Component: p2Assignee: Andrew Niefer <aniefer>
Status: CLOSED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: aniefer, eclipse, Olivier_Thomann, pascal
Version: 3.6Flags: aniefer: review+
Target Milestone: 3.6.1   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
old and new jars
none
Proposed fix none

Description Kim Moir CLA 2010-05-20 14:38:56 EDT 
Difference found for canonical: osgi.bundle,org.eclipse.jdt.compiler.apt,1.0.300.v20100513-0845 between file:/builds/transfer/files/updates/3.6-I-builds/I20100513-1500 and file:/builds/I/org.eclipse.releng.eclipsebuilder/../src/reposource/
			The class org/eclipse/jdt/internal/compiler/apt/dispatch/BatchFilerImpl.class is different.
	packed: osgi.bundle,org.eclipse.jdt.compiler.apt,1.0.300.v20100513-0845
		Difference found for packed: osgi.bundle,org.eclipse.jdt.compiler.apt,1.0.300.v20100513-0845 between file:/builds/transfer/files/updates/3.6-I-builds/I20100513-1500 and file:/builds/I/org.eclipse.releng.eclipsebuilder/../src/reposource/
			The class org/eclipse/jdt/internal/compiler/apt/dispatch/BatchFilerImpl.class is different.


See http://download.eclipse.org/eclipse/downloads/drops/I20100519-1548/buildlogs/comparatorlog.txt
for details
Comment 1 Walter Harley CLA 2010-05-20 15:29:33 EDT 
According to CVS, BatchFilerImpl hasn't changed since 2007.

Kim, can you help me understand what this is about?
Comment 2 Kim Moir CLA 2010-05-20 16:00:58 EDT 
It might be a different compiler or dependancies on another bundle that have made a difference in the class.  I'm not sure.  All that I know is that the comparator is flagging it as a difference.
Comment 3 Olivier Thomann CLA 2010-05-20 16:36:45 EDT 
Kim, can I get access to the two jars used by the comparator? I'd like to find out what could be the differences.
Thanks.
Comment 4 Walter Harley CLA 2010-05-20 16:38:35 EDT 
Olivier, in the comparator log I see that Compiler also is reporting a change. 
Maybe BatchFilerImpl changed because of a change in Compiler?

I agree, we should understand the difference before just tagging to ignore it.  I am concerned about unexplained changes in class files at this stage in the game.
Comment 5 Olivier Thomann CLA 2010-05-20 16:47:24 EDT 
I will find out what the differences are once I get the two jars and report back here.
Once this is understood, we should tag the bundle.
Comment 6 Kim Moir CLA 2010-05-20 16:51:59 EDT 
Created attachment 169420 [details]
old and new jars

I also emailed them to you Olivier
Comment 7 Olivier Thomann CLA 2010-05-20 17:16:51 EDT 
The differences are only the constant pool order.
For some reason the constant pool is not the same between the current jar and the old jar. The compiler doesn't do that kind of changes so this must come from jar file manipulation during the build.
Kim, any idea ?
Comment 8 Olivier Thomann CLA 2010-05-21 11:21:37 EDT 
Ok, using the comparator used inside the builder, the reason why there is a difference is that the inner class infos are in a different order.
I think the fix should be to sort the inner class infos inside the disassembler when comparing the .class file as the order is irrelevant.
Comment 9 Olivier Thomann CLA 2010-05-21 11:25:22 EDT 
Created attachment 169507 [details]
Proposed fix

With this patch, the two jars are now reported as being identical.
Comment 10 Olivier Thomann CLA 2010-05-21 11:26:01 EDT 
I think we should try a test build with a new version of the comparator that would include this patch to see if we can get rid of the remaining comparator issues.
Comment 11 Andrew Niefer CLA 2010-05-21 11:41:05 EDT 
Given that the differences are irrelevant, the only reason to tag is to silence the warning in the releng build.  I'll look at Olivier's change.
Comment 12 Andrew Niefer CLA 2010-05-21 15:02:57 EDT 
Patch looks ok to me.  Marking for 3.6.1
Comment 13 Pascal Rapicault CLA 2010-08-18 19:44:18 EDT 
I've released the attached patch to 3.6.x and HEAD.
Comment 14 DJ Houghton CLA 2010-09-16 12:04:35 EDT 
There was a problem with tagging HEAD so this fix won't appear in integration builds until the first build after 3.7 M2.