fixup! FindLicense: implemented license finding via Go gRPC

1 files changed, 45 insertions, 51 deletions

diff --git a/internal/gitaly/service/repository/license.go b/internal/gitaly/service/repository/license.go
index 6db404641..ee28b3d8d 100644
--- a/internal/gitaly/service/repository/license.go
+++ b/internal/gitaly/service/repository/license.go
@@ -6,15 +6,14 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"os/exec"
 	"strings"
 
 	"github.com/go-enry/go-license-detector/v4/licensedb"
 	"github.com/go-enry/go-license-detector/v4/licensedb/api"
 	"github.com/go-enry/go-license-detector/v4/licensedb/filer"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
+	"gitlab.com/gitlab-org/gitaly/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/internal/git/lstree"
-	"gitlab.com/gitlab-org/gitaly/internal/git/repository"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
@@ -23,12 +22,22 @@ import (
 
 func (s *server) FindLicense(ctx context.Context, req *gitalypb.FindLicenseRequest) (*gitalypb.FindLicenseResponse, error) {
 	if featureflag.IsEnabled(ctx, featureflag.GoFindLicense) {
-		repo := req.GetRepository()
-		if repo == nil {
+		if req.GetRepository() == nil {
 			return &gitalypb.FindLicenseResponse{}, nil
 		}
-		repoFiler := &gitFiler{s.gitCmdFactory, ctx, repo}
+		repo := localrepo.New(s.gitCmdFactory, req.GetRepository(), s.cfg)
+
+		hasHeadRevision, err := repo.HasRevision(ctx, "HEAD")
+		if err != nil {
+			return nil, helper.ErrInternalf("cannot check HEAD revision: %v", err)
+		}
+		if !hasHeadRevision {
+			return &gitalypb.FindLicenseResponse{}, nil
+		}
+
+		repoFiler := &gitFiler{ctx, repo}
 		defer repoFiler.Close()
+
 		licenses, err := licensedb.Detect(repoFiler)
 		if err != nil {
 			if errors.Is(err, licensedb.ErrNoLicenseFound) {
@@ -36,6 +45,7 @@ func (s *server) FindLicense(ctx context.Context, req *gitalypb.FindLicenseReque
 			}
 			return nil, helper.ErrInternal(fmt.Errorf("FindLicense: Err: %w", err))
 		}
+
 		var result string
 		var best api.Match
 		for candidate, match := range licenses {
@@ -43,6 +53,7 @@ func (s *server) FindLicense(ctx context.Context, req *gitalypb.FindLicenseReque
 				result = candidate
 			}
 		}
+
 		return &gitalypb.FindLicenseResponse{LicenseShortName: strings.ToLower(result)}, nil
 	}
 
@@ -58,26 +69,23 @@ func (s *server) FindLicense(ctx context.Context, req *gitalypb.FindLicenseReque
 }
 
 type gitFiler struct {
-	commander git.CommandFactory
-	ctx       context.Context
-	repo      repository.GitRepo
+	ctx  context.Context
+	repo *localrepo.Repo
 }
 
 func (f *gitFiler) ReadFile(path string) (content []byte, err error) {
 	if path == "" {
 		return nil, licensedb.ErrNoLicenseFound
 	}
-	var stdout bytes.Buffer
-	cmd, err := f.commander.New(f.ctx, f.repo, git.SubCmd{
+
+	var stdout, stderr bytes.Buffer
+	if err := f.repo.ExecAndWait(f.ctx, git.SubCmd{
 		Name: "cat-file",
 		Args: []string{"blob", fmt.Sprintf(":%s", path)},
-	}, git.WithStdout(&stdout))
-	if err != nil {
-		return nil, err
-	}
-	if err := cmd.Wait(); err != nil {
-		return nil, err
+	}, git.WithStdout(&stdout), git.WithStderr(&stderr)); err != nil {
+		return nil, fmt.Errorf("cat-file failed: %w, stderr: %q", err, stderr.String())
 	}
+
 	return stdout.Bytes(), nil
 }
 
@@ -86,7 +94,11 @@ func (f *gitFiler) ReadDir(path string) ([]filer.File, error) {
 	if dotPath == "" {
 		dotPath = "."
 	}
-	cmd, err := f.commander.New(f.ctx, f.repo, git.SubCmd{
+
+	// We're doing a recursive listing returning all files at once such that we do not have to
+	// call git-ls-tree(1) multiple times.
+	var stderr bytes.Buffer
+	cmd, err := f.repo.Exec(f.ctx, git.SubCmd{
 		Name: "ls-tree",
 		Flags: []git.Option{
 			git.Flag{Name: "--full-tree"},
@@ -94,13 +106,14 @@ func (f *gitFiler) ReadDir(path string) ([]filer.File, error) {
 			git.Flag{Name: "-r"},
 		},
 		Args: []string{"HEAD", dotPath},
-	})
+	}, git.WithStderr(&stderr))
 	if err != nil {
 		return nil, err
 	}
+
 	tree := lstree.NewParser(cmd)
-	out := make([]filer.File, 0)
-	skip := len(path)
+
+	var files []filer.File
 	for {
 		entry, err := tree.NextEntry()
 		if err != nil {
@@ -109,30 +122,24 @@ func (f *gitFiler) ReadDir(path string) ([]filer.File, error) {
 			}
 			return nil, err
 		}
+
+		// Given that we're doing a recursive listing, we skip over all types which aren't
+		// blobs.
 		if entry.Type != lstree.Blob {
 			continue
 		}
-		f := entryToFile(skip, entry)
-		var duplicate bool
-		for _, o := range out {
-			if o.Name == f.Name {
-				duplicate = true
-				break
-			}
-		}
-		if !duplicate {
-			out = append(out, f)
-		}
+
+		files = append(files, filer.File{
+			Name:  entry.Path,
+			IsDir: false,
+		})
 	}
+
 	if err := cmd.Wait(); err != nil {
-		exitError := &exec.ExitError{}
-		if errors.As(err, &exitError) && exitError.ProcessState.ExitCode() > 0 {
-			// not a git repository
-			return []filer.File{}, nil
-		}
-		return nil, fmt.Errorf("cmd failed: %w", err)
+		return nil, fmt.Errorf("ls-tree failed: %w, stderr: %q", err, stderr.String())
 	}
-	return out, nil
+
+	return files, nil
 }
 
 func (f *gitFiler) Close() {}
@@ -141,16 +148,3 @@ func (f *gitFiler) PathsAreAlwaysSlash() bool {
 	// git ls-files uses unix slash `/`
 	return true
 }
-
-func entryToFile(skip int, entry *lstree.Entry) filer.File {
-	path := entry.Path[skip:]
-	idx := strings.Index(path, "/")
-	var f filer.File
-	if idx > 0 {
-		f.Name = path[:idx]
-		f.IsDir = true
-	} else {
-		f.Name = path
-	}
-	return f
-}