On Fri, Sep 03, 2021 at 06:38:38AM +0300, Amir Goldstein wrote:
> > diff --git a/include/buildgrouplist b/include/buildgrouplist
> > index d898efa3..489de965 100644
> > --- a/include/buildgrouplist
> > +++ b/include/buildgrouplist
> > @@ -6,3 +6,4 @@
> > group.list:
> > @echo " [GROUP] $$PWD/$@"
> > $(Q)$(TOPDIR)/tools/mkgroupfile $@
> > + $(Q)$(TOPDIR)/tools/check-groups $(TOPDIR)/doc/group-names.txt $@
>
> I would like to argue against checking groups post mkgroupfile
> and for checking groups during mkgroupfile
Done.
> > diff --git a/tools/check-groups b/tools/check-groups
> > new file mode 100755
> > index 00000000..0d193615
> > --- /dev/null
> > +++ b/tools/check-groups
> > @@ -0,0 +1,35 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2021 Oracle. All Rights Reserved.
> > +#
> > +# Make sure that all groups listed in a group.list file are mentioned in the
> > +# group description file.
> > +
> > +if [ -z "$1" ] || [ "$1" = "--help" ]; then
> > + echo "Usage: $0 path_to_group_names [group.list files...]"
> > + exit 1
> > +fi
> > +
> > +groups_doc_file="$1"
> > +shift
> > +
> > +get_group_list() {
> > + for file in "$@"; do
> > + while read testname groups; do
> > + test -z "${testname}" && continue
> > + test "${testname:0:1}" = "#" && continue
> > +
> > + echo "${groups}" | tr ' ' '\n'
> > + done < "${file}"
> > + done | sort | uniq
> > +}
> > +
> > +ret=0
> > +while read group; do
> > + if ! grep -q "^${group}[[:space:]]" "${groups_doc_file}"; then
> > + echo "${group}: group not mentioned in documentation." 1>&2
>
> This message would have been more informative with the offending
> test file.
Hm. This becomes much easier if I make the _begin_fstest helper do the
checking of the group names.
> Now after you crunched all the test files into group.list files and
> all the group.list files into a unique group set, this is too late.
> But this same check during generate_groupfile() would have
> been trivial and would allow reporting the offending test.
>
> While we are on the subject of generate_groupfile(), can you please
> explain the rationale behind the method of extracting the test file
> groups by executing the test with GENERATE_GROUPS=yes?
> As opposed to just getting the list of groups on the stop from the file
> using grep?
Well... now that you point that out, it's so that we can put in custom
logic like checking group names. ;)
--D
>
> Thanks,
> Amir.
