Skip to content
This repository was archived by the owner on Sep 9, 2020. It is now read-only.

Commit 67c45f4

Browse files
sdboyersdboyer
authored
Merge pull request #1413 from darkowlzz/concurrent-collectconstraints
status: make collectConstraints() concurrent
2 parents 132ea90 + e8a2617 commit 67c45f4

File tree

2 files changed

+133
-69
lines changed

2 files changed

+133
-69
lines changed

‎cmd/dep/status.go‎

Lines changed: 76 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,12 @@ func runStatusAll(ctx *dep.Ctx, out outputter, p *dep.Project, sm gps.SourceMana
429429
return false, 0, errors.Wrapf(err, "could not set up solver for input hashing")
430430
}
431431

432-
cm := collectConstraints(ctx, p, sm)
432+
// Errors while collecting constraints should not fail the whole status run.
433+
// It should count the error and tell the user about incomplete results.
434+
cm, ccerrs := collectConstraints(ctx, p, sm)
435+
if len(ccerrs) > 0 {
436+
errCount += len(ccerrs)
437+
}
433438

434439
// Get the project list and sort it so that the printed output users see is
435440
// deterministically ordered. (This may be superfluous if the lock is always
@@ -583,7 +588,7 @@ func runStatusAll(ctx *dep.Ctx, out outputter, p *dep.Project, sm gps.SourceMana
583588

584589
// Count ListVersions error because we get partial results when
585590
// this happens.
586-
errCount = len(errListVerCh)
591+
errCount += len(errListVerCh)
587592
if ctx.Verbose {
588593
for err := range errListVerCh {
589594
ctx.Err.Println(err.Error())
@@ -712,37 +717,88 @@ type projectConstraint struct {
712717
type constraintsCollection map[string][]projectConstraint
713718

714719
// collectConstraints collects constraints declared by all the dependencies.
715-
func collectConstraints(ctx *dep.Ctx, p *dep.Project, sm gps.SourceManager) constraintsCollection {
720+
// It returns constraintsCollection and a slice of errors encountered while
721+
// collecting the constraints, if any.
722+
func collectConstraints(ctx *dep.Ctx, p *dep.Project, sm gps.SourceManager) (constraintsCollection, []error) {
723+
logger := ctx.Err
724+
if !ctx.Verbose {
725+
logger = log.New(ioutil.Discard, "", 0)
726+
}
727+
728+
logger.Println("Collecting project constraints:")
729+
730+
var mutex sync.Mutex
716731
constraintCollection := make(constraintsCollection)
717732

718733
// Get direct deps of the root project.
719734
_, directDeps, err := getDirectDependencies(sm, p)
720735
if err != nil {
721-
ctx.Err.Println("Error getting direct deps:", err)
736+
// Return empty collection, not nil, if we fail here.
737+
return constraintCollection, []error{errors.Wrap(err, "failed to get direct dependencies")}
722738
}
739+
723740
// Create a root analyzer.
724741
rootAnalyzer := newRootAnalyzer(true, ctx, directDeps, sm)
725742

743+
lp := p.Lock.Projects()
744+
745+
// Channel for receiving all the errors.
746+
errCh := make(chan error, len(lp))
747+
748+
var wg sync.WaitGroup
749+
726750
// Iterate through the locked projects and collect constraints of all the projects.
727-
for _, proj := range p.Lock.Projects() {
728-
manifest, _, err := sm.GetManifestAndLock(proj.Ident(), proj.Version(), rootAnalyzer)
729-
if err != nil {
730-
ctx.Err.Println("Error getting manifest and lock:", err)
731-
continue
732-
}
751+
for i, proj := range lp {
752+
wg.Add(1)
753+
logger.Printf("(%d/%d) %s\n", i+1, len(lp), proj.Ident().ProjectRoot)
754+
755+
go func(proj gps.LockedProject) {
756+
defer wg.Done()
757+
758+
manifest, _, err := sm.GetManifestAndLock(proj.Ident(), proj.Version(), rootAnalyzer)
759+
if err != nil {
760+
errCh <- errors.Wrap(err, "error getting manifest and lock")
761+
return
762+
}
733763

734-
// Get project constraints.
735-
pc := manifest.DependencyConstraints()
764+
// Get project constraints.
765+
pc := manifest.DependencyConstraints()
766+
767+
// Obtain a lock for constraintCollection.
768+
mutex.Lock()
769+
defer mutex.Unlock()
770+
// Iterate through the project constraints to get individual dependency
771+
// project and constraint values.
772+
for pr, pp := range pc {
773+
tempCC := append(
774+
constraintCollection[string(pr)],
775+
projectConstraint{proj.Ident().ProjectRoot, pp.Constraint},
776+
)
777+
778+
// Sort the inner projectConstraint slice by Project string.
779+
// Required for consistent returned value.
780+
sort.Sort(byProject(tempCC))
781+
constraintCollection[string(pr)] = tempCC
782+
}
783+
}(proj)
784+
}
785+
786+
wg.Wait()
787+
close(errCh)
736788

737-
// Iterate through the project constraints to get individual dependency
738-
// project and constraint values.
739-
for pr, pp := range pc {
740-
constraintCollection[string(pr)] = append(
741-
constraintCollection[string(pr)],
742-
projectConstraint{proj.Ident().ProjectRoot, pp.Constraint},
743-
)
789+
var errs []error
790+
if len(errCh) > 0 {
791+
for e := range errCh {
792+
errs = append(errs, e)
793+
logger.Println(e.Error())
744794
}
745795
}
746796

747-
return constraintCollection
797+
return constraintCollection, errs
748798
}
799+
800+
type byProject []projectConstraint
801+
802+
func (p byProject) Len() int { return len(p) }
803+
func (p byProject) Swap(i, j int) { p[i], p[j] = p[j], p[i] }
804+
func (p byProject) Less(i, j int) bool { return p[i].Project > p[j].Project }

‎cmd/dep/status_test.go‎

Lines changed: 57 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"bytes"
99
"io/ioutil"
1010
"log"
11+
"path/filepath"
1112
"reflect"
1213
"testing"
1314
"text/tabwriter"
@@ -306,83 +307,79 @@ func TestCollectConstraints(t *testing.T) {
306307

307308
cases := []struct {
308309
name string
309-
project dep.Project
310+
lock dep.Lock
310311
wantConstraints constraintsCollection
312+
wantErr bool
311313
}{
312314
{
313315
name: "without any constraints",
314-
project: dep.Project{
315-
Lock: &dep.Lock{
316-
P: []gps.LockedProject{
317-
gps.NewLockedProject(
318-
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/sdboyer/deptest")},
319-
gps.NewVersion("v1.0.0"),
320-
[]string{"."},
321-
),
322-
},
316+
lock: dep.Lock{
317+
P: []gps.LockedProject{
318+
gps.NewLockedProject(
319+
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/sdboyer/deptest")},
320+
gps.NewVersion("v1.0.0"),
321+
[]string{"."},
322+
),
323323
},
324324
},
325325
wantConstraints: constraintsCollection{},
326326
},
327327
{
328328
name: "with multiple constraints",
329-
project: dep.Project{
330-
Lock: &dep.Lock{
331-
P: []gps.LockedProject{
332-
gps.NewLockedProject(
333-
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/sdboyer/deptest")},
334-
gps.NewVersion("v1.0.0"),
335-
[]string{"."},
336-
),
337-
gps.NewLockedProject(
338-
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/darkowlzz/deptest-project-1")},
339-
gps.NewVersion("v0.1.0"),
340-
[]string{"."},
341-
),
342-
gps.NewLockedProject(
343-
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/darkowlzz/deptest-project-2")},
344-
gps.NewBranch("master").Pair(gps.Revision("824a8d56a4c6b2f4718824a98cd6d70d3dbd4c3e")),
345-
[]string{"."},
346-
),
347-
},
329+
lock: dep.Lock{
330+
P: []gps.LockedProject{
331+
gps.NewLockedProject(
332+
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/sdboyer/deptest")},
333+
gps.NewVersion("v1.0.0"),
334+
[]string{"."},
335+
),
336+
gps.NewLockedProject(
337+
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/darkowlzz/deptest-project-1")},
338+
gps.NewVersion("v0.1.0"),
339+
[]string{"."},
340+
),
341+
gps.NewLockedProject(
342+
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/darkowlzz/deptest-project-2")},
343+
gps.NewBranch("master").Pair(gps.Revision("824a8d56a4c6b2f4718824a98cd6d70d3dbd4c3e")),
344+
[]string{"."},
345+
),
348346
},
349347
},
350348
wantConstraints: constraintsCollection{
351-
"github.com/sdboyer/deptest": []projectConstraint{
352-
{"github.com/darkowlzz/deptest-project-1", ver1},
353-
{"github.com/darkowlzz/deptest-project-2", ver08},
354-
},
355349
"github.com/sdboyer/deptestdos": []projectConstraint{
356350
{"github.com/darkowlzz/deptest-project-2", ver2},
357351
},
358352
"github.com/sdboyer/dep-test": []projectConstraint{
359353
{"github.com/darkowlzz/deptest-project-2", ver1},
360354
},
355+
"github.com/sdboyer/deptest": []projectConstraint{
356+
{"github.com/darkowlzz/deptest-project-2", ver08},
357+
{"github.com/darkowlzz/deptest-project-1", ver1},
358+
},
361359
},
362360
},
363361
{
364362
name: "skip projects with invalid versions",
365-
project: dep.Project{
366-
Lock: &dep.Lock{
367-
P: []gps.LockedProject{
368-
gps.NewLockedProject(
369-
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/darkowlzz/deptest-project-1")},
370-
gps.NewVersion("v0.1.0"),
371-
[]string{"."},
372-
),
373-
gps.NewLockedProject(
374-
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/darkowlzz/deptest-project-2")},
375-
gps.NewVersion("v1.0.0"),
376-
[]string{"."},
377-
),
378-
},
363+
lock: dep.Lock{
364+
P: []gps.LockedProject{
365+
gps.NewLockedProject(
366+
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/darkowlzz/deptest-project-1")},
367+
gps.NewVersion("v0.1.0"),
368+
[]string{"."},
369+
),
370+
gps.NewLockedProject(
371+
gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/darkowlzz/deptest-project-2")},
372+
gps.NewVersion("v1.0.0"),
373+
[]string{"."},
374+
),
379375
},
380376
},
381377
wantConstraints: constraintsCollection{
382378
"github.com/sdboyer/deptest": []projectConstraint{
383379
{"github.com/darkowlzz/deptest-project-1", ver1},
384380
},
385381
},
382+
wantErr: true,
386383
},
387384
}
388385

@@ -403,12 +400,23 @@ func TestCollectConstraints(t *testing.T) {
403400
h.Must(err)
404401
defer sm.Release()
405402

403+
// Create new project and set root. Setting root is required for PackageList
404+
// to run properly.
405+
p := new(dep.Project)
406+
p.SetRoot(filepath.Join(pwd, "src"))
407+
406408
for _, c := range cases {
407409
t.Run(c.name, func(t *testing.T) {
408-
gotConstraints := collectConstraints(ctx, &c.project, sm)
410+
p.Lock = &c.lock
411+
gotConstraints, err := collectConstraints(ctx, p, sm)
412+
if len(err) > 0 && !c.wantErr {
413+
t.Fatalf("unexpected errors while collecting constraints: %v", err)
414+
} else if len(err) == 0 && c.wantErr {
415+
t.Fatalf("expected errors while collecting constraints, but got none")
416+
}
409417

410418
if !reflect.DeepEqual(gotConstraints, c.wantConstraints) {
411-
t.Fatalf("Unexpected collected constraints: \n\t(GOT): %v\n\t(WNT): %v", gotConstraints, c.wantConstraints)
419+
t.Fatalf("unexpected collected constraints: \n\t(GOT): %v\n\t(WNT): %v", gotConstraints, c.wantConstraints)
412420
}
413421
})
414422
}

0 commit comments

Comments
 (0)