Remove repos package (#1191)

Co-authored-by: Azeem Shaikh <[email protected]>

Makefile

@@ -143,7 +143,7 @@ build-webhook: ## Runs go build on the cron webhook
 
 build-add-script: ## Runs go build on the add script
 build-add-script: cron/data/add/add
-cron/data/add/add: cron/data/add/*.go cron/data/*.go repos/*.go cron/data/projects.csv
+cron/data/add/add: cron/data/add/*.go cron/data/*.go cron/data/projects.csv
 	# Run go build on the add script
 	cd cron/data/add && CGO_ENABLED=0 go build -trimpath -a -ldflags '$(LDFLAGS)' -o add
 

cmd/root.go

@@ -39,11 +39,10 @@ import (
 	sce "github.com/ossf/scorecard/v3/errors"
 	"github.com/ossf/scorecard/v3/pkg"
 	spol "github.com/ossf/scorecard/v3/policy"
-	"github.com/ossf/scorecard/v3/repos"
 )
 
 var (
-	repo        repos.RepoURI
+	repo        string
 	checksToRun []string
 	metaData    []string
 	// This one has to use goflag instead of pflag because it's defined by zap.
@@ -139,23 +138,16 @@ func validateFormat(format string) bool {
 	}
 }
 
-func createRepoClient(ctx context.Context, uri *repos.RepoURI, logger *zap.Logger) (clients.RepoClient, error) {
-	var rc clients.RepoClient
-	switch uri.RepoType() {
-	// URL.
-	case repos.RepoTypeURL:
-		if err := repo.IsValidGitHubURL(); err != nil {
-			return rc, sce.WithMessage(sce.ErrScorecardInternal, err.Error())
-		}
-		rc = githubrepo.CreateGithubRepoClient(ctx, logger)
-		return rc, nil
-
-	// Local directory.
-	case repos.RepoTypeLocalDir:
-		return localdir.CreateLocalDirClient(ctx, logger), nil
+func getRepoAccessors(ctx context.Context, uri string, logger *zap.Logger) (clients.Repo, clients.RepoClient, error) {
+	if repo, err := localdir.MakeLocalDirRepo(uri); err == nil {
+		// Local directory.
+		return repo, localdir.CreateLocalDirClient(ctx, logger), nil
 	}
-
-	return nil, sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("unspported URI: %v", uri.RepoType()))
+	if repo, err := githubrepo.MakeGithubRepo(uri); err == nil {
+		// GitHub URL.
+		return repo, githubrepo.CreateGithubRepoClient(ctx, logger), nil
+	}
+	return nil, nil, sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("unspported URI: %s", uri))
 }
 
 var rootCmd = &cobra.Command{
@@ -223,7 +215,7 @@ var rootCmd = &cobra.Command{
 		// nolint
 		defer logger.Sync() // Flushes buffer, if any.
 
-		repoClient, err := createRepoClient(ctx, &repo, logger)
+		repoURI, repoClient, err := getRepoAccessors(ctx, repo, logger)
 		if err != nil {
 			log.Fatal(err)
 		}
@@ -240,7 +232,7 @@ var rootCmd = &cobra.Command{
 			}
 		}
 
-		repoResult, err := pkg.RunScorecards(ctx, &repo, enabledChecks, repoClient)
+		repoResult, err := pkg.RunScorecards(ctx, repoURI, enabledChecks, repoClient)
 		if err != nil {
 			log.Fatal(err)
 		}
@@ -408,7 +400,7 @@ func enableCheck(checkName string, enabledChecks *checker.CheckNameToFnMap) bool
 func init() {
 	// Add the zap flag manually
 	rootCmd.PersistentFlags().AddGoFlagSet(goflag.CommandLine)
-	rootCmd.Flags().Var(&repo, "repo", "repository to check")
+	rootCmd.Flags().StringVar(&repo, "repo", "", "repository to check")
 	rootCmd.Flags().StringVar(
 		&npm, "npm", "",
 		"npm package to check, given that the npm package has a GitHub repository")

cmd/serve.go

@@ -27,7 +27,6 @@ import (
 	"github.com/ossf/scorecard/v3/checks"
 	"github.com/ossf/scorecard/v3/clients/githubrepo"
 	"github.com/ossf/scorecard/v3/pkg"
-	"github.com/ossf/scorecard/v3/repos"
 )
 
 //nolint:gochecknoinits
@@ -59,13 +58,13 @@ var serveCmd = &cobra.Command{
 			if len(s) != length {
 				rw.WriteHeader(http.StatusBadRequest)
 			}
-			repo := repos.RepoURI{}
-			if err := repo.Set(repoParam); err != nil {
+			repo, err := githubrepo.MakeGithubRepo(repoParam)
+			if err != nil {
 				rw.WriteHeader(http.StatusBadRequest)
 			}
 			ctx := r.Context()
 			repoClient := githubrepo.CreateGithubRepoClient(ctx, logger)
-			repoResult, err := pkg.RunScorecards(ctx, &repo, checks.AllChecks, repoClient)
+			repoResult, err := pkg.RunScorecards(ctx, repo, checks.AllChecks, repoClient)
 			if err != nil {
 				sugar.Error(err)
 				rw.WriteHeader(http.StatusInternalServerError)

cron/controller/main.go

@@ -59,7 +59,7 @@ func publishToRepoRequestTopic(ctx context.Context, iter data.Iterator, datetime
 		if err != nil {
 			return shardNum, fmt.Errorf("error reading repoURL: %w", err)
 		}
-		request.Repos = append(request.GetRepos(), repoURL.URL())
+		request.Repos = append(request.GetRepos(), repoURL.Repo)
 		if len(request.GetRepos()) < shardSize {
 			continue
 		}

cron/data/add/main.go

@@ -21,7 +21,6 @@ import (
 	"os"
 
 	"github.com/ossf/scorecard/v3/cron/data"
-	"github.com/ossf/scorecard/v3/repos"
 )
 
 // Script to add new project repositories to the projects.csv file:
@@ -64,34 +63,32 @@ func main() {
 	}
 }
 
-func getRepoURLs(iter data.Iterator) ([]repos.RepoURI, error) {
-	repoURLs := make(map[string]*repos.RepoURI)
+func getRepoURLs(iter data.Iterator) ([]data.RepoFormat, error) {
+	repoURLs := make(map[string]*data.RepoFormat)
 	repoMap := make(map[string]map[string]bool)
 	for iter.HasNext() {
 		repo, err := iter.Next()
 		if err != nil {
 			return nil, fmt.Errorf("iter.Next: %w", err)
 		}
-		if _, ok := repoMap[repo.URL()]; !ok {
-			repoURLs[repo.URL()] = new(repos.RepoURI)
-			*repoURLs[repo.URL()] = repo
-			repoMap[repo.URL()] = make(map[string]bool)
-			for _, metadata := range repo.Metadata() {
-				repoMap[repo.URL()][metadata] = true
+		if _, ok := repoMap[repo.Repo]; !ok {
+			repoURLs[repo.Repo] = new(data.RepoFormat)
+			*repoURLs[repo.Repo] = repo
+			repoMap[repo.Repo] = make(map[string]bool)
+			for _, metadata := range repo.Metadata {
+				repoMap[repo.Repo][metadata] = true
 			}
 			continue
 		}
-		for _, metadata := range repo.Metadata() {
-			if _, ok := repoMap[repo.URL()][metadata]; !ok && metadata != "" {
-				if err := repoURLs[repo.URL()].AppendMetadata(metadata); err != nil {
-					return nil, fmt.Errorf("AppendMetadata: %w", err)
-				}
-				repoMap[repo.URL()][metadata] = true
+		for _, metadata := range repo.Metadata {
+			if _, ok := repoMap[repo.Repo][metadata]; !ok && metadata != "" {
+				repoURLs[repo.Repo].Metadata = append(repoURLs[repo.Repo].Metadata, metadata)
+				repoMap[repo.Repo][metadata] = true
 			}
 		}
 	}
 
-	newRepoURLs := make([]repos.RepoURI, 0)
+	newRepoURLs := make([]data.RepoFormat, 0)
 	for _, repoURL := range repoURLs {
 		newRepoURLs = append(newRepoURLs, *repoURL)
 	}

cron/data/add/main_test.go

@@ -17,119 +17,89 @@ package main
 import (
 	"fmt"
 	"os"
-	"reflect"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 
 	"github.com/ossf/scorecard/v3/cron/data"
-	"github.com/ossf/scorecard/v3/repos"
 )
 
-type fields struct {
-	host     string
-	owner    string
-	repo     string
-	metadata []string
-}
-
-//nolint: gocritic
-func lessThanURI(x, y repos.RepoURI) bool {
+func lessThanURI(x, y data.RepoFormat) bool {
 	return fmt.Sprintf("%+v", x) < fmt.Sprintf("%+v", y)
 }
 
 func TestGetRepoURLs(t *testing.T) {
 	t.Parallel()
 	testcases := []struct {
 		name, filename string
-		outcome        []fields
+		outcome        []data.RepoFormat
 	}{
 		{
 			name:     "NoChange",
 			filename: "testdata/no_change.csv",
-			outcome: []fields{
+			outcome: []data.RepoFormat{
 				{
-					host:     "github.com",
-					owner:    "owner1",
-					repo:     "repo1",
-					metadata: []string{"meta1", "meta2"},
+					Repo:     "github.com/owner1/repo1",
+					Metadata: []string{"meta1", "meta2"},
 				},
 				{
-					host:  "github.com",
-					owner: "owner2",
-					repo:  "repo2",
+					Repo: "github.com/owner2/repo2",
 				},
 			},
 		},
 		{
 			name:     "AddMetadata",
 			filename: "testdata/add_metadata.csv",
-			outcome: []fields{
+			outcome: []data.RepoFormat{
 				{
-					host:     "github.com",
-					owner:    "owner1",
-					repo:     "repo1",
-					metadata: []string{"meta1", "meta2"},
+					Repo:     "github.com/owner1/repo1",
+					Metadata: []string{"meta1", "meta2"},
 				},
 				{
-					host:     "github.com",
-					owner:    "owner2",
-					repo:     "repo2",
-					metadata: []string{"meta1"},
+					Repo:     "github.com/owner2/repo2",
+					Metadata: []string{"meta1"},
 				},
 			},
 		},
 		{
 			name:     "SkipLatest",
 			filename: "testdata/skip_latest.csv",
-			outcome: []fields{
+			outcome: []data.RepoFormat{
 				{
-					host:     "github.com",
-					owner:    "owner1",
-					repo:     "repo1",
-					metadata: []string{"meta1", "meta2"},
+					Repo:     "github.com/owner1/repo1",
+					Metadata: []string{"meta1", "meta2"},
 				},
 				{
-					host:  "github.com",
-					owner: "owner2",
-					repo:  "repo2",
+					Repo: "github.com/owner2/repo2",
 				},
 			},
 		},
 		{
 			name:     "SkipEmpty",
 			filename: "testdata/skip_empty.csv",
-			outcome: []fields{
+			outcome: []data.RepoFormat{
 				{
-					host:     "github.com",
-					owner:    "owner1",
-					repo:     "repo1",
-					metadata: []string{"meta1", "meta2"},
+					Repo:     "github.com/owner1/repo1",
+					Metadata: []string{"meta1", "meta2"},
 				},
 				{
-					host:     "github.com",
-					owner:    "owner2",
-					repo:     "repo2",
-					metadata: []string{"meta3"},
+					Repo:     "github.com/owner2/repo2",
+					Metadata: []string{"meta3"},
 				},
 			},
 		},
 		{
 			name:     "SkipEmpty_2",
 			filename: "testdata/skip_empty_2.csv",
-			outcome: []fields{
+			outcome: []data.RepoFormat{
 				{
-					host:     "github.com",
-					owner:    "owner1",
-					repo:     "repo1",
-					metadata: []string{"meta1", "meta2"},
+					Repo:     "github.com/owner1/repo1",
+					Metadata: []string{"meta1", "meta2"},
 				},
 				{
-					host:     "github.com",
-					owner:    "owner2",
-					repo:     "repo2",
-					metadata: []string{"meta3"},
+					Repo:     "github.com/owner2/repo2",
+					Metadata: []string{"meta3"},
 				},
 			},
 		},
@@ -155,27 +125,8 @@ func TestGetRepoURLs(t *testing.T) {
 				t.Errorf("testcase failed: %v", err)
 			}
 
-			// Create the list of RepoURL from the outcome.
-			var rs []repos.RepoURI
-			for _, r := range testcase.outcome {
-				u := fmt.Sprintf("%s/%s/%s", r.host, r.owner, r.repo)
-				outcomeRepo, err := repos.NewFromURL(u)
-				if err != nil {
-					t.Errorf("repos.NewFromURL: %v", err)
-				}
-				if err := outcomeRepo.AppendMetadata(r.metadata...); err != nil {
-					t.Errorf("outcomeRepo.AppendMetadata: %v", err)
-				}
-				rs = append(rs, *outcomeRepo)
-			}
-
-			// Export all private fields for comparison.
-			exp := cmp.Exporter(func(t reflect.Type) bool {
-				return true
-			})
-
-			if !cmp.Equal(rs, repoURLs, exp, cmpopts.EquateEmpty(), cmpopts.SortSlices(lessThanURI)) {
-				t.Errorf("testcase failed. expected %+v, got %+v", rs, repoURLs)
+			if !cmp.Equal(testcase.outcome, repoURLs, cmpopts.EquateEmpty(), cmpopts.SortSlices(lessThanURI)) {
+				t.Errorf("testcase failed. expected equal, got diff: %s", cmp.Diff(testcase.outcome, repoURLs))
 			}
 		})
 	}

cron/data/format.go

@@ -16,13 +16,16 @@ package data
 
 import "strings"
 
-type csvStrings []string
+// CSVStrings is []string with support for CSV formatting.
+type CSVStrings []string
 
-func (s csvStrings) MarshalCSV() ([]byte, error) {
+// MarshalCSV implements []string -> []byte serialization.
+func (s CSVStrings) MarshalCSV() ([]byte, error) {
 	return []byte(strings.Join(s, ",")), nil
 }
 
-func (s *csvStrings) UnmarshalCSV(input []byte) error {
+// UnmarshalCSV implements []byte -> []string de-serializtion.
+func (s *CSVStrings) UnmarshalCSV(input []byte) error {
 	if len(input) == 0 || string(input) == "" {
 		*s = nil
 		return nil
@@ -31,7 +34,8 @@ func (s *csvStrings) UnmarshalCSV(input []byte) error {
 	return nil
 }
 
-type repoFormat struct {
+// RepoFormat is used to read input repos.
+type RepoFormat struct {
 	Repo     string     `csv:"repo"`
-	Metadata csvStrings `csv:"metadata"`
+	Metadata CSVStrings `csv:"metadata"`
 }

cron/data/format_test.go

@@ -25,7 +25,7 @@ func TestUnmarshalCsv(t *testing.T) {
 	testcases := []struct {
 		name   string
 		input  []byte
-		output csvStrings
+		output CSVStrings
 	}{
 		{
 			name:   "Basic",
@@ -53,7 +53,7 @@ func TestUnmarshalCsv(t *testing.T) {
 		testcase := testcase
 		t.Run(testcase.name, func(t *testing.T) {
 			t.Parallel()
-			s := new(csvStrings)
+			s := new(CSVStrings)
 			if err := s.UnmarshalCSV(testcase.input); err != nil {
 				t.Errorf("testcase failed: %v", err)
 			}