[coreboot-gerrit] Change in coreboot[master]: util/scripts/maintainers.go: Use a full glob parser

[Patrick Georgi (Code Review)]

Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/29603


Change subject: util/scripts/maintainers.go: Use a full glob parser
......................................................................

util/scripts/maintainers.go: Use a full glob parser

Instead of checking only for three cases, just use a glob parser.
This requires to download the parser first, using:

    $ go get github.com/gobwas/glob # It's MIT licensed

After that, maintainers src/arch/x86/memlayout.h emits:

    src/arch/x86/memlayout.h is in subsystem X86 ARCHITECTURE
    Maintainers:  []
    src/arch/x86/memlayout.h is in subsystem MEMLAYOUT
    Maintainers:  [Julius Werner <jwerner at chromium.org>]

The latter entry was invisible to the maintainers tool because its path
description wasn't in one of the supported formats.

Change-Id: I7e5cf4269415269552e35f2c73952ce3dff487e1
Signed-off-by: Patrick Georgi <pgeorgi at google.com>
---
M util/scripts/maintainers.go
1 file changed, 22 insertions(+), 71 deletions(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/29603/1

diff --git a/util/scripts/maintainers.go b/util/scripts/maintainers.go
index 352d0a1..f99dc37 100644
--- a/util/scripts/maintainers.go
+++ b/util/scripts/maintainers.go
@@ -19,13 +19,15 @@
 	"log"
 	"os"
 	"os/exec"
-	"path/filepath"
+
+	"github.com/gobwas/glob"
 )
 
 type subsystem struct {
 	name       string
 	maintainer []string
-	file       []string
+	paths      []string
+	globs      []glob.Glob
 }
 
 var subsystems []subsystem
@@ -84,6 +86,14 @@
 	return maintainers, nil
 }
 
+func path_to_glob(path string) glob.Glob {
+	// if prefix, allow all subdirectories
+	if path[len(path)-1] == '/' {
+		path += "**"
+	}
+	return glob.MustCompile(path)
+}
+
 func build_maintainers(maintainers []string) {
 	var current *subsystem
 	for _, line := range maintainers {
@@ -119,9 +129,8 @@
 			case 'F':
 				{
 					// add files
-					current.file =
-						append(current.file,
-							line[3:len(line)])
+					current.paths = append(current.paths, line[3:len(line)])
+					current.globs = append(current.globs, path_to_glob(line[3:len(line)]))
 					break
 				}
 			case 'W':
@@ -142,77 +151,23 @@
 	for _, subsystem := range subsystems {
 		fmt.Println(subsystem.name)
 		fmt.Println("  ", subsystem.maintainer)
-		fmt.Println("  ", subsystem.file)
+		fmt.Println("  ", subsystem.paths)
 	}
 }
 
-func match_file(fname string, files []string) (bool, error) {
-	var matched bool
-	var err error
-
-	for _, file := range files {
-		/* Direct match */
-		matched, err = filepath.Match(file, fname)
-		if err != nil {
-			return false, err
-		}
-		if matched {
-			return true, nil
-		}
-
-		/* There are three cases that match_file can handle:
-		 *
-		 *  dirname/filename
-		 *  dirname/*
-		 *  dirname/
-		 *
-		 * The first case is an exact match, the second case is a
-		 * direct match of everything in that directory, and the third
-		 * is a direct match of everything in that directory and its
-		 * subdirectories.
-		 *
-		 * The first two cases are handled above, the code below is
-		 * only for that latter case, so if file doesn't end in /,
-		 * skip to the next file.
-		 */
-		if file[len(file)-1] != '/' {
-			continue
-		}
-
-		/* Remove / because we add it again below */
-		file = file[:len(file)-1]
-
-		/* Maximum tree depth, as calculated by
-		 * $(( `git ls-files | tr -d "[a-z][A-Z][0-9]\-\_\." | \
-		 *     sort -u | tail -1 | wc -c` - 1 ))
-		 * 11
-		 */
-		max_depth := 11
-
-		for i := 0; i < max_depth; i++ {
-			/* Subdirectory match */
-			file += "/*"
-
-			if matched, err = filepath.Match(file, fname); err != nil {
-				return false, err
-			}
-			if matched {
-				return true, nil
-			}
-
+func match_file(fname string, component subsystem) bool {
+	for _, glob := range component.globs {
+		if glob.Match(fname) {
+			return true
 		}
 	}
-	return false, nil
+	return false
 }
 
 func find_maintainer(fname string) {
 	success := false
 	for _, subsystem := range subsystems {
-		matched, err := match_file(fname, subsystem.file)
-		if err != nil {
-			log.Fatalf("match_file failed: %v", err)
-			return
-		}
+		matched := match_file(fname, subsystem)
 		if matched && subsystem.name != "THE REST" {
 			success = true
 			fmt.Println(fname, "is in subsystem",
@@ -228,11 +183,7 @@
 func find_unmaintained(fname string) {
 	success := false
 	for _, subsystem := range subsystems {
-		matched, err := match_file(fname, subsystem.file)
-		if err != nil {
-			log.Fatalf("match_file failed: %v", err)
-			return
-		}
+		matched := match_file(fname, subsystem)
 		if matched && subsystem.name != "THE REST" {
 			success = true
 			fmt.Println(fname, "is in subsystem",

-- 
To view, visit https://review.coreboot.org/29603
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7e5cf4269415269552e35f2c73952ce3dff487e1
Gerrit-Change-Number: 29603
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Georgi <pgeorgi at google.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20181112/919fdf22/attachment-0001.html>