[modules-dev] [Fwd: [Fwd: Review request: 6563535, URLRepository.install]]

[Stanley M. Ho]

Hi Dave,

Looks good. Several comments:

- src/share/classes/sun/module/repository/URLRepository.java

563         if (!(srcJamFileName.endsWith(".jam")  || 
srcJamFileName.endsWith(".jam.pack.gz"))) {

Should also support .jar.

  630         // Refuse to install if comparable module is already 
installed.
  631         // XXX Use platform, arch when available from metadata.
  632         List<ModuleDefinition> defns = findModuleDefinitions(
  633             Query.and(Query.name(moduleName), 
Query.version(moduleVersion)));

findModuleDefinitions() will not return platform specific module 
definitions, and this logic will reject installation of platform 
specific modules that do not match the running platform. What you want 
to check is probably the list of ModuleArchiveInfo instead.

657         FileOutputStream fos = new FileOutputStream(destMDFile);

Use BufferedOutputStream.

688             contents.put(mai, installedMD);

If exception is thrown in the try/catch block, the contents will be 
updated incorrectly. Even contents is updated again in the catch block, 
there is still a small window that contents does not represent the 
correct information. Instead, this should be updated in a finally block.

  717         ModuleDefinition md = contents.get(mai);
  718         if (md == null) {
  719             rc = false;

If the module definition is platform specific but does not match the 
current platform, the above logic won't allow it to be removed from the 
repository.

748             System.err.println("### jamFile: " + 
jamFile.getCanonicalPath());

Remove.

  752              * them.  If any rename or deletion fails, undo 
whatever can be
  753              * undone. */

If writeRepositoryMetadata in 773 throws exception, it is unclear if the 
unwinding will be done properly.

  805     private void writeRepositoryMetadata(ModuleArchiveInfo mai) 
throws IOException {

It never uses specified mai when writing the metadata.

- Stanley