Hi Chris,
Here is a new patch containing new version of the tests. The new
versions generate a the Jar on the fly as we discussed.
Let me know if there is anything else you think should be improved.
Best,
Diego
On Thu, Jun 14, 2012 at 7:32 AM, Chris Hegarty <chris.hega...@oracle.com
<mailto:chris.hega...@oracle.com>> wrote:
Diego,
It's not too difficult to create jars on the fly using the Jar API.
Here is a small example that I think would work nice in this case.
Files created ( and paths are relative to the jtreg scratch, or
working dir if running outside of jtreg ).
Do you think you could use similar to create the jars for your test?
createJar("a.jar", jarAList);
createJar("b.jar", jarBList);
.......
static void createJar(String jarName, Map<String,String> contents)
throws Exception
{
try (FileOutputStream aJar = new FileOutputStream(jarName);
JarOutputStream jos = new JarOutputStream(aJar)) {
Set<Entry<String,String>> entries = contents.entrySet();
for (Entry<String,String> entry : entries)
writeJarEntry(jos, entry.getKey(),
entry.getValue().getBytes("__ASCII"));
}
}
static void writeJarEntry(JarOutputStream jos, String name,
byte[] data)
throws Exception
{
JarEntry entry = new JarEntry(name);
jos.putNextEntry(entry);
jos.write(data);
}
static final Map<String,String> jarAList = new HashMap<>();
static final Map<String,String> jarBList = new HashMap<>();
static {
jarAList.put("com/foo/__resource1.txt", "some random data");
jarAList.put("com/bar/__resource2.txt", "some more random
data!");
jarAList.put("com/baz/__resource3.txt", "even more random
data!!!");
jarBList.put("x/y/resourceA.__dat", "Hello there");
jarBList.put("x/y/resourceB.__dat", "Goodbye");
jarBList.put("x/y/resourceC.__dat", "Hello\nfrom\nb\ndot\njar");
}
Thanks,
-Chris.
On 14/06/2012 03:20, Diego Belfer wrote:
Hi Chris,
There is no way to generate a jar without directory entries
using the
jar tool; there is no option for that. What do you think is the
best way
to handle this ?
I don't want to re-implement the logic for creating a jar using the
JarOutputStream class.
Do you think it is possible to add a new option to the Jar tool Main
class to exclude directory entries? The option does not need to be
exposed by the command line tool to final users if this an issue,
although I think it may be useful for them too.
Best,
Diego
On Wed, Jun 13, 2012 at 7:12 PM, Diego Belfer <dbel...@gmail.com
<mailto:dbel...@gmail.com>
<mailto:dbel...@gmail.com <mailto:dbel...@gmail.com>>> wrote:
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 <mailto:chris.hega...@oracle.com>
<mailto:chris.hegarty@oracle.__com
<mailto: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.hega...@oracle.com>
<mailto:chris.hegarty@oracle.__com
<mailto:chris.hega...@oracle.com>>
<mailto:chris.hegarty@oracle. <mailto:chris.hegarty@oracle.>____com
<mailto:chris.hegarty@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>
<mailto:dbel...@gmail.com <mailto:dbel...@gmail.com>>
<mailto:dbel...@gmail.com <mailto:dbel...@gmail.com>
<mailto: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]
