native cmds and libs in the module library
mandy.chung at oracle.com
Mon Jan 23 20:16:14 PST 2012
Thanks, Chris. This looks much better.
L141: I would suggest to replace "$$mlib/.." with
"$(MODULE_IMAGES_DIR)/$$image/bin" to make the path explicit and
similarly for "$$mlib/../../bin". Perhaps define a new variable for
the root "$(MODULE_IMAGES_DIR)/$$image" would help.
L191: since the jmod files have been created, do we really need
L228-232: should they be final fields?
L295: extra blank line.
On 1/20/2012 9:56 AM, Chris Hegarty wrote:
> Thanks Mandy,
> You are correct. I was storing absolute paths.
> Updated webrev:
> - Paths to 'jmod create -L lib -natlib <dir> ...' are resolved
> relative to the current working dir, as you would expect.
> - natlib, natcmd, config, paths are stored relative to the module
> library root, in the libraries metadata.
> - The per module 'files' contains paths relative to the modules
> install directory.
> - I made most of the specific code suggestions you said, with the
> exception that natlibs, natcmds, and configs can still be null
> in SimpleLibrary. I use this to differentiate between a passed
> path and a per module path.
> On 01/18/12 07:03 AM, Mandy Chung wrote:
>> On 1/17/2012 6:28 AM, Chris Hegarty wrote:
>>> Here is an updated webrev based on the comments and feedback I received
>>> on this.
>>> - An additional option is added for supporting config files
>> If a relative path is specified as an argument to --natlib, --natbin,
>> or --config option, what is it relative to and what gets recorded
>> in the module library header? I believe the implementation currently
>> uses cwd (i.e. the directory where jmod install is invoked) which
>> means that we can't simply copy the module library and the natlibs
>> to a different location to use?? See my comment for Modules.gmk.
>> L141: --natlib $mlib/.. --natcmd $mlib/../../bin --config $$mlib/..
>> $mlib is an absolute path. Since these alt paths are recorded
>> in the module library header, if we zip up the jdk-module-image and
>> unzip it at a different location, these paths need fixup in order
>> to install new modules in it. It seems that we should keep
>> these paths relative to the system module library when creating
>> the jdk modules image.
>> Similarly, the realpath of the files are stored in "files"
>> which would need fixup too which is not ideal.
>> L346-348: I wonder if it's cleaner to initialize these instance
>> variables to the default path, i.e. new File(root, "bin") if the
>> given argument is null so that the null check is not needed in
>> various place.
>> L301: I think this warning should be output in the Librarian
>> implementation rather than in the SimpleLibrary. Instead
>> it should throw an exception if the natlibs is not writeable
>> and when it writes a native library.
>> L1289: it's a good question when we should check the extension.
>> It depends if the specification requires the extension of a
>> module file be a specific value. I think we can leave as it
>> is and get back to it later.
>> L199-201: may be cleaner if these variables are initialized
>> to the default path e.g. File(destination, "lib") and the
>> non-null check in the getDirOfSection method will not be needed.
>> L581-585: minor: since they are interested in 3 section types,
>> would it be better to handle each section type in this postExtract
>> method rather than checking the section type in both
>> markNativeCodeExecutable and trackFiles methods. Probably the
>> markNativeCodeExecutable method can be merged with this postExtract
>> method and remove L501-504 as well.
>> L55, 151: this comment line can be removed.
>> It might be good to verify if the path of the native library
>> is expected.
>>> Issues I intend to address separately:
>>> - Separate out native libs and config files during the build, so they
>>> can be put into the right sections of the jmod file
>>> - Investigate the potential for global configuration files versus
>>> non-editable per-module config files.
>> That's fine with me.
More information about the jigsaw-dev