diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c8e0f02e6..fa206b0815 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ All notable changes to `src-cli` are documented in this file. ### Fixed +- The evaluation of the [`repository.branch` attribute](https://docs.sourcegraph.com/campaigns/references/campaign_spec_yaml_reference#on-repository) has been fixed to actually cause the correct version of the repository to be used. [#393](https://github.com/sourcegraph/src-cli/pull/393) + ### Removed ## 3.22.3 diff --git a/internal/campaigns/archive_fetcher.go b/internal/campaigns/archive_fetcher.go index e472e64889..69d4a61fb9 100644 --- a/internal/campaigns/archive_fetcher.go +++ b/internal/campaigns/archive_fetcher.go @@ -120,12 +120,11 @@ func fetchRepositoryArchive(ctx context.Context, client api.Client, repo *graphq } func repositoryZipArchivePath(repo *graphql.Repository) string { - return path.Join("", repo.Name+"@"+repo.DefaultBranch.Name, "-", "raw") + return path.Join("", repo.Name+"@"+repo.BaseRef(), "-", "raw") } func localRepositoryZipArchivePath(dir string, repo *graphql.Repository) string { - ref := repo.DefaultBranch.Target.OID - return filepath.Join(dir, fmt.Sprintf("%s-%s.zip", repo.Slug(), ref)) + return filepath.Join(dir, fmt.Sprintf("%s-%s.zip", repo.Slug(), repo.Rev())) } func unzip(zipFile, dest string) error { diff --git a/internal/campaigns/archive_fetcher_test.go b/internal/campaigns/archive_fetcher_test.go index 87f6827f4d..9e536173b7 100644 --- a/internal/campaigns/archive_fetcher_test.go +++ b/internal/campaigns/archive_fetcher_test.go @@ -30,7 +30,7 @@ func TestWorkspaceCreator_Create(t *testing.T) { repo := &graphql.Repository{ ID: "src-cli", Name: "github.com/sourcegraph/src-cli", - DefaultBranch: &graphql.Branch{Name: "main", Target: struct{ OID string }{OID: "d34db33f"}}, + DefaultBranch: &graphql.Branch{Name: "main", Target: graphql.Target{OID: "d34db33f"}}, } archive := mockRepoArchive{ @@ -156,6 +156,44 @@ func TestWorkspaceCreator_Create(t *testing.T) { t.Fatalf("zip file in temp dir was not cleaned up") } }) + + t.Run("non-default branch", func(t *testing.T) { + otherBranchOID := "f00b4r" + repo := &graphql.Repository{ + ID: "src-cli-with-non-main-branch", + Name: "github.com/sourcegraph/src-cli", + DefaultBranch: &graphql.Branch{Name: "main", Target: graphql.Target{OID: "d34db33f"}}, + + Commit: graphql.Target{OID: otherBranchOID}, + Branch: graphql.Branch{Name: "other-branch", Target: graphql.Target{OID: otherBranchOID}}, + } + + archive := mockRepoArchive{repo: repo, files: map[string]string{}} + + ts := httptest.NewServer(newZipArchivesMux(t, nil, archive)) + defer ts.Close() + + var clientBuffer bytes.Buffer + client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) + + testTempDir := workspaceTmpDir(t) + + creator := &WorkspaceCreator{dir: testTempDir, client: client} + + _, err := creator.Create(context.Background(), repo) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + wantZipFile := "github.com-sourcegraph-src-cli-" + otherBranchOID + ".zip" + ok, err := dirContains(creator.dir, wantZipFile) + if err != nil { + t.Fatal(err) + } + if !ok { + t.Fatalf("temp dir doesnt contain zip file") + } + }) } func TestMkdirAll(t *testing.T) { diff --git a/internal/campaigns/executor_test.go b/internal/campaigns/executor_test.go index fd42149697..d69f1d37ea 100644 --- a/internal/campaigns/executor_test.go +++ b/internal/campaigns/executor_test.go @@ -37,14 +37,14 @@ func TestExecutor_Integration(t *testing.T) { srcCLIRepo := &graphql.Repository{ ID: "src-cli", Name: "github.com/sourcegraph/src-cli", - DefaultBranch: &graphql.Branch{Name: "main", Target: struct{ OID string }{OID: "d34db33f"}}, + DefaultBranch: &graphql.Branch{Name: "main", Target: graphql.Target{OID: "d34db33f"}}, } sourcegraphRepo := &graphql.Repository{ ID: "sourcegraph", Name: "github.com/sourcegraph/sourcegraph", DefaultBranch: &graphql.Branch{ Name: "main", - Target: struct{ OID string }{OID: "f00b4r3r"}, + Target: graphql.Target{OID: "f00b4r3r"}, }, } @@ -220,7 +220,7 @@ func newZipArchivesMux(t *testing.T, callback http.HandlerFunc, archives ...mock for _, archive := range archives { files := archive.files - path := fmt.Sprintf("/%s@%s/-/raw", archive.repo.Name, archive.repo.DefaultBranch.Name) + path := fmt.Sprintf("/%s@%s/-/raw", archive.repo.Name, archive.repo.BaseRef()) downloadName := filepath.Base(archive.repo.Name) mediaType := mime.FormatMediaType("Attachment", map[string]string{ diff --git a/internal/campaigns/graphql/repository.go b/internal/campaigns/graphql/repository.go index 6147bae78b..4b1c4d98a8 100644 --- a/internal/campaigns/graphql/repository.go +++ b/internal/campaigns/graphql/repository.go @@ -19,12 +19,19 @@ fragment repositoryFields on Repository { oid } } + commit(rev: $rev) @include(if:$queryCommit) { + oid + } } ` +type Target struct { + OID string +} + type Branch struct { Name string - Target struct{ OID string } + Target Target } type Repository struct { @@ -32,16 +39,41 @@ type Repository struct { Name string URL string ExternalRepository struct{ ServiceType string } - DefaultBranch *Branch + + DefaultBranch *Branch + + Commit Target + // Branch is populated by resolveRepositoryNameAndBranch with the queried + // branch's name and the contents of the Commit property. + Branch Branch FileMatches map[string]bool } +func (r *Repository) HasBranch() bool { + return r.DefaultBranch != nil || (r.Commit.OID != "" && r.Branch.Name != "") +} + func (r *Repository) BaseRef() string { + if r.Branch.Name != "" { + return ensurePrefix(r.Branch.Name) + } + return r.DefaultBranch.Name } +func ensurePrefix(rev string) string { + if strings.HasPrefix(rev, "refs/heads/") { + return rev + } + return "refs/heads/" + rev +} + func (r *Repository) Rev() string { + if r.Branch.Target.OID != "" { + return r.Branch.Target.OID + } + return r.DefaultBranch.Target.OID } diff --git a/internal/campaigns/service.go b/internal/campaigns/service.go index c6f532d1dc..f59cca6c8a 100644 --- a/internal/campaigns/service.go +++ b/internal/campaigns/service.go @@ -363,8 +363,7 @@ func (svc *Service) ResolveNamespace(ctx context.Context, namespace string) (str } func (svc *Service) ResolveRepositories(ctx context.Context, spec *CampaignSpec) ([]*graphql.Repository, error) { - final := []*graphql.Repository{} - seen := map[string]struct{}{} + seen := map[string]*graphql.Repository{} unsupported := UnsupportedRepoSet{} // TODO: this could be trivially parallelised in the future. @@ -375,11 +374,12 @@ func (svc *Service) ResolveRepositories(ctx context.Context, spec *CampaignSpec) } for _, repo := range repos { - if _, ok := seen[repo.ID]; !ok { - if repo.DefaultBranch == nil { - continue - } - seen[repo.ID] = struct{}{} + if !repo.HasBranch() { + continue + } + + if other, ok := seen[repo.ID]; !ok { + seen[repo.ID] = repo switch st := strings.ToLower(repo.ExternalRepository.ServiceType); st { case "github", "gitlab", "bitbucketserver": default: @@ -388,12 +388,20 @@ func (svc *Service) ResolveRepositories(ctx context.Context, spec *CampaignSpec) continue } } - - final = append(final, repo) + } else { + // If we've already seen this repository, we overwrite the + // Commit/Branch fields with the latest value we have + other.Commit = repo.Commit + other.Branch = repo.Branch } } } + final := make([]*graphql.Repository, 0, len(seen)) + for _, repo := range seen { + final = append(final, repo) + } + if unsupported.hasUnsupported() && !svc.allowUnsupported { return final, unsupported } @@ -404,6 +412,12 @@ func (svc *Service) ResolveRepositories(ctx context.Context, spec *CampaignSpec) func (svc *Service) ResolveRepositoriesOn(ctx context.Context, on *OnQueryOrRepository) ([]*graphql.Repository, error) { if on.RepositoriesMatchingQuery != "" { return svc.resolveRepositorySearch(ctx, on.RepositoriesMatchingQuery) + } else if on.Repository != "" && on.Branch != "" { + repo, err := svc.resolveRepositoryNameAndBranch(ctx, on.Repository, on.Branch) + if err != nil { + return nil, err + } + return []*graphql.Repository{repo}, nil } else if on.Repository != "" { repo, err := svc.resolveRepositoryName(ctx, on.Repository) if err != nil { @@ -418,7 +432,7 @@ func (svc *Service) ResolveRepositoriesOn(ctx context.Context, on *OnQueryOrRepo } const repositoryNameQuery = ` -query Repository($name: String!) { +query Repository($name: String!, $queryCommit: Boolean!, $rev: String!) { repository(name: $name) { ...repositoryFields } @@ -428,7 +442,9 @@ query Repository($name: String!) { func (svc *Service) resolveRepositoryName(ctx context.Context, name string) (*graphql.Repository, error) { var result struct{ Repository *graphql.Repository } if ok, err := svc.client.NewRequest(repositoryNameQuery, map[string]interface{}{ - "name": name, + "name": name, + "queryCommit": false, + "rev": "", }).Do(ctx, &result); err != nil || !ok { return nil, err } @@ -438,10 +454,36 @@ func (svc *Service) resolveRepositoryName(ctx context.Context, name string) (*gr return result.Repository, nil } +func (svc *Service) resolveRepositoryNameAndBranch(ctx context.Context, name, branch string) (*graphql.Repository, error) { + var result struct{ Repository *graphql.Repository } + if ok, err := svc.client.NewRequest(repositoryNameQuery, map[string]interface{}{ + "name": name, + "queryCommit": true, + "rev": branch, + }).Do(ctx, &result); err != nil || !ok { + return nil, err + } + if result.Repository == nil { + return nil, errors.New("no repository found") + } + if result.Repository.Commit.OID == "" { + return nil, fmt.Errorf("no branch matching %q found for repository %s", branch, name) + } + + result.Repository.Branch = graphql.Branch{ + Name: branch, + Target: result.Repository.Commit, + } + + return result.Repository, nil +} + // TODO: search result alerts. const repositorySearchQuery = ` query ChangesetRepos( $query: String!, + $queryCommit: Boolean!, + $rev: String!, ) { search(query: $query, version: V2) { results { @@ -471,7 +513,9 @@ func (svc *Service) resolveRepositorySearch(ctx context.Context, query string) ( } } if ok, err := svc.client.NewRequest(repositorySearchQuery, map[string]interface{}{ - "query": setDefaultQueryCount(query), + "query": setDefaultQueryCount(query), + "queryCommit": false, + "rev": "", }).Do(ctx, &result); err != nil || !ok { return nil, err }