On 6/2/2026 12:00 AM, Markus Armbruster wrote:
> Pierrick Bouvier <[email protected]> writes:
>
>> On 6/1/2026 3:06 AM, Markus Armbruster wrote:
>>> Pierrick Bouvier <[email protected]> writes:
>>>
>>>> Hi Markus,
>>>>
>>>> On 5/21/2026 1:04 AM, Markus Armbruster wrote:
>>>>> Quite a few F: lines don't match any files. The quick & dirty check
>>>>>
>>>>> $ ls `sed -n 's/^F: *//p' MAINTAINERS ` >/dev/null
>>>>>
>>>>> finds about fifty.
>>>>>
>>>>> Philippe Mathieu-Daudé recently posted a few fixes:
>>>>>
>>>>> MAINTAINERS: Fix docker/dockerfiles/debian-hexagon-cross.docker path
>>>>> MAINTAINERS: Cover debian-loongarch-cross.docker with LoongArch section
>>>>> MAINTAINERS: Cover debian-xtensa-cross.docker with Xtensa section
>>>>> MAINTAINERS: Cover debian-tricore-cross.docker with TriCore section
>>>>> MAINTAINERS: Cover python.docker with Python library section
>>>>> MAINTAINERS: Fix s390x storage key/attribute device paths
>>>>> MAINTAINERS: Fix tcg/s390x/ path
>>>>> MAINTAINERS: Correct scripts/coverity-model.c path
>>>>> MAINTAINERS: Fix hexagon-linux-user.mak path
>
> [...]
>
>>>> In addition, see the patch attached to this email.
>>>> It integrates checking this directly at configure time, so we never run
>>>> into any missing entry again in the future.
>>>>
>>>> I share this here not for a review, but simply to avoid a duplicated
>>>> effort, and make sure people know it will be sent after this series.
>>>>
>>>> I don't believe in adding this in checkpatch, because it's not enforced
>>>> systematically unfortunately. Breaking the meson configuration is a good
>>>> way to make sure it's enforced by design.
>>>
>>> No objection.
>>>
>>>> With your series applied, the left entries are:
>>>
>>> Most of these are fixed in Philippe's patches mentioned above.
>>>
>>
>> 👍
>>
>>>> No matching files for /usr2/pbouvier/.work/qemu/MAINTAINERS +258:
>>>> configs/targets/hexagon-linux-user/default.mak
>>>
>>> MAINTAINERS: Fix hexagon-linux-user.mak path
>>>
>>>> No matching files for /usr2/pbouvier/.work/qemu/MAINTAINERS +259:
>>>> docker/dockerfiles/debian-hexagon-cross.docker
>>>
>>> MAINTAINERS: Fix docker/dockerfiles/debian-hexagon-cross.docker path
>>>
>>>> No matching files for /usr2/pbouvier/.work/qemu/MAINTAINERS +2956:
>>>> hw/s390x/storage-keys.h
>>>> No matching files for /usr2/pbouvier/.work/qemu/MAINTAINERS +2965:
>>>> hw/s390x/storage-attributes.h
>>>
>>> MAINTAINERS: Fix s390x storage key/attribute device paths
>>>
>>>> No matching files for /usr2/pbouvier/.work/qemu/MAINTAINERS +3241:
>>>> scripts/coverity-model.c
>>>
>>> MAINTAINERS: Correct scripts/coverity-model.c path
>>>
>>>> No matching files for /usr2/pbouvier/.work/qemu/MAINTAINERS +3468:
>>>> tests/*.py
>>>
>>> This is the exception I mentioned above.
>>>
>>> The intent is to match *.py below tests/. It actually matches only in
>>> tests/, not in its subdirectories.
>>>
>>> Here's a dumb fix:
>>>
>>> F: tests/*.py
>>> F: tests/*/*.py
>>> F: tests/*/*/*.py
>>> F: tests/*/*/*/*.py
>>>
>>
>> It's acceptable, and nice because it's not ambiguous.
>> Also, it shows us the first line is not a correct entry (there is no
>> tests/*.py).
>
> It's also brittle: breaks when we add the first .py at level.
>
>>> for however many levels we have. Same for the scripts/ line next to it.
>>> Blech.
>>>
>>> The smart fix might be to port N: from the kernel.
>>>
>>> N: Files and directories *Regex* patterns.
>>> N: [^a-z]tegra all files whose path contains tegra
>>> (not including files like integrator)
>>> One pattern per line. Multiple N: lines acceptable.
>>> scripts/get_maintainer.pl has different behavior for files that
>>> match F: pattern and matches of N: patterns. By default,
>>> get_maintainer will not look at git log history when an F: pattern
>>> match occurs. When an N: match occurs, git log history is used
>>> to also notify the people that have git commit signatures.
>>>
>>> But I wonder: is this section useful at all?
>>
>> I would prefer to use the manual entries approach above, especially if
>> there is only folder that is concerned.
>> When we'll have two different use case, maybe we can consider a more
>> generic approach.
>
> I'm aware of just two:
>
> Python scripts
> M: John Snow <[email protected]>
> M: Cleber Rosa <[email protected]>
> S: Odd Fixes
> F: scripts/*.py
> F: tests/*.py
>
> The section was added almost a decade ago (commit ad904f6689f), and
> never worked as intended. I asked John Snow about deleting it to
> unblock your work. He told me he'd be fine with that. Could add it
> back later fixed.
>
>>>> No matching files for /usr2/pbouvier/.work/qemu/MAINTAINERS +4149:
>>>> tcg/s390/
>>>
>>> MAINTAINERS: Fix tcg/s390x/ path
>>>
>>>> Once your current series is pulled, I'll fix the remaining and send the
>>>> attached patch. Or feel free to do it directly if you like the idea :)
>>>>
>>>> Regards,
>>>> Pierrick
>>>
>>> Two remarks inline.
>>>
>>>> From 6c9b49ac7ec06c0159d2b4ba9c9d1081e02ef765 Mon Sep 17 00:00:00 2001
>>>> From: Pierrick Bouvier <[email protected]>
>>>> Date: Fri, 22 May 2026 12:23:17 -0700
>>>> Subject: [PATCH] meson.build: check MAINTAINERS file is consistent with
>>>> source
>>>> tree
>>>>
>>>> We add a new script: scripts/check-maintainers-file.py, that will run at
>>>> configuration time (and not at build time), to not hurt build time.
>>>> This script runs in 0.2s on my dev VM, which has an old cpu.
>>>>
>>>> We can expect things to be mostly in sync since adding or removing a
>>>> source or test file will trigger a configure step.
>>>> For the rest, like docs, tcg tests, or remaining files, GitLab CI will
>>>> build things from scratch and always run the configure step.
>>>>
>>>> With this, it should be impossible by design to have an upstream
>>>> MAINTAINERS file with non existing file entries.
>>>>
>>>> Signed-off-by: Pierrick Bouvier <[email protected]>
>>>> ---
>>>> meson.build | 5 +++
>>>> scripts/check-maintainers-file.py | 53 +++++++++++++++++++++++++++++++
>>>> 2 files changed, 58 insertions(+)
>>>> create mode 100755 scripts/check-maintainers-file.py
>>>>
>>>> diff --git a/meson.build b/meson.build
>>>> index eeb096c1487..ddfb0b90ca6 100644
>>>> --- a/meson.build
>>>> +++ b/meson.build
>>>> @@ -18,6 +18,11 @@ add_test_setup('thorough',
>>>>
>>>> meson.add_postconf_script(find_program('scripts/symlink-install-tree.py'))
>>>>
>>>> +# check our MAINTAINERS file is consistent
>>>> +check_maintainers = find_program('scripts/check-maintainers-file.py')
>>>> +maintainers_file = files('MAINTAINERS')
>>>> +run_command([check_maintainers, maintainers_file], check: true, console:
>>>> true)
>>>
>>> My version of meson (1.8.5) chokes on console: true. According to
>>> https://mesonbuild.com/Reference-manual_functions_run_command.html#run_command_console
>>> it's new in 1.11.0.
>>>
>>
>> Our configure script creates a venv with an updated version of meson,
>> which is 1.11.1 at the moment.
>> If you called meson directly (which uses your host meson), it's not how
>> we're supposed to build QEMU.
>> How did you try this?
>
> I ran make :)
>
>>> I tested with it deleted.
>>>
>>>> +
>>>> ####################
>>>> # Global variables #
>>>> ####################
>>>> diff --git a/scripts/check-maintainers-file.py
>>>> b/scripts/check-maintainers-file.py
>>>> new file mode 100755
>>>> index 00000000000..b001816a401
>>>> --- /dev/null
>>>> +++ b/scripts/check-maintainers-file.py
>>>> @@ -0,0 +1,53 @@
>>>> +#! /usr/bin/env python3
>>>> +
>>>> +# Check incorrect file entries in MAINTAINERS
>>>> +#
>>>> +# Author: Pierrick Bouvier <[email protected]>
>>>> +#
>>>> +# SPDX-License-Identifier: GPL-2.0-or-later
>>>> +
>>>> +import argparse
>>>> +import glob
>>>> +import sys
>>>> +
>>>> +
>>>> +def check_one_entry(line) -> bool:
>>>> + return True
>>>> +
>>>> +
>>>> +def main() -> None:
>>>> + parser = argparse.ArgumentParser(description="Check MAINTAINERS file")
>>>> + parser.add_argument("maintainers", help="Path to MAINTAINERS file")
>>>> + args = parser.parse_args()
>>>> +
>>>> + found_file_entry = False
>>>> + found_incorrect_entries = False
>>>> + line_counter = 0
>>>> +
>>>> + with open(args.maintainers) as file:
>>>> + for entry in file:
>>>> + line_counter += 1
>>>> +
>>>> + if not entry.startswith("F:"):
>>>> + continue
>>>> + entry = entry[2:].strip()
>>>> + found_file_entry = True
>>>> +
>>>> + file_exists = len(glob.glob(entry, recursive=True)) > 0
>>>
>>> I'm afraid this matches files not in git, just like my quick & dirty
>>> one-liner. Shouldn't we match against contents of HEAD, say output of
>>> "git-ls-tree -r --name-only @"?
>>>
>>
>> I don't think it's needed to restrict to git ls-tree. The only risk is
>> that people have a local file they forgot (or didn't want) to add in
>> git. It will be caught by CI that won't have such a file, so we're safe.
>>
>> What do you think?
>
> Yes, we're safe, but the earlier we catch mistakes, the better. Now,
> the juice isn't always worth the squeeze. How hard would we have to
> squeeze here? We could use fnmatch.filter(names, pat) to test whether
> @pat matches anything in @names, where @names is from git-ls-tree.
>
"the earlier we catch mistakes, the better" is a good paradigm, but I
don't think we should project that to developer machine. The only thing
we need to keep "green" are our upstream branches.
I like the simplicity of current script, only dealing with file paths,
instead of having to do string regexp compare with git ls-files output,
but it's just a matter of personal taste and choosing the easiest
solution for a given problem.
That said, if filtering through git ls-files is a strong requirement, I
would be happy to implement it. Only thing that matters is that we have
something catching mistakes.
>>>> + if file_exists:
>>>> + continue
>>>> +
>>>> + found_incorrect_entries = True
>>>> + print(
>>>> + f"No matching files for {args.maintainers}
>>>> +{line_counter}: {entry}",
>>>> + file=sys.stderr,
>>>> + )
>>>> +
>>>> + if not found_file_entry:
>>>> + raise Exception("no file entry found - is MAINTAINERS path
>>>> correct?")
>>>> + if found_incorrect_entries:
>>>> + raise Exception(f"incorrect entries found in {args.maintainers}")
>>>> +
>>>> +
>>>> +if __name__ == "__main__":
>>>> + main()
>>>
>>
>> Regards,
>> Pierrick
>
