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

Commit

Permalink
Merge pull request #1218 from darkowlzz/concurrent-ensure-add
Browse files Browse the repository at this point in the history 
Concurrent ensure add
  • Loading branch information
@jmank88
jmank88 authored Sep 28, 2017
2 parents 6fb987b + e7899a3 commit 82b3908
Show file tree
Hide file tree
Showing 2 changed files with 112 additions and 63 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

IMPROVEMENTS:

* `dep ensure -add` now concurrently fetches the source and adds the projects.
(#1218)
* File name case check is now performed on `Gopkg.toml` and `Gopkg.lock`.
(#1114)
* gps: gps now supports pruning. (#1020)
Expand Down
173 changes: 110 additions & 63 deletions cmd/dep/ensure.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,10 @@ dep ensure -update -no-vendor
`

var errUpdateArgsValidation = errors.New("update arguments validation failed")
var (
errUpdateArgsValidation = errors.New("update arguments validation failed")
errAddDepsFailed = errors.New("adding dependencies failed")
)

func (cmd *ensureCommand) Name() string { return "ensure" }
func (cmd *ensureCommand) Args() string {
Expand Down Expand Up @@ -468,86 +471,130 @@ func (cmd *ensureCommand) runAdd(ctx *dep.Ctx, args []string, p *dep.Project, sm
}
addInstructions := make(map[gps.ProjectRoot]addInstruction)

for _, arg := range args {
// TODO(sdboyer) return all errors, not just the first one we encounter
// TODO(sdboyer) do these concurrently?
pc, path, err := getProjectConstraint(arg, sm)
if err != nil {
// TODO(sdboyer) ensure these errors are contextualized in a sensible way for -add
return err
}
// A mutex for limited access to addInstructions by goroutines.
var mutex sync.Mutex

// check if the the parsed path is the current root path
if strings.EqualFold(string(p.ImportRoot), string(pc.Ident.ProjectRoot)) {
return errors.New("cannot add current project to itself")
}
// Channel for receiving all the errors.
errCh := make(chan error, len(args))

inManifest := p.Manifest.HasConstraintsOn(pc.Ident.ProjectRoot)
inImports := exrmap[pc.Ident.ProjectRoot]
if inManifest && inImports {
return errors.Errorf("nothing to -add, %s is already in %s and the project's direct imports or required list", pc.Ident.ProjectRoot, dep.ManifestName)
}
var wg sync.WaitGroup

err = sm.SyncSourceFor(pc.Ident)
if err != nil {
return errors.Wrapf(err, "failed to fetch source for %s", pc.Ident.ProjectRoot)
fmt.Println("Fetching sources...")

for i, arg := range args {
wg.Add(1)

if ctx.Verbose {
ctx.Err.Printf("(%d/%d) %s\n", i+1, len(args), arg)
}

someConstraint := !gps.IsAny(pc.Constraint) || pc.Ident.Source != ""
go func(arg string) {
defer wg.Done()

pc, path, err := getProjectConstraint(arg, sm)
if err != nil {
// TODO(sdboyer) ensure these errors are contextualized in a sensible way for -add
errCh <- err
return
}

// check if the the parsed path is the current root path
if strings.EqualFold(string(p.ImportRoot), string(pc.Ident.ProjectRoot)) {
errCh <- errors.New("cannot add current project to itself")
return
}

inManifest := p.Manifest.HasConstraintsOn(pc.Ident.ProjectRoot)
inImports := exrmap[pc.Ident.ProjectRoot]
if inManifest && inImports {
errCh <- errors.Errorf("nothing to -add, %s is already in %s and the project's direct imports or required list", pc.Ident.ProjectRoot, dep.ManifestName)
return
}

instr, has := addInstructions[pc.Ident.ProjectRoot]
if has {
// Multiple packages from the same project were specified as
// arguments; make sure they agree on declared constraints.
// TODO(sdboyer) until we have a general method for checking constraint equality, only allow one to declare
if someConstraint {
if !gps.IsAny(instr.constraint) || instr.id.Source != "" {
return errors.Errorf("can only specify rules once per project being added; rules were given at least twice for %s", pc.Ident.ProjectRoot)
err = sm.SyncSourceFor(pc.Ident)
if err != nil {
errCh <- errors.Wrapf(err, "failed to fetch source for %s", pc.Ident.ProjectRoot)
return
}

someConstraint := !gps.IsAny(pc.Constraint) || pc.Ident.Source != ""

// Obtain a lock for addInstructions
mutex.Lock()
defer mutex.Unlock()
instr, has := addInstructions[pc.Ident.ProjectRoot]
if has {
// Multiple packages from the same project were specified as
// arguments; make sure they agree on declared constraints.
// TODO(sdboyer) until we have a general method for checking constraint equality, only allow one to declare
if someConstraint {
if !gps.IsAny(instr.constraint) || instr.id.Source != "" {
errCh <- errors.Errorf("can only specify rules once per project being added; rules were given at least twice for %s", pc.Ident.ProjectRoot)
return
}
instr.constraint = pc.Constraint
instr.id = pc.Ident
}
} else {
instr.ephReq = make(map[string]bool)
instr.constraint = pc.Constraint
instr.id = pc.Ident
}
} else {
instr.ephReq = make(map[string]bool)
instr.constraint = pc.Constraint
instr.id = pc.Ident
}

if inManifest {
if someConstraint {
return errors.Errorf("%s already contains rules for %s, cannot specify a version constraint or alternate source", dep.ManifestName, path)
}

instr.ephReq[path] = true
instr.typ |= isInManifest
} else if inImports {
if !someConstraint {
if exmap[path] {
return errors.Errorf("%s is already imported or required, so -add is only valid with a constraint", path)
if inManifest {
if someConstraint {
errCh <- errors.Errorf("%s already contains rules for %s, cannot specify a version constraint or alternate source", dep.ManifestName, path)
return
}

// No constraints, but the package isn't imported; require it.
// TODO(sdboyer) this case seems like it's getting overly specific and risks muddying the water more than it helps
instr.ephReq[path] = true
instr.typ |= isInImportsNoConstraint
} else {
// Don't require on this branch if the path was a ProjectRoot;
// most common here will be the user adding constraints to
// something they already imported, and if they specify the
// root, there's a good chance they don't actually want to
// require the project's root package, but are just trying to
// indicate which project should receive the constraints.
if !exmap[path] && string(pc.Ident.ProjectRoot) != path {
instr.typ |= isInManifest
} else if inImports {
if !someConstraint {
if exmap[path] {
errCh <- errors.Errorf("%s is already imported or required, so -add is only valid with a constraint", path)
return
}

// No constraints, but the package isn't imported; require it.
// TODO(sdboyer) this case seems like it's getting overly specific and risks muddying the water more than it helps
instr.ephReq[path] = true
instr.typ |= isInImportsNoConstraint
} else {
// Don't require on this branch if the path was a ProjectRoot;
// most common here will be the user adding constraints to
// something they already imported, and if they specify the
// root, there's a good chance they don't actually want to
// require the project's root package, but are just trying to
// indicate which project should receive the constraints.
if !exmap[path] && string(pc.Ident.ProjectRoot) != path {
instr.ephReq[path] = true
}
instr.typ |= isInImportsWithConstraint
}
instr.typ |= isInImportsWithConstraint
} else {
instr.typ |= isInNeither
instr.ephReq[path] = true
}
} else {
instr.typ |= isInNeither
instr.ephReq[path] = true
}

addInstructions[pc.Ident.ProjectRoot] = instr
addInstructions[pc.Ident.ProjectRoot] = instr
}(arg)
}

wg.Wait()
close(errCh)

// Newline after printing the fetching source output.
ctx.Err.Println()

// Log all the errors.
if len(errCh) > 0 {
ctx.Err.Printf("Failed to add the dependencies:\n\n")
for err := range errCh {
ctx.Err.Println(" ✗", err.Error())
}
ctx.Err.Println()
return errAddDepsFailed
}

// We're now sure all of our add instructions are individually and mutually
Expand Down

0 comments on commit 82b3908

Please sign in to comment.