Request for assistance: Verify and update mailing list filter rules for jdk/jdk in preparation for Skara transition

[David Holmes]

On 28/08/2020 8:44 am, Thomas Stüfe wrote:
> Hi Robin,
> 
> FWIW I agree with David and Jesper.
> 
> IMHO any automated system will not be sufficient. It should only generate
> coarse grained proposals, and the developer should be able to manually
> cull, extend or completely overwrite them. Especially for broader changes
> which only touch a few lines per file, e.g. cleanups, I would really
> dislike auto-crossposting to a large number of lists. Also, there are many
> cases where the correct list is not clear - e.g. changes inside
> hotspot/share/jfr may be better suited for hotspot-runtime or hotspot-dev
> instead of jfr, depending on what is changed. This is especially true for
> changes to os-port files.
> 
> I think that the json files are way too expansive, and fear the effort to
> maintain them would outgrow the effort to direct newcomers to the new
> mailing list.
> 
> Like Igor I also would prefer some sort of notation in-tree instead of a
> separate control file in a different repo (how would that work with
> different jdk repositories?).
> 
> Maybe a simple stupid mechanism with the ability to redefine targets at
> deeper folders or at files would be more concise. For example,
> pragmatically everything inside "hotspot" could be runtime, unless
> "hotspot/share/gc", which would be gc, "hotspot/share/compiler" would be
> compiler, "hotspot/share/jfr" would be jfr, hotspot/os/<xx> could be <xxx>
> port... I don't even see a need for regular expressions tbh.

There's also serviceability-dev (where the line with runtime is often 
very blurred).

The os and cpu directories are very tricky to classify on a file basis 
as the the files do not clearly delineate functional areas that map to 
mailing lists. In many cases it is the nature of the change that 
dictates which MLs should be getting the RFR. And we don't really have 
any guidelines for when to use hotspot-dev versus one or more 
hotspot-x-dev lists. There is a subjectivity here that is not amenable 
to automated classification without reorganising files/directories in a 
way that reflects the ML granularity (and I certainly would not want to 
see that kind of churn).

Also we don't have xxx_port mailing lists for all xxx, and those lists 
are intended for discussion of porting projects, not mainline issues 
that affect platform specific code per-se. Though Aarch64 is blurring 
that distinction these days (just an observation not a criticism).

Cheers,
David

> Cheers, Thomas
> 
> On Thu, Aug 27, 2020 at 12:35 PM Robin Westberg <robin.westberg at oracle.com>
> wrote:
> 
>> Hi all,
>>
>> As part of transitioning the jdk/jdk repository to Git with project Skara,
>> we have created a set of rules that determine which mailing list(s) should
>> be the default recipient of a review request, depending on which files are
>> changed. The initial version of these rules was created by looking at
>> historical commits and matching them with existing mailing list review
>> threads. This has produced a reasonable basis, but it can most certainly be
>> made better with some additional manual curating.
>>
>> Therefore, it would be very helpful if people with good knowledge of the
>> various subsystems and source files that make up the JDK would be willing
>> to take a look at these rules, and also suggest improvements where needed.
>> In addition, lists like [1] would also be very useful insofar they exist.
>>
>> The current version of these rules is located in a JSON file in the Skara
>> repository at [2]. In order to check the validity of the rules, there is
>> also a CLI tool that can be used to apply it to either a subset of files or
>> existing commits and produce a suggestion [3] [4]. If you are interested in
>> helping out with curating these rules, these are the steps to get started:
>>
>> 1. Install the Skara CLI tools:
>> https://wiki.openjdk.java.net/display/SKARA/CLI+Tools
>> 2. Locate a suitable clone of the jdk/jdk repository (either Mercurial or
>> Git is fine)
>> 3. Change (cd) to the root of your jdk/jdk repository
>> 3. Run the “debug mlrules” command on your favorite subset of files, for
>> example like this (use the actual location of jdk.json on your system):
>>
>> $ git skara debug mlrules -v ~/git/skara/config/mailinglist/rules/jdk.json
>> src/hotspot/share/jfr/
>> Reading rules file...
>> src/hotspot/share/jfr/dcmd: [hotspot-jfr-dev]
>> src/hotspot/share/jfr/instrumentation/jfrEventClassTransformer.cpp:
>> [hotspot-jfr-dev]
>> src/hotspot/share/jfr/instrumentation/jfrEventClassTransformer.hpp:
>> [hotspot-jfr-dev]
>> src/hotspot/share/jfr/instrumentation/jfrJvmtiAgent.cpp: [hotspot-jfr-dev,
>> serviceability-dev]
>> …
>> Final list suggestion is: [hotspot-jfr-dev, serviceability-dev]
>>
>> The command accepts multiple folder and/or file names to make it possible
>> to simulate a potential change to a given set of files:
>>
>> $ git skara debug mlrules -v ../skara/config/mailinglist/rules/jdk.json
>> doc/ide.md src/hotspot/cpu/x86/x86.ad
>> src/hotspot/os/linux/gc/z/zNUMA_linux.cpp
>> Reading rules file...
>> doc: [build-dev]
>> src/hotspot/cpu: [hotspot-compiler-dev]
>> src/hotspot/os: [hotspot-runtime-dev, hotspot-gc-dev]
>>
>> Combined list suggestion: [build-dev, hotspot-compiler-dev,
>> hotspot-gc-dev, hotspot-runtime-dev]
>> Final list suggestion is: [build-dev, hotspot-dev]
>>
>> If the suggestions look fine, all is well. If not, you are welcome to
>> propose a change to the rules, preferably by editing the jdk.json file [6]
>> and creating a pull request towards the Skara project as described in [5].
>> Coincidentally, this is the same way that future changes to the jdk/jdk
>> repository will be integrated, so this exercise could also serve as a way
>> of getting started with Git / Skara!
>>
>> Best regards,
>> Robin
>>
>> [1] https://openjdk.java.net/groups/2d/2dawtfiles.html
>> [2]
>> https://git.openjdk.java.net/skara/blob/master/config/mailinglist/rules/jdk.json
>>
>> [3]
>> $ git skara debug mlrules -v ~/git/skara/config/mailinglist/rules/jdk.json
>> src/java.desktop/unix/native
>> Reading rules file...
>> src/java.desktop/unix/native/common: [2d-dev]
>> src/java.desktop/unix/native/include: []
>> src/java.desktop/unix/native/libawt: [2d-dev]
>> src/java.desktop/unix/native/libawt_headless: [awt-dev]
>> src/java.desktop/unix/native/libawt_xawt: [awt-dev]
>> src/java.desktop/unix/native/libfontmanager: [2d-dev]
>> src/java.desktop/unix/native/libjawt: [awt-dev]
>> src/java.desktop/unix/native/libsplashscreen: [awt-dev]
>>
>> Combined list suggestion: [2d-dev, awt-dev]
>> Final list suggestion is: [2d-dev, awt-dev]
>>
>> [4]
>> $ git skara debug mlrules -d 30 -v
>> ~/git/skara/config/mailinglist/rules/jdk.json .
>> ...
>> ✅ [2d-dev, awt-dev, serviceability-dev] c32923e0: 8240487: Cleanup
>> whitespace in .cc, .hh, .m, and .mm files
>> ❌ [awt-dev] 7f74c7dd: 8212226: SurfaceManager throws "Invalid Image
>> variant" for MultiResolutionImage (Windows)
>>      Suggested lists: [2d-dev, awt-dev]
>>      Rules matching unmentioned lists [2d-dev]:
>>        src/java.desktop/share/classes/sun/java2d/SunGraphics2D.java -
>> [2d-dev: ^src/java.desktop/share/classes/sun/java2d/]
>> …
>>
>> [5] https://wiki.openjdk.java.net/display/SKARA/Skara#Skara-Workflow
>>
>> [6]
>> The rules are made up of sets of regular expressions for the various
>> mailing lists that are used for reviewing changes going into the JDK. If
>> any rule matches, that mailing list will get a copy of the review request
>> email. For directories containing files that belong to different
>> subsystems, it’s usually a good idea to write the rules in a complementary
>> fashion if possible, so that anything not explicitly mentioned gets a
>> reasonable default. As an example, see these rules for a subset of awt / 2d
>> / i18n files:
>>
>> “awt-dev”:
>> "src/java.desktop/share/classes/sun/awt/(?!font|sunhints|color/|font/|geom/|im/|image/|print/)”
>> “2d-dev”:
>> "src/java.desktop/share/classes/sun/awt/(font|sunhints|color/|font/|geom/|image/|print/)"
>> “I18n-dev”: "src/java.desktop/share/classes/sun/awt/im/“
>>
>> In this example, anything not explicitly indicated as belonging to either
>> 2d-dev or i18-dev will be matched by awt-dev.
>>
>>