> On Sep 13, 2016, at 5:52 PM, Steve Drach <[email protected]> wrote:
>
> The latest web revs. Answers to questions in-line below:
>
> http://cr.openjdk.java.net/~sdrach/8153654/webrev.10/
>
> http://cr.openjdk.java.net/~sdrach/8153654/webrev.jdk/
This looks pretty good. My main comment is in VersionHelper (see below).
ClassFileReader.java
337 String[] msg = {"err.multirelease.option.exists",
f.getName()};
389 String[] msg =
{"err.multirelease.jar.malformed”,
390 jarfile.getName(), rn
391 };
`msg` is not used. These lines can be removed.
line 81-95: this wants to add to VersionHelper if it’s a versioned entry. I
suggest to move this to VersionHelper, something like this and replace the add
method.
public static void addIfVersionedEntry(Path jarfile, String realEntryName,
String className) {
if (realEntryName.startsWith(META_INF_VERSIONS)) {
int len = META_INF_VERSIONS.length();
int n = realEntryName.indexOf('/', len);
if (n > 0) {
String version = realEntryName.substring(len, n);
assert (Integer.parseInt(version) > 8);
String v = versionMap.get(className);
if (v != null && !v.equals(version)) {
// name associated with a different ClassFile
throw new
MultiReleaseException("err.multirelease.version.associated", className,
v, version);
}
versionMap.put(className, version);
} else {
throw new MultiReleaseException("err.multirelease.jar.malformed",
jarfile.toString(), realEntryName);
}
}
}
I prefer to call String::length rather than hardcoding the length. I don’t see
why VersionHelper caches ClassFile. I think it can cache a map from class name
to the version for now. We may have to handle duplicated classes in more than
one modular jar (it’s not exported and should be allowed). We could file a
separate issue to look into later.
This needs a CCC for the new --multi-release option.
Mandy
