diff --git a/cmd/dep/ensure.go b/cmd/dep/ensure.go index ea07f0cd9d..84b4e4cafb 100644 --- a/cmd/dep/ensure.go +++ b/cmd/dep/ensure.go @@ -147,25 +147,27 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error { 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")) + vendorExists, err := dep.IsNonEmptyDir(filepath.Join(p.AbsRoot, "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") + var sw dep.SafeWriter + var manifest *dep.Manifest + if !cmd.update { + manifest = p.Manifest + } + + newLock := dep.LockFromInterface(solution) + sw.Prepare(manifest, p.Lock, newLock, writeV) + if cmd.dryRun { + return sw.PrintPreparedActions() + } + + return errors.Wrap(sw.Write(p.AbsRoot, sm), "grouped write of manifest, lock and vendor") } func applyUpdateArgs(args []string, params *gps.SolveParameters) { diff --git a/cmd/dep/init.go b/cmd/dep/init.go index fdaf864ebf..0eb67cb162 100644 --- a/cmd/dep/init.go +++ b/cmd/dep/init.go @@ -106,13 +106,13 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error { if err != nil { return err } - m := dep.Manifest{ + m := &dep.Manifest{ Dependencies: pd.constraints, } // Make an initial lock from what knowledge we've collected about the // versions on disk - l := dep.Lock{ + l := &dep.Lock{ P: make([]gps.LockedProject, 0, len(pd.ondisk)), } @@ -134,19 +134,13 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error { ) } - sw := dep.SafeWriter{ - Root: root, - SourceManager: sm, - Manifest: &m, - } - if len(pd.notondisk) > 0 { vlogf("Solving...") params := gps.SolveParameters{ RootDir: root, RootPackageTree: pkgT, - Manifest: &m, - Lock: &l, + Manifest: m, + Lock: l, } if *verbose { @@ -163,14 +157,14 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error { handleAllTheFailuresOfTheWorld(err) return err } - sw.Lock = dep.LockFromInterface(soln) - } else { - sw.Lock = &l + l = dep.LockFromInterface(soln) } vlogf("Writing manifest and lock files.") - if err := sw.WriteAllSafe(true); err != nil { + var sw dep.SafeWriter + sw.Prepare(m, l, nil, true) + if err := sw.Write(root, sm); err != nil { return errors.Wrap(err, "safe write of manifest and lock") } diff --git a/cmd/dep/integration_test.go b/cmd/dep/integration_test.go index ba4cbc87c8..e96210e3e0 100644 --- a/cmd/dep/integration_test.go +++ b/cmd/dep/integration_test.go @@ -50,10 +50,16 @@ func TestIntegration(t *testing.T) { // Run commands testProj.RecordImportPaths() + + var err error for _, args := range testCase.Commands { - testProj.DoRun(args) + if err = testProj.DoRun(args); err != nil { + break + } } + testCase.CompareError(err) + // Check final manifest and lock testCase.CompareFile("manifest.json", testProj.ProjPath("manifest.json")) testCase.CompareFile("lock.json", testProj.ProjPath("lock.json")) diff --git a/cmd/dep/remove.go b/cmd/dep/remove.go index 6b5b209ed4..a6d5d8f939 100644 --- a/cmd/dep/remove.go +++ b/cmd/dep/remove.go @@ -179,15 +179,10 @@ func (cmd *removeCommand) Run(ctx *dep.Ctx, args []string) error { return err } - sw := dep.SafeWriter{ - Root: p.AbsRoot, - Manifest: p.Manifest, - Lock: p.Lock, - NewLock: soln, - SourceManager: sm, - } - - if err := sw.WriteAllSafe(false); err != nil { + var sw dep.SafeWriter + newLock := dep.LockFromInterface(soln) + sw.Prepare(p.Manifest, p.Lock, newLock, false) + if err := sw.Write(p.AbsRoot, sm); err != nil { return errors.Wrap(err, "grouped write of manifest, lock and vendor") } return nil diff --git a/cmd/dep/testdata/harness_tests/README.md b/cmd/dep/testdata/harness_tests/README.md index a34d68d186..4417faec18 100644 --- a/cmd/dep/testdata/harness_tests/README.md +++ b/cmd/dep/testdata/harness_tests/README.md @@ -57,7 +57,8 @@ The `testcase.json` file has the following format: "github.com/sdboyer/deptestdos", "github.com/sdboyer/deptesttres", "github.com/sdboyer/deptestquatro" - ] + ], + "error-expected":"something went wrong" } All of the categories are optional - if the `imports` list for a test is empty, @@ -71,9 +72,10 @@ The test procedure is as follows: 4. Fetch the repos and versions in `gopath-initial` into `$TMPDIR/src` directory 5. Fetch the repos and versions in `vendor-initial` to the project's `vendor` directory 6. Run `commands` on the project, in declaration order -7. Check the resulting files against those in the `final` input directory -8. Check the `vendor` directory for the projects listed under `vendor-final` -9. Check that there were no changes to `src` listings -10. Clean up +7. Check that only expected errors were raised +8. Check the resulting files against those in the `final` input directory +9. Check the `vendor` directory for the projects listed under `vendor-final` +10. Check that there were no changes to `src` listings +11. Clean up Note that for the remote fetches, only git repos are currently supported. diff --git a/cmd/dep/testdata/harness_tests/ensure/update/case1/testcase.json b/cmd/dep/testdata/harness_tests/ensure/update/case1/testcase.json index 32da9821de..cb357104b5 100644 --- a/cmd/dep/testdata/harness_tests/ensure/update/case1/testcase.json +++ b/cmd/dep/testdata/harness_tests/ensure/update/case1/testcase.json @@ -1,6 +1,5 @@ { "commands": [ - ["init"], ["ensure", "-update", "github.com/sdboyer/deptest"] ], "vendor-final": [ diff --git a/cmd/dep/testdata/harness_tests/ensure/update/case2/final/lock.json b/cmd/dep/testdata/harness_tests/ensure/update/case2/final/lock.json new file mode 100644 index 0000000000..47cfd344d7 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/update/case2/final/lock.json @@ -0,0 +1,21 @@ +{ + "memo": "9a5243dd3fa20feeaa20398e7283d6c566532e2af1aae279a010df34793761c5", + "projects": [ + { + "name": "github.com/sdboyer/deptest", + "version": "v0.8.0", + "revision": "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", + "packages": [ + "." + ] + }, + { + "name": "github.com/sdboyer/deptestdos", + "version": "v2.0.0", + "revision": "5c607206be5decd28e6263ffffdcee067266015e", + "packages": [ + "." + ] + } + ] +} diff --git a/cmd/dep/testdata/harness_tests/ensure/update/case2/final/manifest.json b/cmd/dep/testdata/harness_tests/ensure/update/case2/final/manifest.json new file mode 100644 index 0000000000..dfc83c31bc --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/update/case2/final/manifest.json @@ -0,0 +1,7 @@ +{ + "dependencies": { + "github.com/sdboyer/deptest": { + "version": "~0.8.0" + } + } +} diff --git a/cmd/dep/testdata/harness_tests/ensure/update/case2/initial/lock.json b/cmd/dep/testdata/harness_tests/ensure/update/case2/initial/lock.json new file mode 100644 index 0000000000..47cfd344d7 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/update/case2/initial/lock.json @@ -0,0 +1,21 @@ +{ + "memo": "9a5243dd3fa20feeaa20398e7283d6c566532e2af1aae279a010df34793761c5", + "projects": [ + { + "name": "github.com/sdboyer/deptest", + "version": "v0.8.0", + "revision": "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", + "packages": [ + "." + ] + }, + { + "name": "github.com/sdboyer/deptestdos", + "version": "v2.0.0", + "revision": "5c607206be5decd28e6263ffffdcee067266015e", + "packages": [ + "." + ] + } + ] +} diff --git a/cmd/dep/testdata/harness_tests/ensure/update/case2/initial/main.go b/cmd/dep/testdata/harness_tests/ensure/update/case2/initial/main.go new file mode 100644 index 0000000000..2eae5b511d --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/update/case2/initial/main.go @@ -0,0 +1,18 @@ +// Copyright 2016 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" + "github.com/sdboyer/deptestdos" +) + +func main() { + err := nil + if err != nil { + deptest.Map["yo yo!"] + } + deptestdos.diMeLo("whatev") +} diff --git a/cmd/dep/testdata/harness_tests/ensure/update/case2/initial/manifest.json b/cmd/dep/testdata/harness_tests/ensure/update/case2/initial/manifest.json new file mode 100644 index 0000000000..dfc83c31bc --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/update/case2/initial/manifest.json @@ -0,0 +1,7 @@ +{ + "dependencies": { + "github.com/sdboyer/deptest": { + "version": "~0.8.0" + } + } +} diff --git a/cmd/dep/testdata/harness_tests/ensure/update/case2/testcase.json b/cmd/dep/testdata/harness_tests/ensure/update/case2/testcase.json new file mode 100644 index 0000000000..5be759213e --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/update/case2/testcase.json @@ -0,0 +1,6 @@ +{ + "commands": [ + ["init"], + ["ensure", "-n", "-update", "github.com/sdboyer/deptest"] + ] +} diff --git a/fs.go b/fs.go index 5e0a761c72..9dab36037c 100644 --- a/fs.go +++ b/fs.go @@ -6,13 +6,14 @@ package dep import ( "encoding/json" - "fmt" "io" "io/ioutil" "os" "path/filepath" "runtime" "syscall" + + "github.com/pkg/errors" ) func IsRegular(name string) (bool, error) { @@ -25,7 +26,7 @@ func IsRegular(name string) (bool, error) { return false, err } if fi.IsDir() { - return false, fmt.Errorf("%q is a directory, should be a file", name) + return false, errors.Errorf("%q is a directory, should be a file", name) } return true, nil } @@ -40,7 +41,7 @@ func IsDir(name string) (bool, error) { return false, err } if !fi.IsDir() { - return false, fmt.Errorf("%q is not a directory", name) + return false, errors.Errorf("%q is not a directory", name) } return true, nil } @@ -80,7 +81,7 @@ func writeFile(path string, in json.Marshaler) error { func renameWithFallback(src, dest string) error { fi, err := os.Lstat(src) if err != nil { - return err + return errors.Wrapf(err, "cannot stat %s", src) } // Windows cannot use syscall.Rename to rename a directory @@ -88,7 +89,7 @@ func renameWithFallback(src, dest string) error { if err := CopyDir(src, dest); err != nil { return err } - return os.RemoveAll(src) + return errors.Wrapf(os.RemoveAll(src), "cannot delete %s", src) } err = os.Rename(src, dest) @@ -98,7 +99,7 @@ func renameWithFallback(src, dest string) error { terr, ok := err.(*os.LinkError) if !ok { - return err + return errors.Wrapf(err, "cannot rename %s to %s", src, dest) } // Rename may fail if src and dest are on different devices; fall back to @@ -123,14 +124,14 @@ func renameWithFallback(src, dest string) error { cerr = CopyFile(src, dest) } } else { - return terr + return errors.Wrapf(terr, "link error: cannot rename %s to %s", src, dest) } if cerr != nil { - return cerr + return errors.Wrapf(cerr, "second attemp failed: cannot rename %s to %s", src, dest) } - return os.RemoveAll(src) + return errors.Wrapf(os.RemoveAll(src), "cannot delete %s", src) } // CopyDir takes in a directory and copies its contents to the destination. @@ -138,23 +139,23 @@ func renameWithFallback(src, dest string) error { func CopyDir(src string, dest string) error { fi, err := os.Lstat(src) if err != nil { - return err + return errors.Wrapf(err, "cannot stat %s", src) } err = os.MkdirAll(dest, fi.Mode()) if err != nil { - return err + return errors.Wrapf(err, "cannot mkdir %s", dest) } dir, err := os.Open(src) if err != nil { - return err + return errors.Wrapf(err, "cannot open %s", src) } defer dir.Close() objects, err := dir.Readdir(-1) if err != nil { - return err + return errors.Wrapf(err, "cannot read directory %s", dir.Name()) } for _, obj := range objects { diff --git a/lock.go b/lock.go index 313036ddca..952db674a8 100644 --- a/lock.go +++ b/lock.go @@ -102,23 +102,7 @@ func (l *Lock) MarshalJSON() ([]byte, error) { } v := lp.Version() - // Figure out how to get the underlying revision - switch tv := v.(type) { - case gps.UnpairedVersion: - // TODO we could error here, if we want to be very defensive about not - // allowing a lock to be written if without an immmutable revision - case gps.Revision: - ld.Revision = tv.String() - case gps.PairedVersion: - ld.Revision = tv.Underlying().String() - } - - switch v.Type() { - case gps.IsBranch: - ld.Branch = v.String() - case gps.IsSemver, gps.IsVersion: - ld.Version = v.String() - } + ld.Revision, ld.Branch, ld.Version = getVersionInfo(v) raw.P[k] = ld } @@ -134,6 +118,29 @@ func (l *Lock) MarshalJSON() ([]byte, error) { return buf.Bytes(), err } +// TODO(carolynvs) this should be moved to gps +func getVersionInfo(v gps.Version) (revision string, branch string, version string) { + // Figure out how to get the underlying revision + switch tv := v.(type) { + case gps.UnpairedVersion: + // TODO we could error here, if we want to be very defensive about not + // allowing a lock to be written if without an immmutable revision + case gps.Revision: + revision = tv.String() + case gps.PairedVersion: + revision = tv.Underlying().String() + } + + switch v.Type() { + case gps.IsBranch: + branch = v.String() + case gps.IsSemver, gps.IsVersion: + version = v.String() + } + + return +} + // LockFromInterface converts an arbitrary gps.Lock to dep's representation of a // lock. If the input is already dep's *lock, the input is returned directly. // @@ -177,14 +184,3 @@ func (s SortedLockedProjects) Less(i, j int) bool { return l.Source < r.Source } - -// locksAreEquivalent compares two locks to see if they differ. If EITHER lock -// is nil, or their memos do not match, or any projects differ, then false is -// returned. -func locksAreEquivalent(l, r *Lock) bool { - if l == nil || r == nil { - return false - } - - return gps.LocksAreEq(l, r, true) -} diff --git a/lock_test.go b/lock_test.go index ea9318ed82..be6cc24288 100644 --- a/lock_test.go +++ b/lock_test.go @@ -19,7 +19,9 @@ func TestReadLock(t *testing.T) { defer h.Cleanup() golden := "lock/golden0.json" - got, err := readLock(h.GetTestFile(golden)) + g0f := h.GetTestFile(golden) + defer g0f.Close() + got, err := readLock(g0f) if err != nil { t.Fatalf("Should have read Lock correctly, but got err %q", err) } @@ -41,7 +43,9 @@ func TestReadLock(t *testing.T) { } golden = "lock/golden1.json" - got, err = readLock(h.GetTestFile(golden)) + g1f := h.GetTestFile(golden) + defer g1f.Close() + got, err = readLock(g1f) if err != nil { t.Fatalf("Should have read Lock correctly, but got err %q", err) } @@ -141,7 +145,9 @@ func TestReadLockErrors(t *testing.T) { } for _, tst := range tests { - _, err = readLock(h.GetTestFile(tst.file)) + lf := h.GetTestFile(tst.file) + defer lf.Close() + _, err = readLock(lf) if err == nil { t.Errorf("Reading lock with %s should have caused error, but did not", tst.name) } else if !strings.Contains(err.Error(), tst.name) { diff --git a/manifest_test.go b/manifest_test.go index d064c3763c..38b40d82ed 100644 --- a/manifest_test.go +++ b/manifest_test.go @@ -17,7 +17,9 @@ func TestReadManifest(t *testing.T) { h := test.NewHelper(t) defer h.Cleanup() - got, err := readManifest(h.GetTestFile("manifest/golden.json")) + mf := h.GetTestFile("manifest/golden.json") + defer mf.Close() + got, err := readManifest(mf) if err != nil { t.Fatalf("Should have read Manifest correctly, but got err %q", err) } @@ -106,7 +108,9 @@ func TestReadManifestErrors(t *testing.T) { } for _, tst := range tests { - _, err = readManifest(h.GetTestFile(tst.file)) + mf := h.GetTestFile(tst.file) + defer mf.Close() + _, err = readManifest(mf) if err == nil { t.Errorf("Reading manifest with %s should have caused error, but did not", tst.name) } else if !strings.Contains(err.Error(), tst.name) { diff --git a/test/integration_testcase.go b/test/integration_testcase.go index 8c10e434a7..bfc967c62b 100644 --- a/test/integration_testcase.go +++ b/test/integration_testcase.go @@ -24,6 +24,7 @@ type IntegrationTestCase struct { RootPath string InitialPath string FinalPath string + ErrorExpected string `json:"error-expected"` Commands [][]string `json:"commands"` GopathInitial map[string]string `json:"gopath-initial"` VendorInitial map[string]string `json:"vendor-initial"` @@ -54,6 +55,25 @@ func NewTestCase(t *testing.T, name string) *IntegrationTestCase { return n } +// CompareError compares expected error to error recived. +func (tc *IntegrationTestCase) CompareError(err error) { + wantExists, want := len(tc.ErrorExpected) > 0, tc.ErrorExpected + gotExists, got := err != nil, "" + if gotExists { + got = err.Error() + } + + if wantExists && gotExists { + if want != got { + tc.t.Errorf("expected error %s, got error %s", want, got) + } + } else if !wantExists && gotExists { + tc.t.Fatalf("%s error raised where none was expected", got) + } else if wantExists && !gotExists { + tc.t.Errorf("%s error was not logged where one was expected", want) + } +} + func (tc *IntegrationTestCase) CompareFile(goldenPath, working string) { golden := filepath.Join(tc.FinalPath, goldenPath) diff --git a/test/test.go b/test/test.go index 8af9f40fa5..52ffbf9c99 100644 --- a/test/test.go +++ b/test/test.go @@ -68,7 +68,7 @@ func (h *Helper) Must(err error) { // check gives a test non-fatal error if err is not nil. func (h *Helper) check(err error) { if err != nil { - h.t.Error(err) + h.t.Errorf("%+v", err) } } @@ -112,10 +112,12 @@ func (h *Helper) Cd(dir string) { h.wd = h.pwd() } abs, err := filepath.Abs(dir) - h.Must(os.Chdir(dir)) if err == nil { h.Setenv("PWD", abs) } + + err = os.Chdir(dir) + h.Must(errors.Wrapf(err, "Unable to cd to %s", dir)) } // Setenv sets an environment variable to use when running the test go @@ -437,22 +439,30 @@ func (h *Helper) WriteTestFile(src string, content string) error { return err } -// GetTestFile reads a file from the testdata directory into memory. src is -// relative to ./testdata. -func (h *Helper) GetTestFile(src string) io.ReadCloser { - content, err := os.Open(filepath.Join(h.origWd, "testdata", src)) +// GetTestFile reads a file into memory +func (h *Helper) GetFile(path string) io.ReadCloser { + content, err := os.Open(path) if err != nil { - panic(err) + h.t.Fatalf("%+v", errors.Wrapf(err, "Unable to open file: %s", path)) } return content } +// GetTestFile reads a file from the testdata directory into memory. src is +// relative to ./testdata. +func (h *Helper) GetTestFile(src string) io.ReadCloser { + fullPath := filepath.Join(h.origWd, "testdata", src) + return h.GetFile(fullPath) +} + // GetTestFileString reads a file from the testdata directory into memory. src is // relative to ./testdata. func (h *Helper) GetTestFileString(src string) string { - content, err := ioutil.ReadAll(h.GetTestFile(src)) + srcf := h.GetTestFile(src) + defer srcf.Close() + content, err := ioutil.ReadAll(srcf) if err != nil { - panic(err) + h.t.Fatalf("%+v", err) } return string(content) } @@ -498,16 +508,25 @@ func (h *Helper) Path(name string) string { // Ensure it's the absolute, symlink-less path we're returning abs, err := filepath.EvalSymlinks(joined) if err != nil { - h.t.Fatalf("internal testsuite error: could not get absolute path for dir(%q), err %q", joined, err) + h.t.Fatalf("%+v", errors.Wrapf(err, "internal testsuite error: could not get absolute path for dir(%q)", joined)) } return abs } // MustExist fails if path does not exist. func (h *Helper) MustExist(path string) { + if err := h.ShouldExist(path); err != nil { + h.t.Fatalf("%+v", err) + } +} + +// ShouldExist returns an error if path does not exist. +func (h *Helper) ShouldExist(path string) error { if !h.Exist(path) { - h.t.Fatalf("%+v", errors.Errorf("%s does not exist but should", path)) + return errors.Errorf("%s does not exist but should", path) } + + return nil } // Exist returns whether or not a path exists @@ -524,9 +543,18 @@ func (h *Helper) Exist(path string) bool { // MustNotExist fails if path exists. func (h *Helper) MustNotExist(path string) { + if err := h.ShouldNotExist(path); err != nil { + h.t.Fatalf("%+v", err) + } +} + +// ShouldNotExist returns an error if path exists. +func (h *Helper) ShouldNotExist(path string) error { if h.Exist(path) { - h.t.Fatalf("%+v", errors.Errorf("%s exists but should not", path)) + return errors.Errorf("%s exists but should not", path) } + + return nil } // Cleanup cleans up a test that runs testgo. diff --git a/test_project_context_test.go b/test_project_context_test.go new file mode 100644 index 0000000000..974f35cbff --- /dev/null +++ b/test_project_context_test.go @@ -0,0 +1,160 @@ +// Copyright 2016 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 dep + +import ( + "path/filepath" + + "github.com/golang/dep/test" + "github.com/pkg/errors" + "github.com/sdboyer/gps" +) + +// TestProjectContext groups together test project files and helps test them +type TestProjectContext struct { + h *test.Helper + tempDir string // Full path to the temp directory + tempProjectDir string // Relative path of the project under the temp directory + + Context *Ctx + Project *Project + SourceManager *gps.SourceMgr +} + +// NewTestProjectContext creates a new on-disk test project +func NewTestProjectContext(h *test.Helper, projectName string) *TestProjectContext { + pc := &TestProjectContext{h: h} + + // Create the test project directory + pc.tempProjectDir = filepath.Join("src", projectName) + h.TempDir(pc.tempProjectDir) + pc.tempDir = h.Path(".") + pc.Project = &Project{AbsRoot: filepath.Join(pc.tempDir, pc.tempProjectDir)} + h.Cd(pc.Project.AbsRoot) + h.Setenv("GOPATH", pc.tempDir) + + // Setup a Source Manager + var err error + pc.Context = &Ctx{GOPATH: pc.tempDir} + pc.SourceManager, err = pc.Context.SourceManager() + h.Must(errors.Wrap(err, "Unable to create a SourceManager")) + + return pc +} + +// CopyFile copies a file from the testdata directory into the project +// projectPath is the destination file path, relative to the project directory +// testdataPath is the source path, relative to the testdata directory +func (pc *TestProjectContext) CopyFile(projectPath string, testdataPath string) string { + path := filepath.Join(pc.tempProjectDir, projectPath) + pc.h.TempCopy(path, testdataPath) + return path +} + +func (pc *TestProjectContext) Load() { + // TODO(carolynvs): Can't use Ctx.LoadProject until dep doesn't require a manifest.json at the project root or it also looks for lock.json + var err error + var m *Manifest + mp := pc.getManifestPath() + if pc.h.Exist(mp) { + mf := pc.h.GetFile(mp) + defer mf.Close() + m, err = readManifest(mf) + pc.h.Must(err) + } + var l *Lock + lp := pc.getLockPath() + if pc.h.Exist(lp) { + lf := pc.h.GetFile(lp) + defer lf.Close() + l, err = readLock(lf) + pc.h.Must(err) + } + pc.Project.Manifest = m + pc.Project.Lock = l +} + +// GetLockPath returns the full path to the lock +func (pc *TestProjectContext) getLockPath() string { + return filepath.Join(pc.Project.AbsRoot, LockName) +} + +// GetManifestPath returns the full path to the manifest +func (pc *TestProjectContext) getManifestPath() string { + return filepath.Join(pc.Project.AbsRoot, ManifestName) +} + +// GetVendorPath returns the full path to the vendor directory +func (pc *TestProjectContext) getVendorPath() string { + return filepath.Join(pc.Project.AbsRoot, "vendor") +} + +// LockShouldMatchGolden returns an error when the lock does not match the golden lock. +// goldenLockPath is the path to the golden lock file relative to the testdata directory +// Updates the golden file when -UpdateGolden flag is present. +func (pc *TestProjectContext) LockShouldMatchGolden(goldenLockPath string) error { + got := pc.h.ReadLock() + return pc.ShouldMatchGolden(goldenLockPath, got) +} + +// LockShouldNotExist returns an error when the lock exists. +func (pc *TestProjectContext) LockShouldNotExist() error { + return pc.h.ShouldNotExist(pc.getLockPath()) +} + +// ManifestShouldMatchGolden returns an error when the manifest does not match the golden manifest. +// goldenManifestPath is the path to the golden manifest file, relative to the testdata directory +// Updates the golden file when -UpdateGolden flag is present +func (pc *TestProjectContext) ManifestShouldMatchGolden(goldenManifestPath string) error { + got := pc.h.ReadManifest() + return pc.ShouldMatchGolden(goldenManifestPath, got) +} + +// ManifestShouldNotExist returns an error when the lock exists. +func (pc *TestProjectContext) ManifestShouldNotExist() error { + return pc.h.ShouldNotExist(pc.getManifestPath()) +} + +// ShouldMatchGolden returns an error when a file does not match the golden file. +// goldenFile is the path to the golden file, relative to the testdata directory +// Updates the golden file when -UpdateGolden flag is present +func (pc *TestProjectContext) ShouldMatchGolden(goldenFile string, got string) error { + want := pc.h.GetTestFileString(goldenFile) + if want != got { + if *test.UpdateGolden { + if err := pc.h.WriteTestFile(goldenFile, got); err != nil { + return errors.Wrapf(err, "Unable to write updated golden file %s", goldenFile) + } + } else { + return errors.Errorf("expected %s, got %s", want, got) + } + } + + return nil +} + +// VendorShouldExist returns an error when the vendor directory does not exist. +func (pc *TestProjectContext) VendorShouldExist() error { + return pc.h.ShouldExist(pc.getVendorPath()) +} + +// VendorFileShouldExist returns an error when the specified file does not exist in vendor. +// filePath is the relative path to the file within vendor +func (pc *TestProjectContext) VendorFileShouldExist(filePath string) error { + fullPath := filepath.Join(pc.getVendorPath(), filePath) + return pc.h.ShouldExist(fullPath) +} + +// VendorShouldNotExist returns an error when the vendor directory exists. +func (pc *TestProjectContext) VendorShouldNotExist() error { + return pc.h.ShouldNotExist(pc.getVendorPath()) +} + +// Release cleans up after test objects created by this instance +func (pc *TestProjectContext) Release() { + if pc.SourceManager != nil { + pc.SourceManager.Release() + } +} diff --git a/testdata/txn_writer/badinput_fileroot b/testdata/txn_writer/badinput_fileroot new file mode 100644 index 0000000000..e69de29bb2 diff --git a/testdata/txn_writer/expected_diff_output.txt b/testdata/txn_writer/expected_diff_output.txt new file mode 100644 index 0000000000..3c59f84304 --- /dev/null +++ b/testdata/txn_writer/expected_diff_output.txt @@ -0,0 +1,33 @@ +Add: [ + { + "name": "github.com/stuff/realthing", + "version": "2.0.0", + "revision": "1f02e52d6bac308da54ab84a234c58a98ca82347", + "packages": [ + "." + ] + } +] +Remove: [ + { + "name": "github.com/stuff/placeholder", + "version": "2.0.0", + "revision": "6694017eeb4e20fd277b049bf29dba4895c97234", + "packages": [ + "." + ] + } +] +Modify: [ + { + "name": "github.com/foo/bar", + "repo": "+ http://github.example.com/foo/bar", + "version": "+ 1.2.0", + "branch": "- master", + "revision": "f24338400f072ef18125ae0fbe6b06fe6d1783e7 -> 2a3a211e171803acb82d1d5d42ceb53228f51751", + "packages": [ + "- placeholder", + "+ thing" + ] + } +] diff --git a/testdata/txn_writer/original_lock.json b/testdata/txn_writer/original_lock.json new file mode 100644 index 0000000000..6e0effd2e7 --- /dev/null +++ b/testdata/txn_writer/original_lock.json @@ -0,0 +1,22 @@ +{ + "memo": "595716d270828e763c811ef79c9c41f85b1d1bfbdfe85280036405c03772206c", + "projects": [ + { + "name": "github.com/foo/bar", + "branch": "master", + "revision": "f24338400f072ef18125ae0fbe6b06fe6d1783e7", + "packages": [ + "placeholder", + "util" + ] + }, + { + "name": "github.com/stuff/placeholder", + "version": "2.0.0", + "revision": "6694017eeb4e20fd277b049bf29dba4895c97234", + "packages": [ + "." + ] + } + ] +} diff --git a/testdata/txn_writer/updated_lock.json b/testdata/txn_writer/updated_lock.json new file mode 100644 index 0000000000..f9be290f27 --- /dev/null +++ b/testdata/txn_writer/updated_lock.json @@ -0,0 +1,23 @@ +{ + "memo": "2252a285ab27944a4d7adcba8dbd03980f59ba652f12db39fa93b927c345593e", + "projects": [ + { + "name": "github.com/foo/bar", + "repo": "http://github.example.com/foo/bar", + "version": "1.2.0", + "revision": "2a3a211e171803acb82d1d5d42ceb53228f51751", + "packages": [ + "thing", + "util" + ] + }, + { + "name": "github.com/stuff/realthing", + "version": "2.0.0", + "revision": "1f02e52d6bac308da54ab84a234c58a98ca82347", + "packages": [ + "." + ] + } + ] +} diff --git a/txn_writer.go b/txn_writer.go index 606e1e78a2..bf92a46656 100644 --- a/txn_writer.go +++ b/txn_writer.go @@ -5,10 +5,14 @@ package dep import ( + "bytes" + "encoding/json" "fmt" "io/ioutil" "os" "path/filepath" + "sort" + "strings" "github.com/pkg/errors" "github.com/sdboyer/gps" @@ -21,85 +25,212 @@ import ( // It is not impervious to errors (writing to disk is hard), but it should // guard against non-arcane failure conditions. type SafeWriter struct { - Root string // absolute path of root dir in which to write - Manifest *Manifest // the manifest to write, if any - Lock *Lock // the old lock, if any - NewLock gps.Lock // the new lock, if any - SourceManager gps.SourceManager + Payload *SafeWriterPayload } -// WriteAllSafe writes out some combination of config yaml, lock, and a vendor -// tree, to a temp dir, then moves them into place if and only if all the write -// operations succeeded. It also does its best to roll back if any moves fail. -// -// This mostly guarantees that dep cannot exit with a partial write that would -// leave an undefined state on disk. +// SafeWriterPayload represents the actions SafeWriter will execute when SafeWriter.Write is called. +type SafeWriterPayload struct { + Manifest *Manifest + Lock *Lock + LockDiff *LockDiff + ForceWriteVendor bool +} + +func (payload *SafeWriterPayload) HasLock() bool { + return payload.Lock != nil +} + +func (payload *SafeWriterPayload) HasManifest() bool { + return payload.Manifest != nil +} + +func (payload *SafeWriterPayload) HasVendor() bool { + // TODO(carolynvs) this can be calculated based on if we are writing the lock + // init -> switch to newlock + // ensure checks existence, why not move that into the prep? + return payload.ForceWriteVendor +} + +// LockDiff is the set of differences between an existing lock file and an updated lock file. +// Fields are only populated when there is a difference, otherwise they are empty. +// TODO(carolynvs) this should be moved to gps +type LockDiff struct { + HashDiff *StringDiff + Add []LockedProjectDiff + Remove []LockedProjectDiff + Modify []LockedProjectDiff +} + +func (diff *LockDiff) Format() (string, error) { + if diff == nil { + return "", nil + } + + var buf bytes.Buffer + + if len(diff.Add) > 0 { + buf.WriteString("Add: ") + + enc := json.NewEncoder(&buf) + enc.SetIndent("", " ") + enc.SetEscapeHTML(false) + err := enc.Encode(diff.Add) + if err != nil { + return "", errors.Wrap(err, "Unable to format LockDiff.Add") + } + } + + if len(diff.Remove) > 0 { + buf.WriteString("Remove: ") + + enc := json.NewEncoder(&buf) + enc.SetIndent("", " ") + enc.SetEscapeHTML(false) + err := enc.Encode(diff.Remove) + if err != nil { + return "", errors.Wrap(err, "Unable to format LockDiff.Remove") + } + } + + if len(diff.Modify) > 0 { + buf.WriteString("Modify: ") + + enc := json.NewEncoder(&buf) + enc.SetIndent("", " ") + enc.SetEscapeHTML(false) + err := enc.Encode(diff.Modify) + if err != nil { + return "", errors.Wrap(err, "Unable to format LockDiff.Modify") + } + } + + return buf.String(), nil +} + +// LockedProjectDiff contains the before and after snapshot of a project reference. +// Fields are only populated when there is a difference, otherwise they are empty. +// TODO(carolynvs) this should be moved to gps +type LockedProjectDiff struct { + Name gps.ProjectRoot `json:"name"` + Source *StringDiff `json:"repo,omitempty"` + Version *StringDiff `json:"version,omitempty"` + Branch *StringDiff `json:"branch,omitempty"` + Revision *StringDiff `json:"revision,omitempty"` + Packages []StringDiff `json:"packages,omitempty"` +} + +type StringDiff struct { + Previous string + Current string +} + +func (diff StringDiff) MarshalJSON() ([]byte, error) { + var value string + + if diff.Previous == "" && diff.Current != "" { + value = fmt.Sprintf("+ %s", diff.Current) + } else if diff.Previous != "" && diff.Current == "" { + value = fmt.Sprintf("- %s", diff.Previous) + } else if diff.Previous != diff.Current { + value = fmt.Sprintf("%s -> %s", diff.Previous, diff.Current) + } else { + value = diff.Current + } + + var buf bytes.Buffer + enc := json.NewEncoder(&buf) + enc.SetEscapeHTML(false) + err := enc.Encode(value) + + return buf.Bytes(), err +} + +// Prepare to write a set of config yaml, lock and vendor tree. // -// - If a sw.Manifest is provided, it will be written to the standard manifest file -// name beneath sw.Root -// - If sw.Lock is provided without an sw.NewLock, it will be written to the standard +// - If manifest is provided, it will be written to the standard manifest file +// name beneath root. +// - If lock is provided it will be written to the standard // lock file name in the root dir, but vendor will NOT be written -// - If sw.Lock and sw.NewLock are both provided and are equivalent, then neither lock +// - If lock and newLock are both provided and are equivalent, then neither lock // nor vendor will be written -// - If sw.Lock and sw.NewLock are both provided and are not equivalent, -// the nl will be written to the same location as above, and a vendor -// tree will be written to sw.Root/vendor -// - If sw.NewLock is provided and sw.Lockock is not, it will write both a lock -// and vendor dir in the same way -// - If the forceVendor param is true, then vendor will be unconditionally -// written out based on sw.NewLock if present, else sw.Lock, else error. -// -// Any of m, l, or nl can be omitted; the grouped write operation will continue -// for whichever inputs are present. A SourceManager is only required if vendor -// is being written. -func (sw SafeWriter) WriteAllSafe(forceVendor bool) error { - // Decide which writes we need to do - var writeM, writeL, writeV bool - writeV = forceVendor - - if sw.Manifest != nil { - writeM = true +// - If lock and newLock are both provided and are not equivalent, +// the newLock will be written to the same location as above, and a vendor +// tree will be written to the vendor directory +// - If newLock is provided and lock is not, it will write both a lock +// and the vendor directory in the same way +// - If the forceVendor param is true, then vendor/ will be unconditionally +// written out based on newLock if present, else lock, else error. +func (sw *SafeWriter) Prepare(manifest *Manifest, lock *Lock, newLock *Lock, forceVendor bool) { + sw.Payload = &SafeWriterPayload{ + Manifest: manifest, + ForceWriteVendor: forceVendor, } - if sw.NewLock != nil { - if sw.Lock == nil { - writeL, writeV = true, true + if newLock != nil { + if lock == nil { + sw.Payload.Lock = newLock + sw.Payload.ForceWriteVendor = true } else { - rlf := LockFromInterface(sw.NewLock) - if !locksAreEquivalent(rlf, sw.Lock) { - writeL, writeV = true, true + diff := diffLocks(lock, newLock) + if diff != nil { + sw.Payload.Lock = newLock + sw.Payload.LockDiff = diff + sw.Payload.ForceWriteVendor = true } } - } else if sw.Lock != nil { - writeL = true + } else if lock != nil { + sw.Payload.Lock = lock } +} - if sw.Root == "" { +func (payload SafeWriterPayload) validate(root string, sm gps.SourceManager) error { + if root == "" { return errors.New("root path must be non-empty") } - if is, err := IsDir(sw.Root); !is { + if is, err := IsDir(root); !is { if err != nil { return err } - return fmt.Errorf("root path %q does not exist", sw.Root) + return fmt.Errorf("root path %q does not exist", root) } - if !writeM && !writeL && !writeV { - // nothing to do - return nil - } - - if writeV && sw.SourceManager == nil { + if payload.HasVendor() && sm == nil { return errors.New("must provide a SourceManager if writing out a vendor dir") } - if writeV && sw.Lock == nil && sw.NewLock == nil { + if payload.HasVendor() && payload.Lock == nil { return errors.New("must provide a lock in order to write out vendor") } - mpath := filepath.Join(sw.Root, ManifestName) - lpath := filepath.Join(sw.Root, LockName) - vpath := filepath.Join(sw.Root, "vendor") + return nil +} + +// Write saves some combination of config yaml, lock, and a vendor tree. +// root is the absolute path of root dir in which to write. +// sm is only required if vendor is being written. +// +// It first writes to a temp dir, then moves them in place if and only if all the write +// operations succeeded. It also does its best to roll back if any moves fail. +// This mostly guarantees that dep cannot exit with a partial write that would +// leave an undefined state on disk. +func (sw *SafeWriter) Write(root string, sm gps.SourceManager) error { + if sw.Payload == nil { + return errors.New("Cannot call SafeWriter.Write before SafeWriter.Prepare") + } + + err := sw.Payload.validate(root, sm) + if err != nil { + return err + } + + if !sw.Payload.HasManifest() && !sw.Payload.HasLock() && !sw.Payload.HasVendor() { + // nothing to do + return nil + } + + mpath := filepath.Join(root, ManifestName) + lpath := filepath.Join(root, LockName) + vpath := filepath.Join(root, "vendor") td, err := ioutil.TempDir(os.TempDir(), "dep") if err != nil { @@ -107,35 +238,20 @@ func (sw SafeWriter) WriteAllSafe(forceVendor bool) error { } defer os.RemoveAll(td) - if writeM { - if err := writeFile(filepath.Join(td, ManifestName), sw.Manifest); err != nil { + if sw.Payload.HasManifest() { + if err := writeFile(filepath.Join(td, ManifestName), sw.Payload.Manifest); err != nil { return errors.Wrap(err, "failed to write manifest file to temp dir") } } - if writeL { - if sw.NewLock == nil { - // the new lock is nil but the flag is on, so we must be writing - // the other one - if err := writeFile(filepath.Join(td, LockName), sw.Lock); err != nil { - return errors.Wrap(err, "failed to write lock file to temp dir") - } - } else { - rlf := LockFromInterface(sw.NewLock) - if err := writeFile(filepath.Join(td, LockName), rlf); err != nil { - return errors.Wrap(err, "failed to write lock file to temp dir") - } + if sw.Payload.HasLock() { + if err := writeFile(filepath.Join(td, LockName), sw.Payload.Lock); err != nil { + return errors.Wrap(err, "failed to write lock file to temp dir") } } - if writeV { - // Prefer the nl, but take the l if only that's available, as could be the - // case if true was passed for forceVendor. - l := sw.NewLock - if l == nil { - l = sw.Lock - } - err = gps.WriteDepTree(filepath.Join(td, "vendor"), l, sw.SourceManager, true) + if sw.Payload.HasVendor() { + err = gps.WriteDepTree(filepath.Join(td, "vendor"), sw.Payload.Lock, sm, true) if err != nil { return errors.Wrap(err, "error while writing out vendor tree") } @@ -150,7 +266,7 @@ func (sw SafeWriter) WriteAllSafe(forceVendor bool) error { var failerr error var vendorbak string - if writeM { + if sw.Payload.HasManifest() { if _, err := os.Stat(mpath); err == nil { // Move out the old one. tmploc := filepath.Join(td, ManifestName+".orig") @@ -168,7 +284,7 @@ func (sw SafeWriter) WriteAllSafe(forceVendor bool) error { } } - if writeL { + if sw.Payload.HasLock() { if _, err := os.Stat(lpath); err == nil { // Move out the old one. tmploc := filepath.Join(td, LockName+".orig") @@ -187,7 +303,7 @@ func (sw SafeWriter) WriteAllSafe(forceVendor bool) error { } } - if writeV { + if sw.Payload.HasVendor() { if _, err := os.Stat(vpath); err == nil { // Move out the old vendor dir. just do it into an adjacent dir, to // try to mitigate the possibility of a pointless cross-filesystem @@ -215,7 +331,7 @@ func (sw SafeWriter) WriteAllSafe(forceVendor bool) error { // Renames all went smoothly. The deferred os.RemoveAll will get the temp // dir, but if we wrote vendor, we have to clean that up directly - if writeV { + if sw.Payload.HasVendor() { // Nothing we can really do about an error at this point, so ignore it os.RemoveAll(vendorbak) } @@ -230,3 +346,227 @@ fail: } return failerr } + +func (sw *SafeWriter) PrintPreparedActions() error { + if sw.Payload.HasManifest() { + fmt.Println("Would have written the following manifest.json:") + m, err := sw.Payload.Manifest.MarshalJSON() + if err != nil { + return errors.Wrap(err, "ensure DryRun cannot serialize manifest") + } + fmt.Println(string(m)) + } + + if sw.Payload.HasLock() { + fmt.Println("Would have written the following changes to lock.json:") + diff, err := sw.Payload.LockDiff.Format() + if err != nil { + return errors.Wrap(err, "ensure DryRun cannot serialize the lock diff") + } + fmt.Println(diff) + } + + if sw.Payload.HasVendor() { + fmt.Println("Would have written the following projects to the vendor directory:") + for _, project := range sw.Payload.Lock.Projects() { + prj := project.Ident() + rev, _, _ := getVersionInfo(project.Version()) + if prj.Source == "" { + fmt.Printf("%s@%s\n", prj.ProjectRoot, rev) + } else { + fmt.Printf("%s -> %s@%s\n", prj.ProjectRoot, prj.Source, rev) + } + } + } + + return nil +} + +// diffLocks compares two locks and identifies the differences between them. +// Returns nil if there are no differences. +// TODO(carolynvs) this should be moved to gps +func diffLocks(l1 gps.Lock, l2 gps.Lock) *LockDiff { + // Default nil locks to empty locks, so that we can still generate a diff + if l1 == nil { + l1 = &gps.SimpleLock{} + } + if l2 == nil { + l2 = &gps.SimpleLock{} + } + + p1, p2 := l1.Projects(), l2.Projects() + + // Check if the slices are sorted already. If they are, we can compare + // without copying. Otherwise, we have to copy to avoid altering the + // original input. + sp1, sp2 := SortedLockedProjects(p1), SortedLockedProjects(p2) + if len(p1) > 1 && !sort.IsSorted(sp1) { + p1 = make([]gps.LockedProject, len(p1)) + copy(p1, l1.Projects()) + sort.Sort(SortedLockedProjects(p1)) + } + if len(p2) > 1 && !sort.IsSorted(sp2) { + p2 = make([]gps.LockedProject, len(p2)) + copy(p2, l2.Projects()) + sort.Sort(SortedLockedProjects(p2)) + } + + diff := LockDiff{} + + h1 := l1.InputHash() + h2 := l2.InputHash() + if !bytes.Equal(h1, h2) { + diff.HashDiff = &StringDiff{Previous: string(h1), Current: string(h2)} + } + + var i2next int + for i1 := 0; i1 < len(p1); i1++ { + lp1 := p1[i1] + pr1 := lp1.Ident().ProjectRoot + + var matched bool + for i2 := i2next; i2 < len(p2); i2++ { + lp2 := p2[i2] + pr2 := lp2.Ident().ProjectRoot + + switch strings.Compare(string(pr1), string(pr2)) { + case 0: // Found a matching project + matched = true + pdiff := diffProjects(lp1, lp2) + if pdiff != nil { + diff.Modify = append(diff.Modify, *pdiff) + } + i2next = i2 + 1 // Don't evaluate to this again + case -1: // Found a new project + add := buildLockedProjectDiff(lp2) + diff.Add = append(diff.Add, add) + i2next = i2 + 1 // Don't evaluate to this again + continue // Keep looking for a matching project + case +1: // Project has been removed, handled below + break + } + + break // Done evaluating this project, move onto the next + } + + if !matched { + remove := buildLockedProjectDiff(lp1) + diff.Remove = append(diff.Remove, remove) + } + } + + // Anything that still hasn't been evaluated are adds + for i2 := i2next; i2 < len(p2); i2++ { + lp2 := p2[i2] + add := buildLockedProjectDiff(lp2) + diff.Add = append(diff.Add, add) + } + + if diff.HashDiff == nil && len(diff.Add) == 0 && len(diff.Remove) == 0 && len(diff.Modify) == 0 { + return nil // The locks are the equivalent + } + return &diff +} + +func buildLockedProjectDiff(lp gps.LockedProject) LockedProjectDiff { + r2, b2, v2 := getVersionInfo(lp.Version()) + var rev, version, branch *StringDiff + if r2 != "" { + rev = &StringDiff{Previous: r2, Current: r2} + } + if b2 != "" { + branch = &StringDiff{Previous: b2, Current: b2} + } + if v2 != "" { + version = &StringDiff{Previous: v2, Current: v2} + } + add := LockedProjectDiff{ + Name: lp.Ident().ProjectRoot, + Revision: rev, + Version: version, + Branch: branch, + Packages: make([]StringDiff, len(lp.Packages())), + } + for i, pkg := range lp.Packages() { + add.Packages[i] = StringDiff{Previous: pkg, Current: pkg} + } + return add +} + +// diffProjects compares two projects and identifies the differences between them. +// Returns nil if there are no differences +// TODO(carolynvs) this should be moved to gps and updated once the gps unexported fields are available to use. +func diffProjects(lp1 gps.LockedProject, lp2 gps.LockedProject) *LockedProjectDiff { + diff := LockedProjectDiff{Name: lp1.Ident().ProjectRoot} + + s1 := lp1.Ident().Source + s2 := lp2.Ident().Source + if s1 != s2 { + diff.Source = &StringDiff{Previous: s1, Current: s2} + } + + r1, b1, v1 := getVersionInfo(lp1.Version()) + r2, b2, v2 := getVersionInfo(lp2.Version()) + if r1 != r2 { + diff.Revision = &StringDiff{Previous: r1, Current: r2} + } + if b1 != b2 { + diff.Branch = &StringDiff{Previous: b1, Current: b2} + } + if v1 != v2 { + diff.Version = &StringDiff{Previous: v1, Current: v2} + } + + p1 := lp1.Packages() + p2 := lp2.Packages() + if !sort.StringsAreSorted(p1) { + p1 = make([]string, len(p1)) + copy(p1, lp1.Packages()) + sort.Strings(p1) + } + if !sort.StringsAreSorted(p2) { + p2 = make([]string, len(p2)) + copy(p2, lp2.Packages()) + sort.Strings(p2) + } + + var i2next int + for i1 := 0; i1 < len(p1); i1++ { + pkg1 := p1[i1] + + var matched bool + for i2 := i2next; i2 < len(p2); i2++ { + pkg2 := p2[i2] + + switch strings.Compare(pkg1, pkg2) { + case 0: // Found matching package + matched = true + i2next = i2 + 1 // Don't evaluate to this again + case +1: // Found a new package + add := StringDiff{Current: pkg2} + diff.Packages = append(diff.Packages, add) + i2next = i2 + 1 // Don't evaluate to this again + continue // Keep looking for a match + case -1: // Package has been removed (handled below) + } + + break // Done evaluating this package, move onto the next + } + + if !matched { + diff.Packages = append(diff.Packages, StringDiff{Previous: pkg1}) + } + } + + // Anything that still hasn't been evaluated are adds + for i2 := i2next; i2 < len(p2); i2++ { + pkg2 := p2[i2] + add := StringDiff{Current: pkg2} + diff.Packages = append(diff.Packages, add) + } + + if diff.Source == nil && diff.Version == nil && diff.Revision == nil && len(diff.Packages) == 0 { + return nil // The projects are equivalent + } + return &diff +} diff --git a/txn_writer_test.go b/txn_writer_test.go index fd994b92ff..925836ca5e 100644 --- a/txn_writer_test.go +++ b/txn_writer_test.go @@ -5,203 +5,434 @@ package dep import ( - "os" "path/filepath" - "strconv" + "strings" "testing" "github.com/golang/dep/test" + "github.com/pkg/errors" ) -func TestTxnWriterBadInputs(t *testing.T) { +const safeWriterProject = "safewritertest" +const safeWriterGoldenManifest = "txn_writer/expected_manifest.json" +const safeWriterGoldenLock = "txn_writer/expected_lock.json" + +func TestSafeWriter_BadInput_MissingRoot(t *testing.T) { h := test.NewHelper(t) defer h.Cleanup() + pc := NewTestProjectContext(h, safeWriterProject) + defer pc.Release() + + var sw SafeWriter + sw.Prepare(nil, nil, nil, false) - h.TempDir("txnwriter") - td := h.Path(".") + err := sw.Write("", pc.SourceManager) + + if err == nil { + t.Fatal("should have errored without a root path, but did not") + } else if !strings.Contains(err.Error(), "root path") { + t.Fatalf("expected root path error, got %s", err.Error()) + } +} + +func TestSafeWriter_BadInput_MissingSourceManager(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + pc := NewTestProjectContext(h, safeWriterProject) + defer pc.Release() var sw SafeWriter + sw.Prepare(nil, nil, nil, true) - // no root - if err := sw.WriteAllSafe(false); err == nil { - t.Errorf("should have errored without a root path, but did not") + err := sw.Write(pc.Project.AbsRoot, nil) + + if err == nil { + t.Fatal("should have errored without a source manager when forceVendor is true, but did not") + } else if !strings.Contains(err.Error(), "SourceManager") { + t.Fatalf("expected SourceManager error, got %s", err.Error()) } - sw.Root = td +} - if err := sw.WriteAllSafe(false); err != nil { - t.Errorf("write with only root should be fine, just a no-op, but got err %q", err) +func TestSafeWriter_BadInput_MissingLock(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + pc := NewTestProjectContext(h, safeWriterProject) + defer pc.Release() + + var sw SafeWriter + sw.Prepare(nil, nil, nil, true) + + err := sw.Write(pc.Project.AbsRoot, pc.SourceManager) + + if err == nil { + t.Fatal("should have errored without a lock when forceVendor is true, but did not") + } else if !strings.Contains(err.Error(), "lock") { + t.Fatalf("expected lock error, got %s", err.Error()) } - if err := sw.WriteAllSafe(true); err == nil { - t.Errorf("should fail because no source manager was provided for writing vendor") +} + +func TestSafeWriter_BadInput_NonexistentRoot(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + pc := NewTestProjectContext(h, safeWriterProject) + defer pc.Release() + + var sw SafeWriter + sw.Prepare(nil, nil, nil, true) + + missingroot := filepath.Join(pc.Project.AbsRoot, "nonexistent") + err := sw.Write(missingroot, pc.SourceManager) + + if err == nil { + t.Fatal("should have errored with nonexistent dir for root path, but did not") + } else if !strings.Contains(err.Error(), "does not exist") { + t.Fatalf("expected does not exist error, got %s", err.Error()) + } +} + +func TestSafeWriter_BadInput_RootIsFile(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + pc := NewTestProjectContext(h, safeWriterProject) + defer pc.Release() + + var sw SafeWriter + sw.Prepare(nil, nil, nil, true) + + fileroot := pc.CopyFile("fileroot", "txn_writer/badinput_fileroot") + err := sw.Write(fileroot, pc.SourceManager) + + if err == nil { + t.Fatal("should have errored when root path is a file, but did not") + } else if !strings.Contains(err.Error(), "does not exist") { + t.Fatalf("expected does not exist error, got %s", err.Error()) } +} + +func TestSafeWriter_ManifestOnly(t *testing.T) { + test.NeedsExternalNetwork(t) + test.NeedsGit(t) + + h := test.NewHelper(t) + defer h.Cleanup() - if err := sw.WriteAllSafe(true); err == nil { - t.Errorf("should fail because no lock was provided from which to write vendor") + pc := NewTestProjectContext(h, safeWriterProject) + defer pc.Release() + pc.CopyFile(ManifestName, safeWriterGoldenManifest) + pc.Load() + + var sw SafeWriter + sw.Prepare(pc.Project.Manifest, nil, nil, false) + + // Verify prepared actions + if !sw.Payload.HasManifest() { + t.Fatal("Expected the payload to contain the manifest") } - // now check dir validation - sw.Root = filepath.Join(td, "nonexistent") - if err := sw.WriteAllSafe(false); err == nil { - t.Errorf("should have errored with nonexistent dir for root path, but did not") + if sw.Payload.HasLock() { + t.Fatal("Did not expect the payload to contain the lock") } + if sw.Payload.HasVendor() { + t.Fatal("Did not expect the payload to contain the vendor directory") + } + + // Write changes + err := sw.Write(pc.Project.AbsRoot, pc.SourceManager) + h.Must(errors.Wrap(err, "SafeWriter.Write failed")) - sw.Root = filepath.Join(td, "myfile") - srcf, err := os.Create(sw.Root) - if err != nil { + // Verify file system changes + if err := pc.ManifestShouldMatchGolden(safeWriterGoldenManifest); err != nil { t.Fatal(err) } - srcf.Close() - if err := sw.WriteAllSafe(false); err == nil { - t.Errorf("should have errored when root path is a file, but did not") + if err := pc.LockShouldNotExist(); err != nil { + t.Fatal(err) + } + if err := pc.VendorShouldNotExist(); err != nil { + t.Fatal(err) } } -func TestTxnWriter(t *testing.T) { +func TestSafeWriter_ManifestAndUnmodifiedLock(t *testing.T) { test.NeedsExternalNetwork(t) test.NeedsGit(t) h := test.NewHelper(t) defer h.Cleanup() - h.TempDir("") + pc := NewTestProjectContext(h, safeWriterProject) + defer pc.Release() + pc.CopyFile(ManifestName, safeWriterGoldenManifest) + pc.CopyFile(LockName, safeWriterGoldenLock) + pc.Load() + + var sw SafeWriter + sw.Prepare(pc.Project.Manifest, pc.Project.Lock, nil, false) + + // Verify prepared actions + if !sw.Payload.HasManifest() { + t.Fatal("Expected the payload to contain the manifest") + } + if !sw.Payload.HasLock() { + t.Fatal("Expected the payload to contain the lock") + } + if sw.Payload.HasVendor() { + t.Fatal("Did not expect the payload to contain the vendor directory") + } + + // Write changes + err := sw.Write(pc.Project.AbsRoot, pc.SourceManager) + h.Must(errors.Wrap(err, "SafeWriter.Write failed")) + + // Verify file system changes + if err := pc.ManifestShouldMatchGolden(safeWriterGoldenManifest); err != nil { + t.Fatal(err) + } + if err := pc.LockShouldMatchGolden(safeWriterGoldenLock); err != nil { + t.Fatal(err) + } + if err := pc.VendorShouldNotExist(); err != nil { + t.Fatal(err) + } +} + +func TestSafeWriter_ManifestAndUnmodifiedLockWithForceVendor(t *testing.T) { + test.NeedsExternalNetwork(t) + test.NeedsGit(t) + + h := test.NewHelper(t) defer h.Cleanup() - c := &Ctx{ - GOPATH: h.Path("."), + pc := NewTestProjectContext(h, safeWriterProject) + defer pc.Release() + pc.CopyFile(ManifestName, safeWriterGoldenManifest) + pc.CopyFile(LockName, safeWriterGoldenLock) + pc.Load() + + var sw SafeWriter + sw.Prepare(pc.Project.Manifest, pc.Project.Lock, nil, true) + + // Verify prepared actions + if !sw.Payload.HasManifest() { + t.Fatal("Expected the payload to contain the manifest") + } + if !sw.Payload.HasLock() { + t.Fatal("Expected the payload to contain the lock") + } + if !sw.Payload.HasVendor() { + t.Fatal("Expected the payload to the vendor directory") } - sm, err := c.SourceManager() - defer sm.Release() - h.Must(err) + + // Write changes + err := sw.Write(pc.Project.AbsRoot, pc.SourceManager) + h.Must(errors.Wrap(err, "SafeWriter.Write failed")) + + // Verify file system changes + if err := pc.ManifestShouldMatchGolden(safeWriterGoldenManifest); err != nil { + t.Fatal(err) + } + if err := pc.LockShouldMatchGolden(safeWriterGoldenLock); err != nil { + t.Fatal(err) + } + if err := pc.VendorShouldExist(); err != nil { + t.Fatal(err) + } +} + +func TestSafeWriter_UnmodifiedLock(t *testing.T) { + test.NeedsExternalNetwork(t) + test.NeedsGit(t) + + h := test.NewHelper(t) + defer h.Cleanup() + + pc := NewTestProjectContext(h, safeWriterProject) + defer pc.Release() + pc.CopyFile(LockName, safeWriterGoldenLock) + pc.Load() var sw SafeWriter - var mpath, lpath, vpath string - var count int - reset := func() { - pr := filepath.Join("src", "txnwriter"+strconv.Itoa(count)) - h.TempDir(pr) - - sw = SafeWriter{ - Root: h.Path(pr), - SourceManager: sm, - } - h.Cd(sw.Root) + sw.Prepare(nil, pc.Project.Lock, pc.Project.Lock, false) - mpath = filepath.Join(sw.Root, ManifestName) - lpath = filepath.Join(sw.Root, LockName) - vpath = filepath.Join(sw.Root, "vendor") + // Verify prepared actions + if sw.Payload.HasManifest() { + t.Fatal("Did not expect the payload to contain the manifest") + } + if sw.Payload.HasLock() { + t.Fatal("Did not expect the payload to contain the lock.") + } + if sw.Payload.HasVendor() { + t.Fatal("Did not expect the payload to contain the vendor directory") + } + + // Write changes + err := sw.Write(pc.Project.AbsRoot, pc.SourceManager) + h.Must(errors.Wrap(err, "SafeWriter.Write failed")) - count++ + // Verify file system changes + // locks are equivalent, so nothing gets written + if err := pc.ManifestShouldNotExist(); err != nil { + t.Fatal(err) } - reset() + if err := pc.VendorShouldNotExist(); err != nil { + t.Fatal(err) + } +} - // super basic manifest and lock - goldenm := "txn_writer/expected_manifest.json" - goldenl := "txn_writer/expected_lock.json" - wantm := h.GetTestFileString(goldenm) - wantl := h.GetTestFileString(goldenl) +func TestSafeWriter_ModifiedLockForceVendor(t *testing.T) { + test.NeedsExternalNetwork(t) + test.NeedsGit(t) - m, err := readManifest(h.GetTestFile(goldenm)) - h.Must(err) - l, err := readLock(h.GetTestFile(goldenl)) - h.Must(err) + h := test.NewHelper(t) + defer h.Cleanup() - // Just write manifest - sw.Manifest = m - h.Must(sw.WriteAllSafe(false)) - h.MustExist(mpath) - h.MustNotExist(lpath) - h.MustNotExist(vpath) - - gotm := h.ReadManifest() - if wantm != gotm { - if *test.UpdateGolden { - wantm = gotm - if err = h.WriteTestFile(goldenm, gotm); err != nil { - t.Fatal(err) - } - } else { - t.Fatalf("expected %s, got %s", wantm, gotm) - } + pc := NewTestProjectContext(h, safeWriterProject) + defer pc.Release() + pc.CopyFile(LockName, safeWriterGoldenLock) + pc.Load() + + var sw SafeWriter + originalLock := new(Lock) + *originalLock = *pc.Project.Lock + originalLock.Memo = []byte{} // zero out the input hash to ensure non-equivalency + sw.Prepare(nil, originalLock, pc.Project.Lock, true) + + // Verify prepared actions + if sw.Payload.HasManifest() { + t.Fatal("Did not expect the payload to contain the manifest") + } + if !sw.Payload.HasLock() { + t.Fatal("Expected the payload to contain the lock") + } + if !sw.Payload.HasVendor() { + t.Fatal("Expected the payload to the vendor directory") } - // Manifest and lock, but no vendor - sw.Lock = l - h.Must(sw.WriteAllSafe(false)) - h.MustExist(mpath) - h.MustExist(lpath) - h.MustNotExist(vpath) + // Write changes + err := sw.Write(pc.Project.AbsRoot, pc.SourceManager) + h.Must(errors.Wrap(err, "SafeWriter.Write failed")) - gotm = h.ReadManifest() - if wantm != gotm { - t.Fatalf("expected %s, got %s", wantm, gotm) + // Verify file system changes + if err := pc.ManifestShouldNotExist(); err != nil { + t.Fatal(err) } + if err := pc.LockShouldMatchGolden(safeWriterGoldenLock); err != nil { + t.Fatal(err) + } + if err := pc.VendorShouldExist(); err != nil { + t.Fatal(err) + } + if err := pc.VendorFileShouldExist("github.com/sdboyer/dep-test"); err != nil { + t.Fatal(err) + } +} - gotl := h.ReadLock() - if wantl != gotl { - if *test.UpdateGolden { - wantl = gotl - if err = h.WriteTestFile(goldenl, gotl); err != nil { - t.Fatal(err) - } - } else { - t.Fatalf("expected %s, got %s", wantl, gotl) - } +func TestSafeWriter_ForceVendorWhenVendorAlreadyExists(t *testing.T) { + test.NeedsExternalNetwork(t) + test.NeedsGit(t) + + h := test.NewHelper(t) + defer h.Cleanup() + + pc := NewTestProjectContext(h, safeWriterProject) + defer pc.Release() + pc.CopyFile(LockName, safeWriterGoldenLock) + pc.Load() + + var sw SafeWriter + // Populate vendor + sw.Prepare(nil, pc.Project.Lock, nil, true) + err := sw.Write(pc.Project.AbsRoot, pc.SourceManager) + h.Must(errors.Wrap(err, "SafeWriter.Write failed")) + + // Verify prepared actions + sw.Prepare(nil, nil, pc.Project.Lock, true) + if sw.Payload.HasManifest() { + t.Fatal("Did not expect the payload to contain the manifest") + } + if !sw.Payload.HasLock() { + t.Fatal("Expected the payload to contain the lock") + } + if !sw.Payload.HasVendor() { + t.Fatal("Expected the payload to the vendor directory") + } + + err = sw.Write(pc.Project.AbsRoot, pc.SourceManager) + h.Must(errors.Wrap(err, "SafeWriter.Write failed")) + + // Verify file system changes + if err := pc.ManifestShouldNotExist(); err != nil { + t.Fatal(err) + } + if err := pc.LockShouldMatchGolden(safeWriterGoldenLock); err != nil { + t.Fatal(err) + } + if err := pc.VendorShouldExist(); err != nil { + t.Fatal(err) + } + if err := pc.VendorFileShouldExist("github.com/sdboyer/dep-test"); err != nil { + t.Fatal(err) } +} + +func TestSafeWriter_DiffLocks(t *testing.T) { + test.NeedsExternalNetwork(t) + test.NeedsGit(t) + + h := test.NewHelper(t) + defer h.Cleanup() + + pc := NewTestProjectContext(h, safeWriterProject) + defer pc.Release() + pc.CopyFile(LockName, "txn_writer/original_lock.json") + pc.Load() + + ulf := h.GetTestFile("txn_writer/updated_lock.json") + defer ulf.Close() + updatedLock, err := readLock(ulf) + h.Must(err) + + var sw SafeWriter + sw.Prepare(nil, pc.Project.Lock, updatedLock, true) - h.Must(sw.WriteAllSafe(true)) - h.MustExist(mpath) - h.MustExist(lpath) - h.MustExist(vpath) - h.MustExist(filepath.Join(vpath, "github.com", "sdboyer", "dep-test")) + // Verify lock diff + diff := sw.Payload.LockDiff + if diff == nil { + t.Fatal("Expected the payload to contain a diff of the lock files") + } + if diff.HashDiff == nil { + t.Fatalf("Expected the lock diff to contain the updated hash: expected %s, got %s", pc.Project.Lock.Memo, updatedLock.Memo) + } - gotm = h.ReadManifest() - if wantm != gotm { - t.Fatalf("expected %s, got %s", wantm, gotm) + if len(diff.Add) != 1 { + t.Fatalf("Expected the lock diff to contain 1 added project, got %d", len(diff.Add)) + } else { + add := diff.Add[0] + if add.Name != "github.com/stuff/realthing" { + t.Errorf("expected new project github.com/stuff/realthing, got %s", add.Name) + } } - gotl = h.ReadLock() - if wantl != gotl { - t.Fatalf("expected %s, got %s", wantl, gotl) + if len(diff.Remove) != 1 { + t.Fatalf("Expected the lock diff to contain 1 removed project, got %d", len(diff.Remove)) + } else { + remove := diff.Remove[0] + if remove.Name != "github.com/stuff/placeholder" { + t.Fatalf("expected new project github.com/stuff/placeholder, got %s", remove.Name) + } } - // start fresh, ignoring the manifest now - reset() - sw.Lock = l - sw.NewLock = l + if len(diff.Modify) != 1 { + t.Fatalf("Expected the lock diff to contain 1 modified project, got %d", len(diff.Modify)) + } else { + modify := diff.Modify[0] + if modify.Name != "github.com/foo/bar" { + t.Fatalf("expected new project github.com/foo/bar, got %s", modify.Name) + } + } - h.Must(sw.WriteAllSafe(false)) - // locks are equivalent, so nothing gets written - h.MustNotExist(mpath) - h.MustNotExist(lpath) - h.MustNotExist(vpath) - - l2 := new(Lock) - *l2 = *l - // zero out the input hash to ensure non-equivalency - l2.Memo = []byte{} - sw.Lock = l2 - h.Must(sw.WriteAllSafe(true)) - h.MustNotExist(mpath) - h.MustExist(lpath) - h.MustExist(vpath) - h.MustExist(filepath.Join(vpath, "github.com", "sdboyer", "dep-test")) - - gotl = h.ReadLock() - if wantl != gotl { - t.Fatalf("expected %s, got %s", wantl, gotl) - } - - // repeat op to ensure good behavior when vendor dir already exists - sw.Lock = nil - h.Must(sw.WriteAllSafe(true)) - h.MustNotExist(mpath) - h.MustExist(lpath) - h.MustExist(vpath) - h.MustExist(filepath.Join(vpath, "github.com", "sdboyer", "dep-test")) - - gotl = h.ReadLock() - if wantl != gotl { - t.Fatalf("expected %s, got %s", wantl, gotl) - } - - // TODO test txn rollback cases. maybe we can force errors with chmodding? + output, err := diff.Format() + h.Must(err) + goldenOutput := "txn_writer/expected_diff_output.txt" + if err = pc.ShouldMatchGolden(goldenOutput, output); err != nil { + t.Fatal(err) + } }