Chris, I was thinking of something similar. I will create the jars and their contents using Java code. There is no need of creating real class files, using a few resource files and some directories will be enough.
Best, Diego On Wed, Jun 13, 2012 at 6:46 PM, chris hegarty <chris.hega...@oracle.com>wrote: > Diego, > > I'm thinking that some of the trivial source files, to compile and built > into the jars, could be simply created and written by the test itself, > rather than checking them all in. If this makes it cleaner. I really don't > like all the file in test/sun/misc/JarIndex/**metaInfFilenames, but at > least it is quite understandable. > > -Chris. > > > On 13/06/2012 20:36, Diego Belfer wrote: > >> Hi Chris, >> >> That's right. The only non-cleanup change is the one in the merge. >> >> Regarding the test case, I will re-write them in order to generate the >> jars on fly. I'd scanned the jdk/test folder and found a few jars, >> that's why I included them. I have seen your test case, I will use it >> as a sample. >> >> I had not seen your comment in the bug report. Maybe there are other >> cases which trigger the InvalidJarIndexException, but, as far as I could >> see, the validIndex method checks that at least one entry of the jar >> matches the target path found in the index. If directory entries are not >> present in the jar, stripped paths generated during the merge and used >> by the index will return jars which may not contain entries for them, >> triggering the exception. >> When all directory entries are present, if a jar contains an entry for >> "xxx/yyy/resource.file", it will contain entries for "xxx", "xxx/yyy" >> and "xxx/yyy/resource.file". >> >> >> Best, >> Diego >> >> >> On Wed, Jun 13, 2012 at 12:05 PM, Chris Hegarty >> <chris.hega...@oracle.com >> <mailto:chris.hegarty@oracle.**com<chris.hega...@oracle.com>>> >> wrote: >> >> Hi Diego, >> >> Thanks for picking up this bug. >> >> I think your changes look fine. Mainly cleanup except for add -> >> addExplicit/addMapping in merge, right? BTW the cleanup makes this >> more readable. >> >> Unfortunately, the tests you created require checking in a binary >> jar file. This is a real no no for the OpenJDK, we really need to >> create these jars on the fly. I did similar for >> test/sun/misc/JarIndex/__**metaInfFilenames/, but I really wish I >> >> generated the source files for these tests rather than checking in >> so many pointless files. >> >> I can look at helping with writing suitable tests for this. >> >> >> > That's because I was using jars containing "directory entries" >> > (I was unaware that jar files may not include them) >> >> Strangely I added the comment "Remove directories from jar files >> being indexed." to the workaround section of the bug. You seem to be >> seeing the opposite, right? >> >> -Chris. >> >> >> >> On 13/06/2012 06:11, Diego Belfer wrote: >> >> Hi, >> >> I have finally reproduced the InvalidJarIndexException bug as >> reported in >> the ticket. I mentioned in a previous email, that the only way >> I'd found >> for getting the error was to use an invalid index file >> (INDEX.LIST), which >> did not have any sense. That's because I was using jars containing >> "directory entries" (I was unaware that jar files may not >> include them) >> >> After reviewing the URLClasspath$JarLoader class and the >> validIndex method, >> I notice it is possible to get the exception for a Jar file >> which does not >> include directory entries. In order to trigger the issue, the >> Jar must be >> referenced by an intermediary INDEX.LIST and the intermediary >> Jar index >> should have been merged to its parent index. Although, jar tool >> includes >> directory entries in the generated jar files, Eclipse default >> option for >> exporting jars does not include them (AFAIK), so this might be >> quite common. >> >> I have created a new PATCH which includes an additional test >> case which >> uses the URLClassLoader to trigger the InvalidIndexException. >> >> The patch is attached, please consider it for review. >> >> Best, >> Diego Belfer [muralx] >> >> >> >> On Mon, Jun 11, 2012 at 4:47 PM, Diego Belfer<dbel...@gmail.com >> <mailto:dbel...@gmail.com>> wrote: >> >> >> Hi, >> >> Here is a patch that fixes the merge method of the JarIndex. >> This bug was >> reported as the cause of the bug 6901992. Although, I was >> not able to >> reproduce the BUG itself (InvalidJarIndexException), I did >> verified that >> the method had a bug, and resources/classes where not found >> in a jarIndex >> with merged contents. >> >> If you think it is possible to commit this fix without actually >> reproducing the original bug report, please consider this >> patch for review. >> >> Thanks, >> Diego Belfer [muralx] >> >> >> >>
