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

Commit

Permalink
dep: Make SafeWriter use status map for OnChanged
Browse files Browse the repository at this point in the history 
If vendor was empty, nonexistent, or just missing a few targeted items,
the SafeWriter would miss them.
  • Loading branch information
@sdboyer
sdboyer committed Jul 21, 2018
1 parent 2c0659e commit 354108c
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 19 deletions.
2 changes: 1 addition & 1 deletion cmd/dep/ensure.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ func (cmd *ensureCommand) runVendorOnly(ctx *dep.Ctx, args []string, p *dep.Proj

// Pass the same lock as old and new so that the writer will observe no
// difference, and write out only ncessary vendor/ changes.
dw, err := dep.NewSafeWriter(nil, p.Lock, p.Lock, dep.VendorAlways, p.Manifest.PruneOptions)
dw, err := dep.NewSafeWriter(nil, p.Lock, p.Lock, dep.VendorAlways, p.Manifest.PruneOptions, nil)
//dw, err := dep.NewDeltaWriter(p.Lock, p.Lock, p.Manifest.PruneOptions, filepath.Join(p.AbsRoot, "vendor"), dep.VendorAlways)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion cmd/dep/init.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error {
ctx.Err.Printf("Old vendor backed up to %v", vendorbak)
}

sw, err := dep.NewSafeWriter(p.Manifest, nil, p.Lock, dep.VendorAlways, p.Manifest.PruneOptions)
sw, err := dep.NewSafeWriter(p.Manifest, nil, p.Lock, dep.VendorAlways, p.Manifest.PruneOptions, nil)
if err != nil {
return errors.Wrap(err, "init failed: unable to create a SafeWriter")
}
Expand Down
17 changes: 14 additions & 3 deletions txn_writer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ type SafeWriter struct {
// - If oldLock is provided without newLock, error.
//
// - If vendor is VendorAlways without a newLock, error.
func NewSafeWriter(manifest *Manifest, oldLock, newLock *Lock, vendor VendorBehavior, prune gps.CascadingPruneOptions) (*SafeWriter, error) {
func NewSafeWriter(manifest *Manifest, oldLock, newLock *Lock, vendor VendorBehavior, prune gps.CascadingPruneOptions, status map[string]verify.VendorStatus) (*SafeWriter, error) {
sw := &SafeWriter{
Manifest: manifest,
lock: newLock,
Expand All @@ -114,7 +114,18 @@ func NewSafeWriter(manifest *Manifest, oldLock, newLock *Lock, vendor VendorBeha
case VendorAlways:
sw.writeVendor = true
case VendorOnChanged:
sw.writeVendor = sw.lockDiff.Changed(anyExceptHash & ^verify.InputImportsChanged) || (newLock != nil && oldLock == nil)
if newLock != nil && oldLock == nil {
sw.writeVendor = true
} else if sw.lockDiff.Changed(anyExceptHash & ^verify.InputImportsChanged) {
sw.writeVendor = true
} else {
for _, stat := range status {
if stat != verify.NoMismatch {
sw.writeVendor = true
break
}
}
}
}

if sw.writeVendor && newLock == nil {
Expand Down Expand Up @@ -442,7 +453,7 @@ func NewDeltaWriter(oldLock, newLock *Lock, status map[string]verify.VendorStatu
if err != nil && os.IsNotExist(err) {
// Provided dir does not exist, so there's no disk contents to compare
// against. Fall back to the old SafeWriter.
return NewSafeWriter(nil, oldLock, newLock, behavior, prune)
return NewSafeWriter(nil, oldLock, newLock, behavior, prune, status)
}

sw.lockDiff = verify.DiffLocks(oldLock, newLock)
Expand Down
28 changes: 14 additions & 14 deletions txn_writer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestSafeWriter_BadInput_MissingRoot(t *testing.T) {
pc := NewTestProjectContext(h, safeWriterProject)
defer pc.Release()

sw, _ := NewSafeWriter(nil, nil, nil, VendorOnChanged, defaultCascadingPruneOptions())
sw, _ := NewSafeWriter(nil, nil, nil, VendorOnChanged, defaultCascadingPruneOptions(), nil)
err := sw.Write("", pc.SourceManager, true, nil)

if err == nil {
Expand All @@ -51,7 +51,7 @@ func TestSafeWriter_BadInput_MissingSourceManager(t *testing.T) {
pc.CopyFile(LockName, safeWriterGoldenLock)
pc.Load()

sw, _ := NewSafeWriter(nil, nil, pc.Project.Lock, VendorAlways, defaultCascadingPruneOptions())
sw, _ := NewSafeWriter(nil, nil, pc.Project.Lock, VendorAlways, defaultCascadingPruneOptions(), nil)
err := sw.Write(pc.Project.AbsRoot, nil, true, nil)

if err == nil {
Expand All @@ -67,7 +67,7 @@ func TestSafeWriter_BadInput_ForceVendorMissingLock(t *testing.T) {
pc := NewTestProjectContext(h, safeWriterProject)
defer pc.Release()

_, err := NewSafeWriter(nil, nil, nil, VendorAlways, defaultCascadingPruneOptions())
_, err := NewSafeWriter(nil, nil, nil, VendorAlways, defaultCascadingPruneOptions(), nil)
if err == nil {
t.Fatal("should have errored without a lock when forceVendor is true, but did not")
} else if !strings.Contains(err.Error(), "newLock") {
Expand All @@ -83,7 +83,7 @@ func TestSafeWriter_BadInput_OldLockOnly(t *testing.T) {
pc.CopyFile(LockName, safeWriterGoldenLock)
pc.Load()

_, err := NewSafeWriter(nil, pc.Project.Lock, nil, VendorAlways, defaultCascadingPruneOptions())
_, err := NewSafeWriter(nil, pc.Project.Lock, nil, VendorAlways, defaultCascadingPruneOptions(), nil)
if err == nil {
t.Fatal("should have errored with only an old lock, but did not")
} else if !strings.Contains(err.Error(), "oldLock") {
Expand All @@ -97,7 +97,7 @@ func TestSafeWriter_BadInput_NonexistentRoot(t *testing.T) {
pc := NewTestProjectContext(h, safeWriterProject)
defer pc.Release()

sw, _ := NewSafeWriter(nil, nil, nil, VendorOnChanged, defaultCascadingPruneOptions())
sw, _ := NewSafeWriter(nil, nil, nil, VendorOnChanged, defaultCascadingPruneOptions(), nil)

missingroot := filepath.Join(pc.Project.AbsRoot, "nonexistent")
err := sw.Write(missingroot, pc.SourceManager, true, nil)
Expand All @@ -115,7 +115,7 @@ func TestSafeWriter_BadInput_RootIsFile(t *testing.T) {
pc := NewTestProjectContext(h, safeWriterProject)
defer pc.Release()

sw, _ := NewSafeWriter(nil, nil, nil, VendorOnChanged, defaultCascadingPruneOptions())
sw, _ := NewSafeWriter(nil, nil, nil, VendorOnChanged, defaultCascadingPruneOptions(), nil)

fileroot := pc.CopyFile("fileroot", "txn_writer/badinput_fileroot")
err := sw.Write(fileroot, pc.SourceManager, true, nil)
Expand All @@ -139,7 +139,7 @@ func TestSafeWriter_Manifest(t *testing.T) {
pc.CopyFile(ManifestName, safeWriterGoldenManifest)
pc.Load()

sw, _ := NewSafeWriter(pc.Project.Manifest, nil, nil, VendorOnChanged, defaultCascadingPruneOptions())
sw, _ := NewSafeWriter(pc.Project.Manifest, nil, nil, VendorOnChanged, defaultCascadingPruneOptions(), nil)

// Verify prepared actions
if !sw.HasManifest() {
Expand Down Expand Up @@ -181,7 +181,7 @@ func TestSafeWriter_ManifestAndUnmodifiedLock(t *testing.T) {
pc.CopyFile(LockName, safeWriterGoldenLock)
pc.Load()

sw, _ := NewSafeWriter(pc.Project.Manifest, pc.Project.Lock, pc.Project.Lock, VendorOnChanged, defaultCascadingPruneOptions())
sw, _ := NewSafeWriter(pc.Project.Manifest, pc.Project.Lock, pc.Project.Lock, VendorOnChanged, defaultCascadingPruneOptions(), nil)

// Verify prepared actions
if !sw.HasManifest() {
Expand Down Expand Up @@ -226,7 +226,7 @@ func TestSafeWriter_ManifestAndUnmodifiedLockWithForceVendor(t *testing.T) {
pc.CopyFile(LockName, safeWriterGoldenLock)
pc.Load()

sw, _ := NewSafeWriter(pc.Project.Manifest, pc.Project.Lock, pc.Project.Lock, VendorAlways, defaultCascadingPruneOptions())
sw, _ := NewSafeWriter(pc.Project.Manifest, pc.Project.Lock, pc.Project.Lock, VendorAlways, defaultCascadingPruneOptions(), nil)

// Verify prepared actions
if !sw.HasManifest() {
Expand Down Expand Up @@ -273,12 +273,12 @@ func TestSafeWriter_ForceVendorWhenVendorAlreadyExists(t *testing.T) {
pc.CopyFile(LockName, safeWriterGoldenLock)
pc.Load()

sw, _ := NewSafeWriter(nil, pc.Project.Lock, pc.Project.Lock, VendorAlways, defaultCascadingPruneOptions())
sw, _ := NewSafeWriter(nil, pc.Project.Lock, pc.Project.Lock, VendorAlways, defaultCascadingPruneOptions(), nil)
err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, nil)
h.Must(errors.Wrap(err, "SafeWriter.Write failed"))

// Verify prepared actions
sw, _ = NewSafeWriter(nil, nil, pc.Project.Lock, VendorAlways, defaultCascadingPruneOptions())
sw, _ = NewSafeWriter(nil, nil, pc.Project.Lock, VendorAlways, defaultCascadingPruneOptions(), nil)
if sw.HasManifest() {
t.Fatal("Did not expect the payload to contain the manifest")
}
Expand Down Expand Up @@ -325,7 +325,7 @@ func TestSafeWriter_NewLock(t *testing.T) {
defer lf.Close()
newLock, err := readLock(lf)
h.Must(err)
sw, _ := NewSafeWriter(nil, nil, newLock, VendorOnChanged, defaultCascadingPruneOptions())
sw, _ := NewSafeWriter(nil, nil, newLock, VendorOnChanged, defaultCascadingPruneOptions(), nil)

// Verify prepared actions
if sw.HasManifest() {
Expand Down Expand Up @@ -372,7 +372,7 @@ func TestSafeWriter_NewLockSkipVendor(t *testing.T) {
defer lf.Close()
newLock, err := readLock(lf)
h.Must(err)
sw, _ := NewSafeWriter(nil, nil, newLock, VendorNever, defaultCascadingPruneOptions())
sw, _ := NewSafeWriter(nil, nil, newLock, VendorNever, defaultCascadingPruneOptions(), nil)

// Verify prepared actions
if sw.HasManifest() {
Expand Down Expand Up @@ -433,7 +433,7 @@ func TestSafeWriter_VendorDotGitPreservedWithForceVendor(t *testing.T) {
pc.CopyFile(LockName, safeWriterGoldenLock)
pc.Load()

sw, _ := NewSafeWriter(pc.Project.Manifest, pc.Project.Lock, pc.Project.Lock, VendorAlways, defaultCascadingPruneOptions())
sw, _ := NewSafeWriter(pc.Project.Manifest, pc.Project.Lock, pc.Project.Lock, VendorAlways, defaultCascadingPruneOptions(), nil)

// Verify prepared actions
if !sw.HasManifest() {
Expand Down

0 comments on commit 354108c

Please sign in to comment.