Jon, I pushed the changeset with separated source directories for modules and packages, which worked fine.
Here are the changes in the test: http://hg.openjdk.java.net/jdk/jdk/rev/3b557aef43c4#l3.13 <http://hg.openjdk.java.net/jdk/jdk/rev/3b557aef43c4#l3.13> Hannes > Am 08.04.2020 um 11:42 schrieb Hannes Wallnoefer > <hannes.wallnoe...@oracle.com>: > > Thanks for the review, Jon. > > I agree the test is weird. I had to check it does the right thing with > everything in the same directory. > > I’ll rewrite it to use separate directories as you suggest. > > Hannes > >> Am 07.04.2020 um 19:32 schrieb Jonathan Gibbons <jonathan.gibb...@oracle.com >> <mailto:jonathan.gibb...@oracle.com>>: >> >> Hannes, >> >> The test is weird, and it took me a while to understand what I think is >> going on. This predates your changes, but is still very confusing. >> >> If I understand the code correctly, initModulesAndPackages creates both >> packages and modules (which contain packages) in the same "src" directory. >> The same "src" directory is then used as an argument for both the module >> source path and source path, in different runs of javadoc. This is >> confusing, at least to me, because you can put a module on the source path >> and have javadoc recognize it as a module. But, it seems to work in the test >> because the test is relying on the naming conventions for qualified module >> names, and so it doesn't get confused when looking for packages or modules. >> But, I'm mildly surprised javac/javadoc doesn't complain at the "packages" >> found on the module source path. >> >> The test would be clearer if initModulesAndPackages wrote into two separate >> directories, one for module source code and the other for package source >> code. This would then affect/improve all the existing @Test methods by >> making it clear which directory structure they are reading source code from. >> >> If this simple refactoring works, the review is Approved. Otherwise, I'd >> like to understand the issues in more detail. >> >> -- Jon >> >> >> >> On 4/3/20 1:38 AM, Hannes Wallnoefer wrote: >>> I realised there’s a simpler solution to the problem. Instead of using a >>> Set to track packages with non-modular documentation we can just return the >>> module name that matches our internal model in #checkLinkCompatibility. >>> >>> I also changed the JBS summary to „javadoc fails to link to docs with >>> non-matching modularity“ so it covers both cases. >>> >>> New Webrev: http://cr.openjdk.java.net/~hannesw/8240169/webrev.01/ >>> <http://cr.openjdk.java.net/~hannesw/8240169/webrev.01/> >>> >>> Hannes >>> >>> >>>> Am 02.04.2020 um 16:06 schrieb Hannes Wallnoefer >>>> <hannes.wallnoe...@oracle.com <mailto:hannes.wallnoe...@oracle.com>>: >>>> >>>> Please review: >>>> >>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8240169 >>>> <https://bugs.openjdk.java.net/browse/JDK-8240169> >>>> Webrev: http://cr.openjdk.java.net/~hannesw/8240169/webrev.00/ >>>> <http://cr.openjdk.java.net/~hannesw/8240169/webrev.00/> >>>> >>>> This patch allows using external documentation even if it doesn’t match >>>> the external library in terms of modularity, i.e. non-modular >>>> documentation can be used for an modular library and vice versa. Instead >>>> of showing an error a warning is issued. There is still a warning if code >>>> and documentation do not match, but we use the check to tweak reference >>>> lookup so that we are still able to link to the appropriate documentation. >>>> >>>> I think that all relevant cases for combinations of modular and >>>> non-modular code are covered by existing tests (some of which change with >>>> this patch obviouly). >>>> >>>> TestLinkOptionWithAutomaticModule.java covers using a jar file as >>>> automatic or unnamed module, both of which cases were already supported as >>>> of JDK-8212233, so no changes there. >>>> >>>> TestLinkOptionWithModule.java covers all combinations of modular and >>>> non-modular code and documentation. For the two tests that cover >>>> non-matching combinations, I changed the expected return code to OK and >>>> added expected output with the link HTML. The message about the >>>> mismatching documentation is still there, but it is a warning instead of >>>> an error. >>>> >>>> I also added the JBS id to the @bug tag in TestLinkOptionWithModule.java >>>> as well as in TestClassCrossReferences.java which is another test that >>>> contains significant changes. >>>> >>>> Thanks, >>>> Hannes >>> >
