diff --git a/CHANGELOG.md b/CHANGELOG.md index 65efd66be0..1ef764f4ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # v0.5.1 +IMPROVEMENTS: + +* The `noverify` field in `Gopkg.toml` allows for the preservation of excess files under `vendor`. ([#2002](https://github.com/golang/dep/issue/2002)) + BUG FIXES: * Correctly handle certain cases where `dep ensure` removed projects from Gopkg.lock. ([#1945](https://github.com/golang/dep/issue/1945)). diff --git a/cmd/dep/check.go b/cmd/dep/check.go index a81f05c0d9..878f7bf8e6 100644 --- a/cmd/dep/check.go +++ b/cmd/dep/check.go @@ -141,7 +141,7 @@ func (cmd *checkCommand) Run(ctx *dep.Ctx, args []string) error { noverify[skip] = true } - var vendorfail bool + var vendorfail, hasnoverify bool // One full pass through, to see if we need to print the header, and to // create an array of names to sort for deterministic output. var ordered []string @@ -149,54 +149,75 @@ func (cmd *checkCommand) Run(ctx *dep.Ctx, args []string) error { ordered = append(ordered, path) switch status { - case verify.DigestMismatchInLock, verify.HashVersionMismatch, verify.EmptyDigestInLock: - // NoVerify applies only to these three cases. + case verify.DigestMismatchInLock, verify.HashVersionMismatch, verify.EmptyDigestInLock, verify.NotInLock: if noverify[path] { + hasnoverify = true continue } fallthrough - case verify.NotInTree, verify.NotInLock: + case verify.NotInTree: + // NoVerify cannot be used to make dep check ignore the absence + // of a project entirely. + if noverify[path] { + delete(noverify, path) + } + fail = true if !vendorfail { vendorfail = true - logger.Println("# vendor is out of sync:") } - } } sort.Strings(ordered) + var vfbuf, novbuf bytes.Buffer + var bufptr *bytes.Buffer + + fmt.Fprintf(&vfbuf, "# vendor is out of sync:\n") + fmt.Fprintf(&novbuf, "# out of sync, but ignored, due to noverify in Gopkg.toml:\n") + for _, pr := range ordered { - var nvSuffix string if noverify[pr] { - nvSuffix = " (CHECK IGNORED: marked noverify in Gopkg.toml)" + bufptr = &novbuf + } else { + bufptr = &vfbuf } status := statuses[pr] switch status { case verify.NotInTree: - logger.Printf("%s: missing from vendor\n", pr) + fmt.Fprintf(bufptr, "%s: missing from vendor\n", pr) case verify.NotInLock: fi, err := os.Stat(filepath.Join(p.AbsRoot, "vendor", pr)) if err != nil { return errors.Wrap(err, "could not stat file that VerifyVendor claimed existed") } if fi.IsDir() { - logger.Printf("%s: unused project\n", pr) + fmt.Fprintf(bufptr, "%s: unused project\n", pr) } else { - logger.Printf("%s: orphaned file\n", pr) + fmt.Fprintf(bufptr, "%s: orphaned file\n", pr) } case verify.DigestMismatchInLock: - logger.Printf("%s: hash of vendored tree not equal to digest in Gopkg.lock%s\n", pr, nvSuffix) + fmt.Fprintf(bufptr, "%s: hash of vendored tree not equal to digest in Gopkg.lock\n", pr) case verify.EmptyDigestInLock: - logger.Printf("%s: no digest in Gopkg.lock to compare against hash of vendored tree%s\n", pr, nvSuffix) + fmt.Fprintf(bufptr, "%s: no digest in Gopkg.lock to compare against hash of vendored tree\n", pr) case verify.HashVersionMismatch: // This will double-print if the hash version is zero, but // that's a rare case that really only occurs before the first // run with a version of dep >=0.5.0, so it's fine. - logger.Printf("%s: hash algorithm mismatch, want version %v%s\n", pr, verify.HashVersion, nvSuffix) + fmt.Fprintf(bufptr, "%s: hash algorithm mismatch, want version %v\n", pr, verify.HashVersion) } } + + if vendorfail { + logger.Print(vfbuf.String()) + if hasnoverify { + logger.Println() + } + } + if hasnoverify { + logger.Print(novbuf.String()) + } } if fail { diff --git a/cmd/dep/testdata/harness_tests/check/noverify/hash_mismatch/stdout.txt b/cmd/dep/testdata/harness_tests/check/noverify/hash_mismatch/stdout.txt index 2070de04d5..edc1b55e92 100644 --- a/cmd/dep/testdata/harness_tests/check/noverify/hash_mismatch/stdout.txt +++ b/cmd/dep/testdata/harness_tests/check/noverify/hash_mismatch/stdout.txt @@ -1 +1,2 @@ -github.com/sdboyer/deptest: hash of vendored tree not equal to digest in Gopkg.lock (CHECK IGNORED: marked noverify in Gopkg.toml) +# out of sync, but ignored, due to noverify in Gopkg.toml: +github.com/sdboyer/deptest: hash of vendored tree not equal to digest in Gopkg.lock diff --git a/cmd/dep/testdata/harness_tests/check/noverify/hash_version_mismatch/stdout.txt b/cmd/dep/testdata/harness_tests/check/noverify/hash_version_mismatch/stdout.txt index e86ee5b293..f7b716c9cb 100644 --- a/cmd/dep/testdata/harness_tests/check/noverify/hash_version_mismatch/stdout.txt +++ b/cmd/dep/testdata/harness_tests/check/noverify/hash_version_mismatch/stdout.txt @@ -1 +1,2 @@ -github.com/sdboyer/deptest: hash algorithm mismatch, want version 1 (CHECK IGNORED: marked noverify in Gopkg.toml) +# out of sync, but ignored, due to noverify in Gopkg.toml: +github.com/sdboyer/deptest: hash algorithm mismatch, want version 1 diff --git a/cmd/dep/testdata/harness_tests/ensure/noverify/hash_mismatch/README b/cmd/dep/testdata/harness_tests/ensure/noverify/hash_mismatch/README index 5b8664c494..d795a2d0a1 100644 --- a/cmd/dep/testdata/harness_tests/ensure/noverify/hash_mismatch/README +++ b/cmd/dep/testdata/harness_tests/ensure/noverify/hash_mismatch/README @@ -2,4 +2,4 @@ This is a hack - it's effectively just verifying that the Gopkg.lock doesn't change for projects with noverify set, which (under the current logic) is an indicator that vendor wasn't updated. -Of course, that vendor -> lock relatinoship isn't guaranteed to hold... +Of course, that vendor -> lock relationship isn't guaranteed to hold... diff --git a/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/final/Gopkg.lock b/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/final/Gopkg.lock new file mode 100644 index 0000000000..188ece4f77 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/final/Gopkg.lock @@ -0,0 +1,17 @@ +# This file is autogenerated, do not edit; changes may be undone by the next 'dep ensure'. + + +[[projects]] + digest = "1:ddbbbe7f7a81c86d54e89fa388b532f4c144d666a14e8e483ba04fa58265b135" + name = "github.com/sdboyer/deptest" + packages = ["."] + pruneopts = "" + revision = "ff2948a2ac8f538c4ecd55962e919d1e13e74baf" + version = "v1.0.0" + +[solve-meta] + analyzer-name = "dep" + analyzer-version = 1 + input-imports = ["github.com/sdboyer/deptest"] + solver-name = "gps-cdcl" + solver-version = 1 diff --git a/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/final/Gopkg.toml b/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/final/Gopkg.toml new file mode 100644 index 0000000000..e0c984cbc2 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/final/Gopkg.toml @@ -0,0 +1 @@ +noverify = ["foo", "orphdir"] diff --git a/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/initial/Gopkg.lock b/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/initial/Gopkg.lock new file mode 100644 index 0000000000..188ece4f77 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/initial/Gopkg.lock @@ -0,0 +1,17 @@ +# This file is autogenerated, do not edit; changes may be undone by the next 'dep ensure'. + + +[[projects]] + digest = "1:ddbbbe7f7a81c86d54e89fa388b532f4c144d666a14e8e483ba04fa58265b135" + name = "github.com/sdboyer/deptest" + packages = ["."] + pruneopts = "" + revision = "ff2948a2ac8f538c4ecd55962e919d1e13e74baf" + version = "v1.0.0" + +[solve-meta] + analyzer-name = "dep" + analyzer-version = 1 + input-imports = ["github.com/sdboyer/deptest"] + solver-name = "gps-cdcl" + solver-version = 1 diff --git a/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/initial/Gopkg.toml b/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/initial/Gopkg.toml new file mode 100644 index 0000000000..e0c984cbc2 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/initial/Gopkg.toml @@ -0,0 +1 @@ +noverify = ["foo", "orphdir"] diff --git a/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/initial/main.go b/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/initial/main.go new file mode 100644 index 0000000000..e23fcf34c5 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/initial/main.go @@ -0,0 +1,12 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + _ "github.com/sdboyer/deptest" +) + +func main() { +} diff --git a/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/initial/vendor/foo b/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/initial/vendor/foo new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/initial/vendor/github.com/sdboyer/deptest/deptest.go b/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/initial/vendor/github.com/sdboyer/deptest/deptest.go new file mode 100644 index 0000000000..89983b5a33 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/initial/vendor/github.com/sdboyer/deptest/deptest.go @@ -0,0 +1,3 @@ +package deptest + +type Foo int diff --git a/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/initial/vendor/orphdir/.gitkeep b/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/initial/vendor/orphdir/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/stdout.txt b/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/stdout.txt new file mode 100644 index 0000000000..0953a0a34f --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/stdout.txt @@ -0,0 +1,3 @@ +# out of sync, but ignored, due to noverify in Gopkg.toml: +foo: orphaned file +orphdir: unused project diff --git a/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/testcase.json b/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/testcase.json new file mode 100644 index 0000000000..5f45414e73 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/noverify/vendororphans/testcase.json @@ -0,0 +1,9 @@ +{ + "commands": [ + ["ensure"], + ["check"] + ], + "vendor-final": [ + "github.com/sdboyer/deptest" + ] +} diff --git a/docs/Gopkg.toml.md b/docs/Gopkg.toml.md index c759e92737..0507a196a1 100644 --- a/docs/Gopkg.toml.md +++ b/docs/Gopkg.toml.md @@ -231,18 +231,20 @@ It is usually safe to set `non-go = true`, as well. However, as dep only has a c ## `noverify` -The `noverify` field is a list of [project roots](glossary.md#project-root) to exclude from [vendor verification](glossary.md#vendor-verification). +The `noverify` field is a list of paths, typically [project roots](glossary.md#project-root), to exclude from [vendor verification](glossary.md#vendor-verification). Dep uses per-project hash digests, computed after pruning and recorded in [Gopkg.lock](Gopkg.lock.md#digest), to determine if the contents of `vendor/` are as expected. If the recorded digest and the hash of the corresponding tree in `vendor/` differ, that project is considered to be out of sync: * `dep ensure` will regenerate it * `dep check` will complain of a hash mismatch and exit 1 -It is strongly preferable for almost all workflows that you leave `vendor/` unmodified, in whatever state dep puts it in. However, this isn't always an option. If you have no choice but to modify `vendor/` for a particular project, then add the project root for that project to `noverify`. This will have the following effects: +It is strongly recommended that you leave `vendor/` unmodified, in whatever state dep puts it in. However, this isn't always feasible. If you have no choice but to modify `vendor/` for a particular project, then add the project root for that project to `noverify`. This will have the following effects: * `dep ensure` will ignore hash mismatches for the project, and only regenerate it in `vendor/` if absolutely necessary (prune options change, package list changes, version changes) * `dep check` will continue to report hash mismatches (albeit with an annotation about `noverify`) for the project, but will no longer exit 1. +`noverify` can also be used to preserve certain excess paths that would otherwise be removed; for example, adding `WORKSPACE` to the `noverify` list would allow you to preserve `vendor/WORKSPACE`, which can help with some Bazel-based workflows. + ## Scope `dep` evaluates diff --git a/txn_writer.go b/txn_writer.go index 8e408254fa..29579d5cb3 100644 --- a/txn_writer.go +++ b/txn_writer.go @@ -432,6 +432,7 @@ const ( missingFromTree projectAdded projectRemoved + pathPreserved ) // NewDeltaWriter prepares a vendor writer that will construct a vendor @@ -510,17 +511,26 @@ func NewDeltaWriter(p *Project, newLock *Lock, behavior VendorBehavior) (TreeWri // We don't validate this field elsewhere as it can be difficult to know // at the beginning of a dep ensure command whether or not the noverify // project actually will exist as part of the Lock by the end of the - // run. So, only apply if it's in the lockdiff, and isn't a removal. + // run. So, only apply if it's in the lockdiff. if _, has := dw.lockDiff.ProjectDeltas[pr]; has { - if typ, has := dw.changed[pr]; has && typ < noVerify { - // Avoid writing noverify projects at all for the lower change - // types. - delete(dw.changed, pr) - - // Uncomment this if we want to switch to the safer behavior, - // where we ALWAYS write noverify projects. - //dw.changed[pr] = noVerify + if typ, has := dw.changed[pr]; has { + if typ < noVerify { + // Avoid writing noverify projects at all for the lower change + // types. + delete(dw.changed, pr) + + // Uncomment this if we want to switch to the safer behavior, + // where we ALWAYS write noverify projects. + //dw.changed[pr] = noVerify + } else if typ == projectRemoved { + // noverify can also be used to preserve files that would + // otherwise be removed. + dw.changed[pr] = pathPreserved + } } + // It's also allowed to preserve entirely unknown paths using noverify. + } else if _, has := status[spr]; has { + dw.changed[pr] = pathPreserved } } @@ -564,29 +574,28 @@ func (dw *DeltaWriter) Write(path string, sm gps.SourceManager, examples bool, l projs[lp.Ident().ProjectRoot] = lp } - dropped := []gps.ProjectRoot{} + var dropped, preserved []gps.ProjectRoot i := 0 tot := len(dw.changed) - if len(dw.changed) > 0 { - logger.Println("# Bringing vendor into sync") + for _, reason := range dw.changed { + if reason != pathPreserved { + logger.Println("# Bringing vendor into sync") + break + } } + for pr, reason := range dw.changed { - if reason == projectRemoved { + switch reason { + case projectRemoved: dropped = append(dropped, pr) continue + case pathPreserved: + preserved = append(preserved, pr) + continue } to := filepath.FromSlash(filepath.Join(vnewpath, string(pr))) - proj, has := projs[pr] - if !has { - // This shouldn't be reachable, but it's preferable to print an - // error and continue rather than panic. https://github.com/golang/dep/issues/1945 - // TODO(sdboyer) remove this once we've increased confidence around - // this case. - fmt.Fprintf(os.Stderr, "Internal error - %s had change code %v but was not in new Gopkg.lock. Re-running dep ensure should fix this. Please file a bug at https://github.com/golang/dep/issues/new!\n", pr, reason) - continue - } - po := proj.(verify.VerifiableProject).PruneOpts + po := projs[pr].(verify.VerifiableProject).PruneOpts if err := sm.ExportPrunedProject(context.TODO(), projs[pr], po, to); err != nil { return errors.Wrapf(err, "failed to export %s", pr) } @@ -666,13 +675,18 @@ func (dw *DeltaWriter) Write(path string, sm gps.SourceManager, examples bool, l } } - // Ensure vendor/.git is preserved if present + // Special case: ensure vendor/.git is preserved if present if hasDotGit(vpath) { - err = fs.RenameWithFallback(filepath.Join(vpath, ".git"), filepath.Join(vnewpath, ".git")) + preserved = append(preserved, ".git") + } + + for _, path := range preserved { + err = fs.RenameWithFallback(filepath.Join(vpath, string(path)), filepath.Join(vnewpath, string(path))) if err != nil { - return errors.Wrap(err, "failed to preserve vendor/.git") + return errors.Wrapf(err, "failed to preserve vendor/%s", path) } } + err = os.RemoveAll(vpath) if err != nil { return errors.Wrap(err, "failed to remove original vendor directory")