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.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]
