RFR: 8060130: Simplify the synchronization of defining and getting java.lang.Package
mandy.chung at oracle.com
Wed Oct 22 21:35:32 UTC 2014
On 10/21/2014 6:43 AM, Claes Redestad wrote:
A minor nit: Package.java line 636: it can return EMPTY_MANIFEST that
will set manifest to non-null so that an invalid file would avoid
opening the file every time getManifest is called (although this case
rarely happens). line 639 can then be simplified.
This is great. Thanks for adding the test.
Can you break the long lines such as the call to JarBuilder.addClassFile
and runSubprocess. Have you run this on windows? I think you need to
use File.separator rather than "/" in the -Xbootclasspath/p argument.
This test uses a special class loader that delegates to null class
loader only. Since you have the verification in place, it'd be good to
also call Package.getPackage and Package.getPackages to verify that the
same "package2" instance is returned. As a sanity check, you could
check "java.lang" package must be present.
In the verifyPackage, it throws Exception. You can simply throw
RuntimeException that would avoid the need of the checked exception.
Nit: line 35-37, 43-46 - no need to declare these import as java.lang.*
are imported by default.
Copyright year should be 2014.
> I'd prefer sticking to the double-checked idiom here, unless you insist.
> I've cleaned up the code to avoid assignment in if-clauses, which
> according to
> anonymous sources makes the code more readable. Perhaps this addresses
> some of your concerns?
Looks okay. Initialize manifest to EMPTY_MANIFEST would clean that a
> Since I don't want to add binary JAR files, I opted to add a test
> which creates two JAR files,
> each with a single class (with/without manifest) and then proceeds to
> spawn processes
> to verify that:
Thanks for adding the test.
> - when the JAR with manifest is on bootclasspath, the package can be
> found via
> Package.getSystemPackage and the package object reflects values added
> to the manifest
> - when the JAR without manifest is on bootclaspath, the package can be
> found via
> Package.getSystemPackage but is empty apart from name
> - adding the test.classes directory to bootclasspath behaves the same
> as adding the JAR
> without manifest
> - for any case where the class/package is not on the bootclasspath,
> the package information
> can not be found via Package.getSystemPackage().
> Does this cover everything?
This is great. See my comment above.
> I guess there might be a way to make @run main/othervm or
> main/bootclasspath pick
> up a dynamically generated JAR file, but I've failed to find a way
> that would make this work without
> pregenerating the JARs. Suggestions on how this can be simplified are
What you have is fine. The other way is to do it in a shell test that
is not preferable as you would have to manage FS for different OSes etc.
More information about the core-libs-dev