Re: RFR: 8282444: Module finder incorrectly assumes default file system path-separator character [v2]

2022-02-28 Thread Alan Bateman
On Mon, 28 Feb 2022 16:38:27 GMT, Chris Hegarty  wrote:

>> The module finder implementation incorrectly uses the path-separator
>> character from the default file system, when mapping the relative path
>> of an entry in an exploded module to a package name. This causes
>> problems on Windows [*] when using a module finder with a custom file
>> system that has a path-separator character that differs from that of the
>> default file system, e.g. the zip file system (which uses '/',
>> rather than '\' ).
>> 
>> Rather than adding a new test for this, I deciced to just modify an
>> existing one. The existing test exercises the module finder with a
>> custom file system, but does so with modules have trivial single-level
>> packages names. I just expanded the module to have a subpackage. This
>> is sufficient to reproduce the bug, and verify the fix.
>> 
>> [*]
>> 
>> java.lang.module.FindException: Error reading module: /m2
>> at 
>> java.base/jdk.internal.module.ModulePath.readModule(ModulePath.java:350)
>> at 
>> java.base/jdk.internal.module.ModulePath.scanDirectory(ModulePath.java:284)
>> at java.base/jdk.internal.module.ModulePath.scan(ModulePath.java:232)
>> at 
>> java.base/jdk.internal.module.ModulePath.scanNextEntry(ModulePath.java:190)
>> at 
>> java.base/jdk.internal.module.ModulePath.findAll(ModulePath.java:166)
>> at 
>> ModulesInCustomFileSystem.listAllModules(ModulesInCustomFileSystem.java:108)
>> at 
>> ModulesInCustomFileSystem.testZipFileSystem(ModulesInCustomFileSystem.java:97)
>> at 
>> ModulesInCustomFileSystem.testExplodedModulesInZipFileSystem(ModulesInCustomFileSystem.java:68)
>> at ...
>> Caused by: java.lang.module.InvalidModuleDescriptorException: Package q.r 
>> not found in module
>> at 
>> java.base/jdk.internal.module.ModuleInfo.invalidModuleDescriptor(ModuleInfo.java:1212)
>> at 
>> java.base/jdk.internal.module.ModuleInfo.doRead(ModuleInfo.java:330)
>> at java.base/jdk.internal.module.ModuleInfo.read(ModuleInfo.java:129)
>> at 
>> java.base/jdk.internal.module.ModulePath.readExplodedModule(ModulePath.java:689)
>> at 
>> java.base/jdk.internal.module.ModulePath.readModule(ModulePath.java:320)
>> ... 36 more
>
> Chris Hegarty has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - update copyright year
>  - add tag with OrigBugId 8178380, and bugFixId 8282444

Marked as reviewed by alanb (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/7632


Re: RFR: 8282444: Module finder incorrectly assumes default file system path-separator character [v2]

2022-02-28 Thread Chris Hegarty
> The module finder implementation incorrectly uses the path-separator
> character from the default file system, when mapping the relative path
> of an entry in an exploded module to a package name. This causes
> problems on Windows [*] when using a module finder with a custom file
> system that has a path-separator character that differs from that of the
> default file system, e.g. the zip file system (which uses '/',
> rather than '\' ).
> 
> Rather than adding a new test for this, I deciced to just modify an
> existing one. The existing test exercises the module finder with a
> custom file system, but does so with modules have trivial single-level
> packages names. I just expanded the module to have a subpackage. This
> is sufficient to reproduce the bug, and verify the fix.
> 
> [*]
> 
> java.lang.module.FindException: Error reading module: /m2
> at 
> java.base/jdk.internal.module.ModulePath.readModule(ModulePath.java:350)
> at 
> java.base/jdk.internal.module.ModulePath.scanDirectory(ModulePath.java:284)
> at java.base/jdk.internal.module.ModulePath.scan(ModulePath.java:232)
> at 
> java.base/jdk.internal.module.ModulePath.scanNextEntry(ModulePath.java:190)
> at 
> java.base/jdk.internal.module.ModulePath.findAll(ModulePath.java:166)
> at 
> ModulesInCustomFileSystem.listAllModules(ModulesInCustomFileSystem.java:108)
> at 
> ModulesInCustomFileSystem.testZipFileSystem(ModulesInCustomFileSystem.java:97)
> at 
> ModulesInCustomFileSystem.testExplodedModulesInZipFileSystem(ModulesInCustomFileSystem.java:68)
> at ...
> Caused by: java.lang.module.InvalidModuleDescriptorException: Package q.r not 
> found in module
> at 
> java.base/jdk.internal.module.ModuleInfo.invalidModuleDescriptor(ModuleInfo.java:1212)
> at 
> java.base/jdk.internal.module.ModuleInfo.doRead(ModuleInfo.java:330)
> at java.base/jdk.internal.module.ModuleInfo.read(ModuleInfo.java:129)
> at 
> java.base/jdk.internal.module.ModulePath.readExplodedModule(ModulePath.java:689)
> at 
> java.base/jdk.internal.module.ModulePath.readModule(ModulePath.java:320)
> ... 36 more

Chris Hegarty has updated the pull request incrementally with two additional 
commits since the last revision:

 - update copyright year
 - add tag with OrigBugId 8178380, and bugFixId 8282444

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7632/files
  - new: https://git.openjdk.java.net/jdk/pull/7632/files/ca6cdf4c..220c43c2

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7632&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7632&range=00-01

  Stats: 3 lines in 2 files changed: 1 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7632.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7632/head:pull/7632

PR: https://git.openjdk.java.net/jdk/pull/7632