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

[Doug Simon]

Is there a reliable way to infer mailing lists from OpenJDK authors? If so, you could use git blame to derive a set of mailing lists by git blaming around the changed lines. GitHub already does something like this by suggesting reviewers on a PR:

https://www.dropbox.com/s/qog88q5br5m0dsh/Screen%20Shot%202020-08-28%20at%2011.21.11.png?dl=0

-Doug

> On 28 Aug 2020, at 08:46, Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
> 
> Maybe this shows also that we have too many mailing lists? Seems to me like
> an electrical solution to an organizational problem.
> 
> There are a number of lists I am never sure about, so I usually crosspost
> to them-and-some-other-list. Examples are jfr-dev (which intersects with
> runtime a lot), ppc-aix-port-dev, bsd-port-dev - all the porter lists
> really apart from aarch64 as David pointed out - and a number of others. I
> also subscribe to all of them, so to me they don't serve the purpose of a
> mail filter. All of the devs I know do the same but that may be just my
> bubble.
> 
> IMHO if a mailing list has a lot of unique traffic, and serves as an
> exclusion filter for a lot of recipients, it makes sense. I see hs-gc and
> hs-compiler like this. But if all recipients subscribe to all neighboring
> lists too, I don't really see the point. Directing attention can also be
> done with terms in subject headers (e.g. "[Windows]", "NMT", "CDS",
> "Metaspace"). Using these I never had the problem waking up the right
> people.
> 
> All this is messy because we humans are messy, and that is fine. I just
> think culling the list of MLs a bit could simplify semi-automatic ML
> selection.
> 
> Cheers, Thomas
> 
> 
> On Fri, Aug 28, 2020 at 2:45 AM David Holmes <david.holmes at oracle.com>
> wrote:
> 
>> 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.
>>>> 
>>>> 
>>