refactor repo_stats to use os.pipe (#11726)

* refactor repo_stats to use os.pipe

* woops. missing reader.

* stdout not stderr, woops

* Fix copyright date and ensure that the stderr is collected

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: Andrew Thornton <art27@cantab.net>

modules/git/command.go

@@ -1,5 +1,5 @@
-// Copyright 2020 The Gitea Authors. All rights reserved.
 // Copyright 2015 The Gogs Authors. All rights reserved.
+// Copyright 2016 The Gitea Authors. All rights reserved.
 // Use of this source code is governed by a MIT-style
 // license that can be found in the LICENSE file.
 

modules/git/repo_stats.go

@@ -6,8 +6,9 @@ package git
 
 import (
 	"bufio"
-	"bytes"
+	"context"
 	"fmt"
+	"os"
 	"sort"
 	"strconv"
 	"strings"
@@ -49,6 +50,15 @@ func (repo *Repository) GetCodeActivityStats(fromTime time.Time, branch string)
 	}
 	stats.CommitCountInAllBranches = c
 
+	stdoutReader, stdoutWriter, err := os.Pipe()
+	if err != nil {
+		return nil, err
+	}
+	defer func() {
+		_ = stdoutReader.Close()
+		_ = stdoutWriter.Close()
+	}()
+
 	args := []string{"log", "--numstat", "--no-merges", "--pretty=format:---%n%h%n%an%n%ae%n", "--date=iso", fmt.Sprintf("--since='%s'", since)}
 	if len(branch) == 0 {
 		args = append(args, "--branches=*")
@@ -56,79 +66,88 @@ func (repo *Repository) GetCodeActivityStats(fromTime time.Time, branch string)
 		args = append(args, "--first-parent", branch)
 	}
 
-	stdout, err = NewCommand(args...).RunInDirBytes(repo.Path)
+	stderr := new(strings.Builder)
-	if err != nil {
+	err = NewCommand(args...).RunInDirTimeoutEnvFullPipelineFunc(
-		return nil, err
+		nil, -1, repo.Path,
-	}
+		stdoutWriter, stderr, nil,
+		func(ctx context.Context, cancel context.CancelFunc) error {
+			_ = stdoutWriter.Close()
 
-	scanner := bufio.NewScanner(bytes.NewReader(stdout))
+			scanner := bufio.NewScanner(stdoutReader)
-	scanner.Split(bufio.ScanLines)
+			scanner.Split(bufio.ScanLines)
-	stats.CommitCount = 0
+			stats.CommitCount = 0
-	stats.Additions = 0
+			stats.Additions = 0
-	stats.Deletions = 0
+			stats.Deletions = 0
-	authors := make(map[string]*CodeActivityAuthor)
+			authors := make(map[string]*CodeActivityAuthor)
-	files := make(map[string]bool)
+			files := make(map[string]bool)
-	var author string
+			var author string
-	p := 0
+			p := 0
-	for scanner.Scan() {
+			for scanner.Scan() {
-		l := strings.TrimSpace(scanner.Text())
+				l := strings.TrimSpace(scanner.Text())
-		if l == "---" {
+				if l == "---" {
-			p = 1
+					p = 1
-		} else if p == 0 {
+				} else if p == 0 {
-			continue
+					continue
-		} else {
+				} else {
-			p++
+					p++
-		}
-		if p > 4 && len(l) == 0 {
-			continue
-		}
-		switch p {
-		case 1: // Separator
-		case 2: // Commit sha-1
-			stats.CommitCount++
-		case 3: // Author
-			author = l
-		case 4: // E-mail
-			email := strings.ToLower(l)
-			if _, ok := authors[email]; !ok {
-				authors[email] = &CodeActivityAuthor{
-					Name:    author,
-					Email:   email,
-					Commits: 0,
 				}
-			}
+				if p > 4 && len(l) == 0 {
-			authors[email].Commits++
+					continue
-		default: // Changed file
-			if parts := strings.Fields(l); len(parts) >= 3 {
-				if parts[0] != "-" {
-					if c, err := strconv.ParseInt(strings.TrimSpace(parts[0]), 10, 64); err == nil {
-						stats.Additions += c
-					}
 				}
-				if parts[1] != "-" {
+				switch p {
-					if c, err := strconv.ParseInt(strings.TrimSpace(parts[1]), 10, 64); err == nil {
+				case 1: // Separator
-						stats.Deletions += c
+				case 2: // Commit sha-1
+					stats.CommitCount++
+				case 3: // Author
+					author = l
+				case 4: // E-mail
+					email := strings.ToLower(l)
+					if _, ok := authors[email]; !ok {
+						authors[email] = &CodeActivityAuthor{
+							Name:    author,
+							Email:   email,
+							Commits: 0,
+						}
+					}
+					authors[email].Commits++
+				default: // Changed file
+					if parts := strings.Fields(l); len(parts) >= 3 {
+						if parts[0] != "-" {
+							if c, err := strconv.ParseInt(strings.TrimSpace(parts[0]), 10, 64); err == nil {
+								stats.Additions += c
+							}
+						}
+						if parts[1] != "-" {
+							if c, err := strconv.ParseInt(strings.TrimSpace(parts[1]), 10, 64); err == nil {
+								stats.Deletions += c
+							}
+						}
+						if _, ok := files[parts[2]]; !ok {
+							files[parts[2]] = true
+						}
 					}
-				}
-				if _, ok := files[parts[2]]; !ok {
-					files[parts[2]] = true
 				}
 			}
-		}
-	}
 
-	a := make([]*CodeActivityAuthor, 0, len(authors))
+			a := make([]*CodeActivityAuthor, 0, len(authors))
-	for _, v := range authors {
+			for _, v := range authors {
-		a = append(a, v)
+				a = append(a, v)
+			}
+			// Sort authors descending depending on commit count
+			sort.Slice(a, func(i, j int) bool {
+				return a[i].Commits > a[j].Commits
+			})
+
+			stats.AuthorCount = int64(len(authors))
+			stats.ChangedFiles = int64(len(files))
+			stats.Authors = a
+
+			_ = stdoutReader.Close()
+			return nil
+		})
+	if err != nil {
+		return nil, fmt.Errorf("Failed to get GetCodeActivityStats for repository.\nError: %w\nStderr: %s", err, stderr)
 	}
-	// Sort authors descending depending on commit count
-	sort.Slice(a, func(i, j int) bool {
-		return a[i].Commits > a[j].Commits
-	})
-
-	stats.AuthorCount = int64(len(authors))
-	stats.ChangedFiles = int64(len(files))
-	stats.Authors = a
 
 	return stats, nil
 }