native cmds and libs in the module library

[Chris Hegarty]

Thanks Mandy,

You are correct. I was storing absolute paths.

Updated webrev:
   http://cr.openjdk.java.net/~chegar/jigsaw/libbin_webrev.04/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.

-Chris.

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.
>
>
>> http://cr.openjdk.java.net/~chegar/jigsaw/libbin_webrev.03/webrev/
> 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.
>
> SimpleLibrary.java
> 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.
>
> ModuleFile.java
> 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.
>
> Librarian.java
> L55, 151: this comment line can be removed.
>
> hello-native.sh
> 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.
>
> Mandy
>
>>
>> Thanks,
>> -Chris.