Re: [PATCH v2 1/2] devtools/mailmap_ctl: script to work with mailmap

[Bruce Richardson <bruce.richardson@intel.com>]

On Wed, Oct 15, 2025 at 04:20:57PM +0200, Robin Jarry wrote:
> Hi Bruce, see my comments inline.
> 

Thanks for review. Taking nearly all feedback in v3. See inline below.

/Bruce

> Bruce Richardson, Aug 08, 2025 at 23:08:
> > Add a script to easily add entries to, check and sort the mailmap file.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  devtools/mailmap_ctl.py | 212 ++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 212 insertions(+)
> >  create mode 100755 devtools/mailmap_ctl.py
> >
> > diff --git a/devtools/mailmap_ctl.py b/devtools/mailmap_ctl.py
> > new file mode 100755
> > index 0000000000..15548c54cd
> > --- /dev/null
> > +++ b/devtools/mailmap_ctl.py
> > @@ -0,0 +1,212 @@
> > +#!/usr/bin/env python3
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright(c) 2025 Intel Corporation
> > +
> > +"""
> > +A tool for manipulating the .mailmap file in DPDK repository.
> > +
> > +This script supports three operations:
> > +- add: adds a new entry to the mailmap file in the correct position
> > +- check: validates mailmap entries are sorted and correctly formatted
> > +- sort: sorts the mailmap entries alphabetically by name
> 
> You can remove the second paragraph and the bullet points. See at the
> end.
> 

Ack, will remove in v3

> > +"""
> > +
> > +import sys
> > +import re
> > +import argparse
> > +import itertools
> > +import unicodedata
> > +from pathlib import Path
> > +from dataclasses import dataclass
> > +from typing import List, Optional
> 
> That's a matter of preference, but for such a small script I prefer to
> only import top level modules, e.g.:
> 

Given the script is short, I prefer the opposite - the amount of context is
so small there is no need to keep the namespace along with the functions
like Path, or dataclass.

> import pathlib
> import dataclasses
> 
> By the way, since python 3.10, you can use the builtin symbols instead
> of importing typing symbols. See below for more details.
> 

Ack. Not sure what our min python is, but this is not a regular user tool,
so requiring 3.10 should be fine for any devs or maintainers.

> > +
> > +
> > +@dataclass
> > +class MailmapEntry:
> > +    """Represents a single mailmap entry."""
> > +
> > +    name: str
> > +    name_for_sorting: str
> > +    email1: str
> > +    email2: Optional[str]
> 
> Replace with python 3.10 syntax:
> 
> 	email2: str | None
> 

Will fix in v3

> > +    line_number: int
> > +
> > +    def __str__(self) -> str:
> > +        """Format the entry back to mailmap string format."""
> > +        return f"{self.name} <{self.email1}>" + (f" <{self.email2}>" if self.email2 else "")
> > +
> > +    @staticmethod
> > +    def _get_name_for_sorting(name) -> str:
> > +        """Normalize a name for sorting purposes."""
> > +        # Remove accents/diacritics. Separate accented chars into two - so accent is separate,
> > +        # then remove the accent.
> > +        normalized = unicodedata.normalize("NFD", name)
> > +        normalized = "".join(c for c in normalized if unicodedata.category(c) != "Mn")
> > +
> > +        return normalized.lower()
> > +
> > +    @classmethod
> > +    def parse(cls, line: str, line_number: int = 0):
> > +        """
> > +        Parse a mailmap line and create a MailmapEntry instance.
> > +
> > +        Valid formats:
> > +        - Name <email>
> > +        - Name <primary_email> <secondary_email>
> > +        """
> > +        # Pattern to match mailmap entries
> > +        # Group 1: Name, Group 2: first email, Group 3: optional second email
> > +        pattern = r"^([^<]+?)\s*<([^>]+)>(?:\s*<([^>]+)>)?$"
> > +        match = re.match(pattern, line.strip())
> > +        if not match:
> > +            raise argparse.ArgumentTypeError(f"Invalid entry format: '{line}'")
> > +
> > +        name = match.group(1).strip()
> > +        primary_email = match.group(2).strip()
> > +        secondary_email = match.group(3).strip() if match.group(3) else None
> > +
> > +        return cls(
> > +            name=name,
> > +            name_for_sorting=cls._get_name_for_sorting(name),
> > +            email1=primary_email,
> > +            email2=secondary_email,
> > +            line_number=line_number,
> > +        )
> > +
> > +
> > +def read_and_parse_mailmap(mailmap_path: Path, fail_on_err: bool) -> List[MailmapEntry]:
> 
> Replace with python 3.9 builtin list and use pathlib module directly:
> 
> def read_and_parse_mailmap(mailmap_path: pathlib.Path, fail_on_err: bool) -> list[MailmapEntry]:
> 

Ack for using builtin lists. Keep un-namespaced Path.

> > +    """Read and parse a mailmap file, returning entries."""
> > +    try:
> > +        with open(mailmap_path, "r", encoding="utf-8") as f:
> > +            lines = f.readlines()
> > +    except IOError as e:
> > +        print(f"Error reading {mailmap_path}: {e}", file=sys.stderr)
> > +        sys.exit(1)
> > +
> > +    entries = []
> > +    for line_num, line in enumerate(lines, 1):
> > +        stripped_line = line.strip()
> > +
> > +        # Skip empty lines and comments
> > +        if not stripped_line or stripped_line.startswith("#"):
> > +            continue
> > +
> > +        try:
> > +            entry = MailmapEntry.parse(stripped_line, line_num)
> > +        except argparse.ArgumentTypeError as e:
> > +            print(f"Line {line_num}: {e}", file=sys.stderr)
> > +            if fail_on_err:
> > +                sys.exit(1)
> > +            continue
> > +
> > +        entries.append(entry)
> > +    return entries
> > +
> > +
> > +def write_entries_to_file(mailmap_path: Path, entries: List[MailmapEntry]):
> 
> Same:
> 
> def write_entries_to_file(mailmap_path: pathlib.Path, entries: list[MailmapEntry]):
> 
> > +    """Write entries to mailmap file."""
> > +    try:
> > +        with open(mailmap_path, "w", encoding="utf-8") as f:
> > +            for entry in entries:
> > +                f.write(str(entry) + "\n")
> > +    except IOError as e:
> > +        print(f"Error writing {mailmap_path}: {e}", file=sys.stderr)
> > +        sys.exit(1)
> > +
> > +
> > +def check_mailmap(mailmap_path, _):
> > +    """Check that mailmap entries are correctly sorted and formatted."""
> > +    entries = read_and_parse_mailmap(mailmap_path, False)
> > +
> > +    errors = 0
> > +    for e1, e2 in itertools.pairwise(entries):
> > +        if e1.name_for_sorting > e2.name_for_sorting:
> > +            print(
> > +                f"Line {e2.line_number}: '{e2.name}' should come before '{e1.name}'",
> > +                file=sys.stderr,
> > +            )
> > +            errors += 1
> > +
> > +    if errors:
> > +        sys.exit(1)
> > +
> > +
> > +def sort_mailmap(mailmap_path, _):
> > +    """Sort the mailmap entries alphabetically by name."""
> > +    entries = read_and_parse_mailmap(mailmap_path, True)
> > +
> > +    entries.sort(key=lambda x: x.name_for_sorting)
> > +    write_entries_to_file(mailmap_path, entries)
> > +
> > +
> > +def add_entry(mailmap_path, args):
> > +    """Add a new entry to the mailmap file in the correct alphabetical position."""
> > +    if not args.entry:
> > +        print("Error: 'add' operation requires an entry argument", file=sys.stderr)
> > +        sys.exit(1)
> > +
> > +    new_entry = args.entry
> > +    entries = read_and_parse_mailmap(mailmap_path, True)
> > +
> > +    # Check if entry already exists, checking email2 only if it's specified
> > +    if (
> > +        not new_entry.email2
> > +        and any(e.name == new_entry.name and e.email1 == new_entry.email1 for e in entries)
> > +    ) or any(
> > +        e.name == new_entry.name and e.email1 == new_entry.email1 and e.email2 == new_entry.email2
> > +        for e in entries
> > +    ):
> > +        print(
> > +            f"Error: Duplicate entry - '{new_entry.name} <{new_entry.email1}>' already exists",
> > +            file=sys.stderr,
> > +        )
> > +        sys.exit(1)
> > +
> > +    for n, entry in enumerate(entries):
> > +        if entry.name_for_sorting > new_entry.name_for_sorting:
> > +            entries.insert(n, new_entry)
> > +            break
> > +    else:
> > +        entries.append(new_entry)
> > +    write_entries_to_file(mailmap_path, entries)
> > +
> > +
> > +def main():
> > +    """Main function."""
> > +    # ops and functions implementing them
> > +    operations = {"add": add_entry, "check": check_mailmap, "sort": sort_mailmap}
> > +
> > +    parser = argparse.ArgumentParser(
> > +        description=__doc__,
> > +        formatter_class=argparse.RawDescriptionHelpFormatter,
> > +        epilog="NOTE:\n for operations which write .mailmap, any comments or blank lines in the file will be removed",
> > +    )
> > +    parser.add_argument("operation", choices=operations.keys(), help="Operation to perform")
> > +    parser.add_argument("--mailmap", help="Path to .mailmap file (default: search up tree)")
> > +    parser.add_argument(
> > +        "entry",
> > +        nargs="?",
> > +        type=MailmapEntry.parse,
> > +        help='Entry to add. Format: "Name <email@domain.com>"',
> > +    )
> 
> Here you can use sub parsers instead:
> 
> 	parser = argparse.ArgumentParser(
> 		description=__doc__,
> 		epilog="NOTE: for operations which write .mailmap, any comments or blank lines in the file will be removed",
> 	)
> 	parser.add_argument("--mailmap", help="Path to .mailmap file (default: search up tree)")
> 	sub = parser.add_subparsers(title="sub-command help", metavar="SUB_COMMAND")
> 	sub.required = True
> 	add = sub.add_parser("add", description=add_entry.__doc__, help=add_entry.__doc__)
> 	add.add_argument(
> 		"entry",
> 		type=MailmapEntry.parse,
> 		help='Entry to add. Format: "Name <email@domain.com>"',
> 	)
> 	add.set_defaults(callback=add_entry)
> 	check = sub.add_parser("check", description=check_mailmap.__doc__, help=check_mailmap.__doc__)
> 	check.set_defaults(callback=check_mailmap)
> 	sort = sub.add_parser("sort", description=sort_mailmap.__doc__, help=sort_mailmap.__doc__)
> 	sort.set_defaults(callback=sort_mailmap)
> 

Very neat thanks. Adding to v3.

> > +
> > +    args = parser.parse_args()
> > +
> > +    if args.mailmap:
> > +        mailmap_path = Path(args.mailmap)
> 
> 	args.mailmap = pathlib.Path(args.mailmap)
> 
> > +    else:
> > +        # Find mailmap file
> > +        mailmap_path = Path(".").resolve()
> > +        while not (mailmap_path / ".mailmap").exists():
> > +            if mailmap_path == mailmap_path.parent:
> > +                print("Error: No .mailmap file found", file=sys.stderr)
> > +                sys.exit(1)
> > +            mailmap_path = mailmap_path.parent
> > +        mailmap_path = mailmap_path / ".mailmap"
> 
> 	args.mailmap = mailmap_path / ".mailmap"
> > +
> > +    # call appropriate operation
> > +    operations[args.operation](mailmap_path, args)
> 
> And there (you'll need to adjust all callbacks to take a single args
> parameter):
> 
> 	args.callback(args)
> 

Changed in v3.

> > +
> > +
> > +if __name__ == "__main__":
> > +    main()
> 
> 
> -- 
> Robin
> 
> > Do not use or store near heat or open flame.
>