diff options
| -rw-r--r-- | WORKSPACE | 21 | ||||
| -rw-r--r-- | go.mod | 2 | ||||
| -rw-r--r-- | go.sum | 2 | ||||
| -rw-r--r-- | tools/issue_reviver/BUILD | 12 | ||||
| -rw-r--r-- | tools/issue_reviver/github/BUILD | 17 | ||||
| -rw-r--r-- | tools/issue_reviver/github/github.go | 164 | ||||
| -rw-r--r-- | tools/issue_reviver/main.go | 89 | ||||
| -rw-r--r-- | tools/issue_reviver/reviver/BUILD | 19 | ||||
| -rw-r--r-- | tools/issue_reviver/reviver/reviver.go | 192 | ||||
| -rw-r--r-- | tools/issue_reviver/reviver/reviver_test.go | 88 |
10 files changed, 606 insertions, 0 deletions
@@ -290,6 +290,27 @@ go_repository( version = "v1.3.1", ) +go_repository( + name = "com_github_google_go-github", + importpath = "github.com/google/go-github", + sum = "h1:N0LgJ1j65A7kfXrZnUDaYCs/Sf4rEjNlfyDHW9dolSY=", + version = "v17.0.0", +) + +go_repository( + name = "org_golang_x_oauth2", + importpath = "golang.org/x/oauth2", + sum = "h1:pE8b58s1HRDMi8RDc79m0HISf9D4TzseP40cEA6IGfs=", + version = "v0.0.0-20191202225959-858c2ad4c8b6", +) + +go_repository( + name = "com_github_google_go-querystring", + importpath = "github.com/google/go-querystring", + sum = "h1:Xkwi/a1rcvNg1PPYe5vI8GbeBY/jrVuDX5ASuANWTrk=", + version = "v1.0.0", +) + # System Call test dependencies. http_archive( name = "com_google_absl", @@ -9,6 +9,7 @@ require ( github.com/golang/protobuf v1.3.1 github.com/google/btree v1.0.0 github.com/google/go-cmp v0.2.0 + github.com/google/go-github/v28 v28.1.1 github.com/google/subcommands v0.0.0-20190508160503-636abe8753b8 github.com/google/uuid v0.0.0-20171129191014-dec09d789f3d github.com/kr/pty v1.1.1 @@ -17,5 +18,6 @@ require ( github.com/vishvananda/netlink v1.0.1-0.20190318003149-adb577d4a45e github.com/vishvananda/netns v0.0.0-20171111001504-be1fbeda1936 golang.org/x/net v0.0.0-20190311183353-d8887717615a + golang.org/x/oauth2 v0.0.0-20191202225959-858c2ad4c8b6 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a ) @@ -4,6 +4,7 @@ github.com/golang/mock v1.3.1/go.mod h1:sBzyDLLjw3U8JLTeZvSv8jJB+tU5PVekmnlKIyFU github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= +github.com/google/go-github/v28 v28.1.1/go.mod h1:bsqJWQX05omyWVmc00nEUql9mhQyv38lDZ8kPZcQVoM= github.com/google/subcommands v0.0.0-20190508160503-636abe8753b8/go.mod h1:ZjhPrFU+Olkh9WazFPsl27BQ4UPiG37m3yTrtFlrHVk= github.com/google/uuid v0.0.0-20171129191014-dec09d789f3d/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= @@ -13,6 +14,7 @@ github.com/vishvananda/netlink v1.0.1-0.20190318003149-adb577d4a45e/go.mod h1:+S github.com/vishvananda/netns v0.0.0-20171111001504-be1fbeda1936/go.mod h1:ZjcWmFBXmLKZu9Nxj3WKYEafiSqer2rnvPr0en9UNpI= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/oauth2 v0.0.0-20191202225959-858c2ad4c8b6/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= diff --git a/tools/issue_reviver/BUILD b/tools/issue_reviver/BUILD new file mode 100644 index 000000000..ee7ea11fd --- /dev/null +++ b/tools/issue_reviver/BUILD @@ -0,0 +1,12 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_binary") + +package(licenses = ["notice"]) + +go_binary( + name = "issue_reviver", + srcs = ["main.go"], + deps = [ + "//tools/issue_reviver/github", + "//tools/issue_reviver/reviver", + ], +) diff --git a/tools/issue_reviver/github/BUILD b/tools/issue_reviver/github/BUILD new file mode 100644 index 000000000..6da22ba1c --- /dev/null +++ b/tools/issue_reviver/github/BUILD @@ -0,0 +1,17 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +package(licenses = ["notice"]) + +go_library( + name = "github", + srcs = ["github.go"], + importpath = "gvisor.dev/gvisor/tools/issue_reviver/github", + visibility = [ + "//tools/issue_reviver:__subpackages__", + ], + deps = [ + "//tools/issue_reviver/reviver", + "@com_github_google_go-github//github:go_default_library", + "@org_golang_x_oauth2//:go_default_library", + ], +) diff --git a/tools/issue_reviver/github/github.go b/tools/issue_reviver/github/github.go new file mode 100644 index 000000000..e07949c8f --- /dev/null +++ b/tools/issue_reviver/github/github.go @@ -0,0 +1,164 @@ +// Copyright 2019 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package github implements reviver.Bugger interface on top of Github issues. +package github + +import ( + "context" + "fmt" + "strconv" + "strings" + "time" + + "github.com/google/go-github/github" + "golang.org/x/oauth2" + "gvisor.dev/gvisor/tools/issue_reviver/reviver" +) + +// Bugger implements reviver.Bugger interface for github issues. +type Bugger struct { + owner string + repo string + dryRun bool + + client *github.Client + issues map[int]*github.Issue +} + +// NewBugger creates a new Bugger. +func NewBugger(token, owner, repo string, dryRun bool) (*Bugger, error) { + b := &Bugger{ + owner: owner, + repo: repo, + dryRun: dryRun, + issues: map[int]*github.Issue{}, + } + if err := b.load(token); err != nil { + return nil, err + } + return b, nil +} + +func (b *Bugger) load(token string) error { + ctx := context.Background() + if len(token) == 0 { + fmt.Print("No OAUTH token provided, using unauthenticated account.\n") + b.client = github.NewClient(nil) + } else { + ts := oauth2.StaticTokenSource( + &oauth2.Token{AccessToken: token}, + ) + tc := oauth2.NewClient(ctx, ts) + b.client = github.NewClient(tc) + } + + err := processAllPages(func(listOpts github.ListOptions) (*github.Response, error) { + opts := &github.IssueListByRepoOptions{State: "open", ListOptions: listOpts} + tmps, resp, err := b.client.Issues.ListByRepo(ctx, b.owner, b.repo, opts) + if err != nil { + return resp, err + } + for _, issue := range tmps { + b.issues[issue.GetNumber()] = issue + } + return resp, nil + }) + if err != nil { + return err + } + + fmt.Printf("Loaded %d issues from github.com/%s/%s\n", len(b.issues), b.owner, b.repo) + return nil +} + +// Activate implements reviver.Bugger. +func (b *Bugger) Activate(todo *reviver.Todo) (bool, error) { + const prefix = "gvisor.dev/issue/" + + // First check if I can handle the TODO. + idStr := strings.TrimPrefix(todo.Issue, prefix) + if len(todo.Issue) == len(idStr) { + return false, nil + } + + id, err := strconv.Atoi(idStr) + if err != nil { + return true, err + } + + // Check against active issues cache. + if _, ok := b.issues[id]; ok { + fmt.Printf("%q is active: OK\n", todo.Issue) + return true, nil + } + + fmt.Printf("%q is not active: reopening issue %d\n", todo.Issue, id) + + // Format comment with TODO locations and search link. + comment := strings.Builder{} + fmt.Fprintln(&comment, "There are TODOs still referencing this issue:") + for _, l := range todo.Locations { + fmt.Fprintf(&comment, + "1. [%s:%d](https://github.com/%s/%s/blob/HEAD/%s#%d): %s\n", + l.File, l.Line, b.owner, b.repo, l.File, l.Line, l.Comment) + } + fmt.Fprintf(&comment, + "\n\nSearch [TODO](https://github.com/%s/%s/search?q=%%22%s%d%%22)", b.owner, b.repo, prefix, id) + + if b.dryRun { + fmt.Printf("[dry-run: skipping change to issue %d]\n%s\n=======================\n", id, comment.String()) + return true, nil + } + + ctx := context.Background() + req := &github.IssueRequest{State: github.String("open")} + _, _, err = b.client.Issues.Edit(ctx, b.owner, b.repo, id, req) + if err != nil { + return true, fmt.Errorf("failed to reactivate issue %d: %v", id, err) + } + + cmt := &github.IssueComment{ + Body: github.String(comment.String()), + Reactions: &github.Reactions{Confused: github.Int(1)}, + } + if _, _, err := b.client.Issues.CreateComment(ctx, b.owner, b.repo, id, cmt); err != nil { + return true, fmt.Errorf("failed to add comment to issue %d: %v", id, err) + } + + return true, nil +} + +func processAllPages(fn func(github.ListOptions) (*github.Response, error)) error { + opts := github.ListOptions{PerPage: 1000} + for { + resp, err := fn(opts) + if err != nil { + if rateErr, ok := err.(*github.RateLimitError); ok { + duration := rateErr.Rate.Reset.Sub(time.Now()) + if duration > 5*time.Minute { + return fmt.Errorf("Rate limited for too long: %v", duration) + } + fmt.Printf("Rate limited, sleeping for: %v\n", duration) + time.Sleep(duration) + continue + } + return err + } + if resp.NextPage == 0 { + return nil + } + opts.Page = resp.NextPage + } +} diff --git a/tools/issue_reviver/main.go b/tools/issue_reviver/main.go new file mode 100644 index 000000000..4256f5a6c --- /dev/null +++ b/tools/issue_reviver/main.go @@ -0,0 +1,89 @@ +// Copyright 2019 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package main is the entry point for issue_reviver. +package main + +import ( + "flag" + "fmt" + "io/ioutil" + "os" + + "gvisor.dev/gvisor/tools/issue_reviver/github" + "gvisor.dev/gvisor/tools/issue_reviver/reviver" +) + +var ( + owner string + repo string + tokenFile string + path string + dryRun bool +) + +// Keep the options simple for now. Supports only a single path and repo. +func init() { + flag.StringVar(&owner, "owner", "google", "Github project org/owner to look for issues") + flag.StringVar(&repo, "repo", "gvisor", "Github repo to look for issues") + flag.StringVar(&tokenFile, "oauth-token-file", "", "Path to file containing the OAUTH token to be used as credential to github") + flag.StringVar(&path, "path", "", "Path to scan for TODOs") + flag.BoolVar(&dryRun, "dry-run", false, "If set to true, no changes are made to issues") +} + +func main() { + flag.Parse() + + // Check for mandatory parameters. + if len(owner) == 0 { + fmt.Println("missing --owner option.") + flag.Usage() + os.Exit(1) + } + if len(repo) == 0 { + fmt.Println("missing --repo option.") + flag.Usage() + os.Exit(1) + } + if len(path) == 0 { + fmt.Println("missing --path option.") + flag.Usage() + os.Exit(1) + } + + // Token is passed as a file so it doesn't show up in command line arguments. + var token string + if len(tokenFile) != 0 { + bytes, err := ioutil.ReadFile(tokenFile) + if err != nil { + fmt.Println(err.Error()) + os.Exit(1) + } + token = string(bytes) + } + + bugger, err := github.NewBugger(token, owner, repo, dryRun) + if err != nil { + fmt.Fprintln(os.Stderr, "Error getting github issues:", err) + os.Exit(1) + } + rev := reviver.New([]string{path}, []reviver.Bugger{bugger}) + if errs := rev.Run(); len(errs) > 0 { + fmt.Fprintf(os.Stderr, "Encountered %d errors:\n", len(errs)) + for _, err := range errs { + fmt.Fprintf(os.Stderr, "\t%v\n", err) + } + os.Exit(1) + } +} diff --git a/tools/issue_reviver/reviver/BUILD b/tools/issue_reviver/reviver/BUILD new file mode 100644 index 000000000..2c3675977 --- /dev/null +++ b/tools/issue_reviver/reviver/BUILD @@ -0,0 +1,19 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +package(licenses = ["notice"]) + +go_library( + name = "reviver", + srcs = ["reviver.go"], + importpath = "gvisor.dev/gvisor/tools/issue_reviver/reviver", + visibility = [ + "//tools/issue_reviver:__subpackages__", + ], +) + +go_test( + name = "reviver_test", + size = "small", + srcs = ["reviver_test.go"], + embed = [":reviver"], +) diff --git a/tools/issue_reviver/reviver/reviver.go b/tools/issue_reviver/reviver/reviver.go new file mode 100644 index 000000000..682db0c01 --- /dev/null +++ b/tools/issue_reviver/reviver/reviver.go @@ -0,0 +1,192 @@ +// Copyright 2019 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package reviver scans the code looking for TODOs and pass them to registered +// Buggers to ensure TODOs point to active issues. +package reviver + +import ( + "bufio" + "fmt" + "io/ioutil" + "os" + "path/filepath" + "regexp" + "sync" +) + +// This is how a TODO looks like. +var regexTodo = regexp.MustCompile(`(\/\/|#)\s*(TODO|FIXME)\(([a-zA-Z0-9.\/]+)\):\s*(.+)`) + +// Bugger interface is called for every TODO found in the code. If it can handle +// the TODO, it must return true. If it returns false, the next Bugger is +// called. If no Bugger handles the TODO, it's dropped on the floor. +type Bugger interface { + Activate(todo *Todo) (bool, error) +} + +// Location saves the location where the TODO was found. +type Location struct { + Comment string + File string + Line uint +} + +// Todo represents a unique TODO. There can be several TODOs pointing to the +// same issue in the code. They are all grouped together. +type Todo struct { + Issue string + Locations []Location +} + +// Reviver scans the given paths for TODOs and calls Buggers to handle them. +type Reviver struct { + paths []string + buggers []Bugger + + mu sync.Mutex + todos map[string]*Todo + errs []error +} + +// New create a new Reviver. +func New(paths []string, buggers []Bugger) *Reviver { + return &Reviver{ + paths: paths, + buggers: buggers, + todos: map[string]*Todo{}, + } +} + +// Run runs. It returns all errors found during processing, it doesn't stop +// on errors. +func (r *Reviver) Run() []error { + // Process each directory in parallel. + wg := sync.WaitGroup{} + for _, path := range r.paths { + wg.Add(1) + go func(path string) { + defer wg.Done() + r.processPath(path, &wg) + }(path) + } + + wg.Wait() + + r.mu.Lock() + defer r.mu.Unlock() + + fmt.Printf("Processing %d TODOs (%d errors)...\n", len(r.todos), len(r.errs)) + dropped := 0 + for _, todo := range r.todos { + ok, err := r.processTodo(todo) + if err != nil { + r.errs = append(r.errs, err) + } + if !ok { + dropped++ + } + } + fmt.Printf("Processed %d TODOs, %d were skipped (%d errors)\n", len(r.todos)-dropped, dropped, len(r.errs)) + + return r.errs +} + +func (r *Reviver) processPath(path string, wg *sync.WaitGroup) { + fmt.Printf("Processing dir %q\n", path) + fis, err := ioutil.ReadDir(path) + if err != nil { + r.addErr(fmt.Errorf("error processing dir %q: %v", path, err)) + return + } + + for _, fi := range fis { + childPath := filepath.Join(path, fi.Name()) + switch { + case fi.Mode().IsDir(): + wg.Add(1) + go func() { + defer wg.Done() + r.processPath(childPath, wg) + }() + + case fi.Mode().IsRegular(): + file, err := os.Open(childPath) + if err != nil { + r.addErr(err) + continue + } + + scanner := bufio.NewScanner(file) + lineno := uint(0) + for scanner.Scan() { + lineno++ + line := scanner.Text() + if todo := r.processLine(line, childPath, lineno); todo != nil { + r.addTodo(todo) + } + } + } + } +} + +func (r *Reviver) processLine(line, path string, lineno uint) *Todo { + matches := regexTodo.FindStringSubmatch(line) + if matches == nil { + return nil + } + if len(matches) != 5 { + panic(fmt.Sprintf("regex returned wrong matches for %q: %v", line, matches)) + } + return &Todo{ + Issue: matches[3], + Locations: []Location{ + { + File: path, + Line: lineno, + Comment: matches[4], + }, + }, + } +} + +func (r *Reviver) addTodo(newTodo *Todo) { + r.mu.Lock() + defer r.mu.Unlock() + + if todo := r.todos[newTodo.Issue]; todo == nil { + r.todos[newTodo.Issue] = newTodo + } else { + todo.Locations = append(todo.Locations, newTodo.Locations...) + } +} + +func (r *Reviver) addErr(err error) { + r.mu.Lock() + defer r.mu.Unlock() + r.errs = append(r.errs, err) +} + +func (r *Reviver) processTodo(todo *Todo) (bool, error) { + for _, bugger := range r.buggers { + ok, err := bugger.Activate(todo) + if err != nil { + return false, err + } + if ok { + return true, nil + } + } + return false, nil +} diff --git a/tools/issue_reviver/reviver/reviver_test.go b/tools/issue_reviver/reviver/reviver_test.go new file mode 100644 index 000000000..a9fb1f9f1 --- /dev/null +++ b/tools/issue_reviver/reviver/reviver_test.go @@ -0,0 +1,88 @@ +// Copyright 2019 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package reviver + +import ( + "testing" +) + +func TestProcessLine(t *testing.T) { + for _, tc := range []struct { + line string + want *Todo + }{ + { + line: "// TODO(foobar.com/issue/123): comment, bla. blabla.", + want: &Todo{ + Issue: "foobar.com/issue/123", + Locations: []Location{ + {Comment: "comment, bla. blabla."}, + }, + }, + }, + { + line: "// FIXME(b/123): internal bug", + want: &Todo{ + Issue: "b/123", + Locations: []Location{ + {Comment: "internal bug"}, + }, + }, + }, + { + line: "TODO(issue): not todo", + }, + { + line: "FIXME(issue): not todo", + }, + { + line: "// TODO (issue): not todo", + }, + { + line: "// TODO(issue) not todo", + }, + { + line: "// todo(issue): not todo", + }, + { + line: "// TODO(issue):", + }, + } { + t.Logf("Testing: %s", tc.line) + r := Reviver{} + got := r.processLine(tc.line, "test", 0) + if got == nil { + if tc.want != nil { + t.Errorf("failed to process line, want: %+v", tc.want) + } + } else { + if tc.want == nil { + t.Errorf("expected error, got: %+v", got) + continue + } + if got.Issue != tc.want.Issue { + t.Errorf("wrong issue, got: %v, want: %v", got.Issue, tc.want.Issue) + } + if len(got.Locations) != len(tc.want.Locations) { + t.Errorf("wrong number of locations, got: %v, want: %v, locations: %+v", len(got.Locations), len(tc.want.Locations), got.Locations) + } + for i, wantLoc := range tc.want.Locations { + if got.Locations[i].Comment != wantLoc.Comment { + t.Errorf("wrong comment, got: %v, want: %v", got.Locations[i].Comment, wantLoc.Comment) + } + } + } + } +} |