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

Support updating a list of dependencies #235

Merged
merged 7 commits into from
Feb 14, 2017
Merged

Support updating a list of dependencies #235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a092cfb
Document passing args to dep ensure -update
carolynvs Feb 10, 2017
1e1a610
Add failing test for dep ensure -update github.com/foo/bar
carolynvs Feb 10, 2017
42109a7
Populate the list of dependencies to update from the command arguments
carolynvs Feb 10, 2017
4ad24d0
Tidy up the logic for -update vs. a bare ensure
carolynvs Feb 11, 2017
cd7da74
Do not modify the manifest during dep ensure -update
carolynvs Feb 11, 2017
088af81
Only allow a version to change when using -update
carolynvs Feb 13, 2017
4e573de
Use golden test files for the ensure tests
carolynvs Feb 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 73 additions & 65 deletions cmd/dep/ensure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ dep ensure -update
ignoring any versions specified in the lock file. Update the lock file with
any changes.

dep ensure -update github.com/pkg/foo github.com/pkg/bar

Update a list of dependencies to the latest versions allowed by the manifest,
ignoring any versions specified in the lock file. Update the lock file with
any changes.

dep ensure github.com/pkg/foo@^1.0.1

Constrain pkg/foo to the latest release matching >= 1.0.1, < 2.0.0, and
Expand Down Expand Up @@ -82,7 +88,7 @@ func (cmd *ensureCommand) Hidden() bool { return false }

func (cmd *ensureCommand) Register(fs *flag.FlagSet) {
fs.BoolVar(&cmd.examples, "examples", false, "print detailed usage examples")
fs.BoolVar(&cmd.update, "update", false, "ensure all dependencies are at the latest version allowed by the manifest")
fs.BoolVar(&cmd.update, "update", false, "ensure dependencies are at the latest version allowed by the manifest")
fs.BoolVar(&cmd.dryRun, "n", false, "dry run, don't actually ensure anything")
fs.Var(&cmd.overrides, "override", "specify an override constraint spec (repeatable)")
}
Expand All @@ -100,10 +106,6 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error {
return nil
}

if cmd.update && len(args) > 0 {
return errors.New("Cannot pass -update and itemized project list (for now)")
}

p, err := ctx.LoadProject("")
if err != nil {
return err
Expand All @@ -116,9 +118,72 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error {
sm.UseDefaultSignalHandling()
defer sm.Release()

params := p.MakeParams()
if *verbose {
params.Trace = true
params.TraceLogger = log.New(os.Stderr, "", 0)
}
params.RootPackageTree, err = gps.ListPackages(p.AbsRoot, string(p.ImportRoot))
if err != nil {
return errors.Wrap(err, "ensure ListPackage for project")
}

if cmd.update {
applyUpdateArgs(args, &params)
} else {
err := applyEnsureArgs(args, cmd.overrides, p, sm, &params)
if err != nil {
return err
}
}

solver, err := gps.Prepare(params, sm)
if err != nil {
return errors.Wrap(err, "ensure Prepare")
}
solution, err := solver.Solve()
if err != nil {
handleAllTheFailuresOfTheWorld(err)
return errors.Wrap(err, "ensure Solve()")
}

sw := dep.SafeWriter{
Root: p.AbsRoot,
Lock: p.Lock,
NewLock: solution,
SourceManager: sm,
}
if !cmd.update {
sw.Manifest = p.Manifest
}

// check if vendor exists, because if the locks are the same but
// vendor does not exist we should write vendor
vendorExists, err := dep.IsNonEmptyDir(filepath.Join(sw.Root, "vendor"))
if err != nil {
return errors.Wrap(err, "ensure vendor is a directory")
}
writeV := !vendorExists && solution != nil

return errors.Wrap(sw.WriteAllSafe(writeV), "grouped write of manifest, lock and vendor")
}

func applyUpdateArgs(args []string, params *gps.SolveParameters) {
// When -update is specified without args, allow every project to change versions, regardless of the lock file
if len(args) == 0 {
params.ChangeAll = true
return
}

// Allow any of specified project versions to change, regardless of the lock file
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we discussed checking for the presence of the named project in the lock file, and erroring out if it wasn't present. At least, that seems to make the most sense to me now, when looking at an implementation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out gps is checking and returning an error in that scenario already. Although I can handle the error and wrap with a different message perhaps? Here's what it looks like now when updating something that isn't already in the lock:

$ dep ensure -update github.com/sillyperson/fakepackage
ensure Prepare: cannot update github.com/sillyperson/fakepackage as it is not in the lock

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was pretty sure I'd guarded against that case :)

If you have an immediate improvement for the text in mind, OK. If not, though, for now I'd say just leave the error as-is. Tackling failures that come back from gps is a major task in itself, and we'll probably want a bit of a framework for it. I'd rather we make a concerted effort around that than have it organically grow through one-offs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I spent a minute thinking about what would be a good error message and came back with "Exactly what gps is already returning, along with a recommended command to run to get back on track". I'd prefer to tackle stuff like this (presenting a user-friendly version of the gps error message) across the board.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that sounds spot-on to me. And it reflects the division of responsibility between gps and dep well - gps says what's wrong with the abstract inputs, and dep knows about what the user input looked like that generated it (and how it might be modified).

Given the change of direction wrt CLI UI described in #213, these possibilities are likely to change and narrow anyway.

for _, arg := range args {
params.ToChange = append(params.ToChange, gps.ProjectRoot(arg))
}
}

func applyEnsureArgs(args []string, overrides stringSlice, p *dep.Project, sm *gps.SourceMgr, params *gps.SolveParameters) error {
var errs []error
for _, arg := range args {
// default persist to manifest
pc, err := getProjectConstraint(arg, sm)
if err != nil {
errs = append(errs, err)
Expand All @@ -144,18 +209,9 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error {
Source: pc.Ident.Source,
Constraint: pc.Constraint,
}

if p.Lock != nil {
for i, lp := range p.Lock.P {
if lp.Ident() == pc.Ident {
p.Lock.P = append(p.Lock.P[:i], p.Lock.P[i+1:]...)
break
}
}
}
}

for _, ovr := range cmd.overrides {
for _, ovr := range overrides {
pc, err := getProjectConstraint(ovr, sm)
if err != nil {
errs = append(errs, err)
Expand All @@ -170,15 +226,6 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error {
Source: pc.Ident.Source,
Constraint: pc.Constraint,
}

if p.Lock != nil {
for i, lp := range p.Lock.P {
if lp.Ident() == pc.Ident {
p.Lock.P = append(p.Lock.P[:i], p.Lock.P[i+1:]...)
break
}
}
}
}

if len(errs) > 0 {
Expand All @@ -190,46 +237,7 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error {
return errors.New(buf.String())
}

params := p.MakeParams()
// If -update was passed, we want the solver to allow all versions to change
params.ChangeAll = cmd.update

if *verbose {
params.Trace = true
params.TraceLogger = log.New(os.Stderr, "", 0)
}

params.RootPackageTree, err = gps.ListPackages(p.AbsRoot, string(p.ImportRoot))
if err != nil {
return errors.Wrap(err, "ensure ListPackage for project")
}
solver, err := gps.Prepare(params, sm)
if err != nil {
return errors.Wrap(err, "ensure Prepare")
}
solution, err := solver.Solve()
if err != nil {
handleAllTheFailuresOfTheWorld(err)
return errors.Wrap(err, "ensure Solve()")
}

sw := dep.SafeWriter{
Root: p.AbsRoot,
Manifest: p.Manifest,
Lock: p.Lock,
NewLock: solution,
SourceManager: sm,
}

// check if vendor exists, because if the locks are the same but
// vendor does not exist we should write vendor
vendorExists, err := dep.IsNonEmptyDir(filepath.Join(sw.Root, "vendor"))
if err != nil {
return errors.Wrap(err, "ensure vendor is a directory")
}
writeV := !vendorExists && solution != nil

return errors.Wrap(sw.WriteAllSafe(writeV), "grouped write of manifest, lock and vendor")
return nil
}

type stringSlice []string
Expand Down
Loading