Skip to content

Commit d28f527

Browse files
authored
Fix nil-panic if repository has no default branch (#635)
This is the fix for the issue reported in sourcegraph/customer#537. The problem: 1. Sometimes repositories don't have a default branch, because they're being cloned or updated by Sourcegraph. See code here: https://sourcegraph.com/github.com/sourcegraph/sourcegraph@1cbb9023b0c77e6fa132298df1b2b5a768288d67/-/blob/internal/vcs/git/default_branch.go?L6 2. We skip the repositories with no repositories, *but too late*. With the introduction of `.batchignore` we added additional code that broke that check.
1 parent 3a48ac7 commit d28f527

File tree

3 files changed

+78
-6
lines changed

3 files changed

+78
-6
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ All notable changes to `src-cli` are documented in this file.
1717

1818
### Fixed
1919

20+
- Fixes a nil-panic that could be caused when `src batch [preview|apply]` would encounter a repository that was currently being cloned or is empty.
21+
2022
### Removed
2123

2224
## 3.33.0

internal/batches/service/service.go

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"strings"
1313

1414
"github.com/cockroachdb/errors"
15+
1516
batcheslib "github.com/sourcegraph/sourcegraph/lib/batches"
1617

1718
"github.com/sourcegraph/src-cli/internal/api"
@@ -431,19 +432,23 @@ func (svc *Service) ResolveRepositories(ctx context.Context, spec *batcheslib.Ba
431432
return nil, errors.Wrapf(err, "resolving %q", on.String())
432433
}
433434

435+
reposWithBranch := make([]*graphql.Repository, 0, len(repos))
436+
for _, repo := range repos {
437+
if !repo.HasBranch() {
438+
continue
439+
}
440+
reposWithBranch = append(reposWithBranch, repo)
441+
}
442+
434443
var repoBatchIgnores map[*graphql.Repository][]string
435444
if !svc.allowIgnored {
436-
repoBatchIgnores, err = svc.FindDirectoriesInRepos(ctx, ".batchignore", repos...)
445+
repoBatchIgnores, err = svc.FindDirectoriesInRepos(ctx, ".batchignore", reposWithBranch...)
437446
if err != nil {
438447
return nil, err
439448
}
440449
}
441450

442-
for _, repo := range repos {
443-
if !repo.HasBranch() {
444-
continue
445-
}
446-
451+
for _, repo := range reposWithBranch {
447452
if other, ok := seen[repo.ID]; !ok {
448453
seen[repo.ID] = repo
449454

internal/batches/service/service_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010

1111
"github.com/google/go-cmp/cmp"
1212
"github.com/google/go-cmp/cmp/cmpopts"
13+
1314
batcheslib "github.com/sourcegraph/sourcegraph/lib/batches"
1415

1516
"github.com/sourcegraph/src-cli/internal/api"
@@ -322,6 +323,70 @@ const testBatchIgnoreInRepos = `{
322323
}
323324
`
324325

326+
func TestResolveRepositories_RepoWithoutBranch(t *testing.T) {
327+
spec := &batcheslib.BatchSpec{
328+
On: []batcheslib.OnQueryOrRepository{
329+
{RepositoriesMatchingQuery: "testquery"},
330+
},
331+
}
332+
333+
client, done := mockGraphQLClient(testResolveRepositoriesNoBranch, testBatchIgnoreInReposNoBranch)
334+
defer done()
335+
336+
svc := &Service{client: client, allowIgnored: false}
337+
338+
repos, err := svc.ResolveRepositories(context.Background(), spec)
339+
if err != nil {
340+
t.Fatalf("unexpected error: %s", err)
341+
}
342+
if len(repos) != 1 {
343+
t.Fatalf("wrong number of repos. want=%d, have=%d", 2, len(repos))
344+
}
345+
}
346+
347+
const testResolveRepositoriesNoBranch = `{
348+
"data": {
349+
"search": {
350+
"results": {
351+
"results": [
352+
{
353+
"__typename": "Repository",
354+
"id": "UmVwb3NpdG9yeToxMw==",
355+
"name": "bitbucket.sgdev.org/SOUR/automation-testing",
356+
"url": "/bitbucket.sgdev.org/SOUR/automation-testing",
357+
"externalRepository": { "serviceType": "bitbucketserver" },
358+
"defaultBranch": null
359+
},
360+
{
361+
"__typename": "Repository",
362+
"id": "UmVwb3NpdG9yeTo0",
363+
"name": "github.com/sourcegraph/automation-testing",
364+
"url": "/github.com/sourcegraph/automation-testing",
365+
"externalRepository": { "serviceType": "github" },
366+
"defaultBranch": null
367+
},
368+
{
369+
"__typename": "Repository",
370+
"id": "UmVwb3NpdG9yeTo2MQ==",
371+
"name": "gitlab.sgdev.org/sourcegraph/automation-testing",
372+
"url": "/gitlab.sgdev.org/sourcegraph/automation-testing",
373+
"externalRepository": { "serviceType": "gitlab" },
374+
"defaultBranch": { "name": "refs/heads/master", "target": { "oid": "3b79a5d479d2af9cfe91e0aad4e9dddca7278150" } }
375+
}
376+
]
377+
}
378+
}
379+
}
380+
}
381+
`
382+
383+
const testBatchIgnoreInReposNoBranch = `{
384+
"data": {
385+
"repo_0": { "results": { "results": [] } }
386+
}
387+
}
388+
`
389+
325390
func TestService_FindDirectoriesInRepos(t *testing.T) {
326391
client, done := mockGraphQLClient(testFindDirectoriesInRepos)
327392
defer done()

0 commit comments

Comments
 (0)