From a7ade81b6eb453a9568462b2060cb1d6b39cb632 Mon Sep 17 00:00:00 2001 From: Aleksandr Nogikh Date: Thu, 23 Feb 2023 16:17:40 +0100 Subject: pkg/subsystem/linux: support custom subsystem grouping There are cases when a subsystem doesn't have a mailing list and yet we'd prefer not to merge it with others. Let's add the ability to add custom rules that join several specified MAINTAINERS records into one Subsystem. --- pkg/subsystem/linux/names.go | 13 ++++++++++ pkg/subsystem/linux/names_test.go | 32 ++++++++++++++++++++++++- pkg/subsystem/linux/rules.go | 4 ++++ pkg/subsystem/linux/subsystems.go | 43 ++++++++++++++++++++++++++++++---- pkg/subsystem/linux/subsystems_test.go | 23 ++++++++++++++++++ 5 files changed, 110 insertions(+), 5 deletions(-) (limited to 'pkg/subsystem/linux') diff --git a/pkg/subsystem/linux/names.go b/pkg/subsystem/linux/names.go index 84bba148a..4727f3d0a 100644 --- a/pkg/subsystem/linux/names.go +++ b/pkg/subsystem/linux/names.go @@ -16,6 +16,19 @@ import ( func setSubsystemNames(list []*subsystem.Subsystem) error { dups := map[string]bool{} for _, item := range list { + if item.Name == "" { + continue + } + if dups[item.Name] { + return fmt.Errorf("duplicate name: %s", item.Name) + } + dups[item.Name] = true + } + + for _, item := range list { + if item.Name != "" { + continue + } // For now, we can only infer name from the list email. if len(item.Lists) == 0 { return fmt.Errorf("no lists for %#v", item) diff --git a/pkg/subsystem/linux/names_test.go b/pkg/subsystem/linux/names_test.go index bcb816592..a158bd753 100644 --- a/pkg/subsystem/linux/names_test.go +++ b/pkg/subsystem/linux/names_test.go @@ -31,12 +31,13 @@ func TestEmailToName(t *testing.T) { } type subsystemTestInput struct { + inName string email string outName string } func (sti subsystemTestInput) ToSubsystem() *subsystem.Subsystem { - s := &subsystem.Subsystem{} + s := &subsystem.Subsystem{Name: sti.inName} if sti.email != "" { s.Lists = append(s.Lists, sti.email) } @@ -90,6 +91,35 @@ func TestSetSubsystemNames(t *testing.T) { }, mustFail: true, }, + { + name: "has existing names", + inputs: []subsystemTestInput{ + { + inName: "some", + email: "some-list@list.com", + outName: "some", + }, + { + email: "ntfs@lists.sourceforge.net", + outName: "ntfs", + }, + }, + }, + { + name: "collision with an existing name", + inputs: []subsystemTestInput{ + { + inName: "ntfs", + email: "ntfs3@lists.sourceforge.net", + outName: "ntfs", + }, + { + email: "ntfs@lists.sourceforge.net", + outName: "ntfs", + }, + }, + mustFail: true, + }, } for _, test := range tests { curr := test diff --git a/pkg/subsystem/linux/rules.go b/pkg/subsystem/linux/rules.go index 3ed8cf0fd..9282a7129 100644 --- a/pkg/subsystem/linux/rules.go +++ b/pkg/subsystem/linux/rules.go @@ -9,6 +9,9 @@ type customRules struct { // These emails do not represent separate subsystems, even though they seem to // per all criteria we have. notSubsystemEmails map[string]struct{} + // These subsystems need to be extracted even without mailing lists. + // Key is the subsystem name, value is the list of raw names in MAINTAINERS. + extraSubsystems map[string][]string } var ( @@ -64,5 +67,6 @@ var ( "coreteam@netfilter.org": {}, "SHA-cyfmac-dev-list@infineon.com": {}, }, + extraSubsystems: map[string][]string{}, } ) diff --git a/pkg/subsystem/linux/subsystems.go b/pkg/subsystem/linux/subsystems.go index 18ac3864e..b14a3cbe5 100644 --- a/pkg/subsystem/linux/subsystems.go +++ b/pkg/subsystem/linux/subsystems.go @@ -30,6 +30,11 @@ func listFromRepoInner(root fs.FS, rules *customRules) ([]*subsystem.Subsystem, extraRules: rules, } list := ctx.groupByList() + extraList, err := ctx.groupByRules() + if err != nil { + return nil, err + } + list = append(list, extraList...) matrix, err := BuildCoincidenceMatrix(root, list, dropPatterns) if err != nil { return nil, err @@ -86,7 +91,7 @@ func (ctx *linuxCtx) groupByList() []*subsystem.Subsystem { if _, skip := exclude[email]; skip { continue } - s := mergeRawRecords(email, list) + s := mergeRawRecords(list, email) // Skip empty subsystems. if len(s.PathRules) > 0 { ret = append(ret, s) @@ -95,6 +100,30 @@ func (ctx *linuxCtx) groupByList() []*subsystem.Subsystem { return ret } +func (ctx *linuxCtx) groupByRules() ([]*subsystem.Subsystem, error) { + if ctx.extraRules == nil { + return nil, nil + } + perName := map[string]*maintainersRecord{} + for _, item := range ctx.rawRecords { + perName[item.name] = item + } + ret := []*subsystem.Subsystem{} + for name, recordNames := range ctx.extraRules.extraSubsystems { + matching := []*maintainersRecord{} + for _, recordName := range recordNames { + if perName[recordName] == nil { + return nil, fmt.Errorf("MAINTAINERS record not found: %#v", recordName) + } + matching = append(matching, perName[recordName]) + } + s := mergeRawRecords(matching, "") + s.Name = name + ret = append(ret, s) + } + return ret, nil +} + func (ctx *linuxCtx) applyExtraRules(list []*subsystem.Subsystem) { if ctx.extraRules == nil { return @@ -104,7 +133,7 @@ func (ctx *linuxCtx) applyExtraRules(list []*subsystem.Subsystem) { } } -func mergeRawRecords(email string, records []*maintainersRecord) *subsystem.Subsystem { +func mergeRawRecords(records []*maintainersRecord, email string) *subsystem.Subsystem { unique := func(list []string) []string { m := make(map[string]struct{}) for _, s := range list { @@ -117,15 +146,21 @@ func mergeRawRecords(email string, records []*maintainersRecord) *subsystem.Subs sort.Strings(ret) return ret } - var maintainers []string - subsystem := &subsystem.Subsystem{Lists: []string{email}} + var lists, maintainers []string + subsystem := &subsystem.Subsystem{} for _, record := range records { rule := record.ToPathRule() if !rule.IsEmpty() { subsystem.PathRules = append(subsystem.PathRules, rule) } + lists = append(lists, record.lists...) maintainers = append(maintainers, record.maintainers...) } + if email != "" { + subsystem.Lists = []string{email} + } else if len(lists) > 0 { + subsystem.Lists = unique(lists) + } // There's a risk that we collect too many unrelated maintainers, so // let's only merge them if there are no lists. if len(records) <= 1 { diff --git a/pkg/subsystem/linux/subsystems_test.go b/pkg/subsystem/linux/subsystems_test.go index cbc4b9699..f60f6d70f 100644 --- a/pkg/subsystem/linux/subsystems_test.go +++ b/pkg/subsystem/linux/subsystems_test.go @@ -63,6 +63,17 @@ func TestCustomCallRules(t *testing.T) { if err != nil { t.Fatal(err) } + for _, s := range subsystems { + // The regexps used for matching rules may change later, so let's not compare them here. + s.PathRules = nil + // It complicates the test, so let's skip it here. + s.Parents = nil + } + // Ensure that the subsystem from the custom rule is present. + assert.Contains(t, subsystems, &subsystem.Subsystem{ + Name: "udf", + Maintainers: []string{"email_udf@email.com"}, + }) expectCalls := map[string][]string{ "ext4": {"syz_mount_image$ext4"}, "tmpfs": {"syz_mount_image$tmpfs"}, @@ -175,6 +186,9 @@ func prepareTestLinuxRepo(t *testing.T, maintainers []byte) fs.FS { `fs/freevxfs/vxfs_olt.c`: {}, `fs/freevxfs/vxfs_olt.h`: {}, `fs/freevxfs/file.c`: {}, + `fs/udf/file.c`: {}, + `fs/udf/file2.c`: {}, + `fs/udf/file3.c`: {}, `fs/file.c`: {}, `fs/internal.h`: {}, `include/linux/fs.h`: {}, @@ -195,6 +209,9 @@ var ( "vxfs": {"syz_mount_image$vxfs"}, "tmpfs": {"syz_mount_image$tmpfs"}, }, + extraSubsystems: map[string][]string{ + "udf": {"UDF FILESYSTEM"}, + }, } testMaintainers = ` Maintainers List @@ -256,6 +273,12 @@ S: Maintained F: include/linux/shmem_fs.h F: mm/shmem* +UDF FILESYSTEM +M: email_udf +S: Maintained +F: Documentation/filesystems/udf.rst +F: fs/udf/ + THE REST M: Developer L: linux-kernel@vger.kernel.org -- cgit mrf-deployment