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

Note diffs b/w imported and solved lock #1475

Merged
merged 10 commits into from
Feb 21, 2018
3 changes: 3 additions & 0 deletions cmd/dep/root_analyzer.go
Original file line number Diff line number Diff line change
@@ -167,6 +167,9 @@ func (a *rootAnalyzer) DeriveManifestAndLock(dir string, pr gps.ProjectRoot) (gp
func (a *rootAnalyzer) FinalizeRootManifestAndLock(m *dep.Manifest, l *dep.Lock, ol dep.Lock) {
// Iterate through the new projects in solved lock and add them to manifest
// if they are direct deps and log feedback for all the new projects.
diff := gps.DiffLocks(&ol, l)
bi := fb.NewBrokenImportFeedback(diff)
bi.LogFeedback(a.ctx.Err)
for _, y := range l.Projects() {
var f *fb.ConstraintFeedback
pr := y.Ident().ProjectRoot
131 changes: 124 additions & 7 deletions internal/feedback/feedback.go
Original file line number Diff line number Diff line change
@@ -82,6 +82,117 @@ func (cf ConstraintFeedback) LogFeedback(logger *log.Logger) {
}
}

type brokenImport interface {
Copy link
Member

Choose a reason for hiding this comment

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

Naming: "Import" is not the noun here. "Dependency" is slightly murky in our terminology, but is probably the best noun to use. And, prefer "changed" to "broken," per other comments.

String() string
}

type modifiedImport struct {
source, branch, revision, version *gps.StringDiff
projectPath string
}

func (mi modifiedImport) String() string {
var pv string
var pr string
pp := mi.projectPath

var cr string
var cv string
cp := ""

if mi.revision != nil {
pr = fmt.Sprintf("(%s)", trimSHA(mi.revision.Previous))
cr = fmt.Sprintf("(%s)", trimSHA(mi.revision.Current))
}

if mi.version != nil {
pv = mi.version.Previous
cv = mi.version.Current
} else if mi.branch != nil {
pv = mi.branch.Previous
cv = mi.branch.Current
}

if mi.source != nil {
pp = fmt.Sprintf("%s(%s)", mi.projectPath, mi.source.Previous)
cp = fmt.Sprintf(" for %s(%s)", mi.projectPath, mi.source.Current)
}

// Warning: Unable to preserve imported lock VERSION/BRANCH (REV) for PROJECT(SOURCE). Locking in VERSION/BRANCH (REV) for PROJECT(SOURCE)
return fmt.Sprintf("%v %s for %s. Locking in %v %s%s", pv, pr, pp, cv, cr, cp)
}

type removedImport struct {
source, branch, revision, version *gps.StringDiff
projectPath string
}

func (ri removedImport) String() string {
var pr string
var pv string
pp := ri.projectPath

if ri.revision != nil {
pr = fmt.Sprintf("(%s)", trimSHA(ri.revision.Previous))
}

if ri.version != nil {
pv = ri.version.Previous
} else if ri.branch != nil {
pv = ri.branch.Previous
}

if ri.source != nil {
pp = fmt.Sprintf("%s(%s)", ri.projectPath, ri.source.Previous)
}

// Warning: Unable to preserve imported lock VERSION/BRANCH (REV) for PROJECT(SOURCE). Locking in VERSION/BRANCH (REV) for PROJECT(SOURCE)
return fmt.Sprintf("%v %s for %s. The project was removed from the lock because it is not used.", pv, pr, pp)
}

// BrokenImportFeedback holds information on changes to locks pre- and post- solving.
type BrokenImportFeedback struct {
brokenImports []brokenImport
}

// NewBrokenImportFeedback builds a feedback entry that compares an initially
// imported, unsolved lock to the same lock after it has been solved.
func NewBrokenImportFeedback(ld *gps.LockDiff) *BrokenImportFeedback {
bi := &BrokenImportFeedback{}
for _, lpd := range ld.Modify {
// Ignore diffs where it's just a modified package set
if lpd.Branch == nil && lpd.Revision == nil && lpd.Source == nil && lpd.Version == nil {
continue
}
bi.brokenImports = append(bi.brokenImports, modifiedImport{
projectPath: string(lpd.Name),
source: lpd.Source,
branch: lpd.Branch,
revision: lpd.Revision,
version: lpd.Version,
})
}

for _, lpd := range ld.Remove {
bi.brokenImports = append(bi.brokenImports, removedImport{
projectPath: string(lpd.Name),
source: lpd.Source,
branch: lpd.Branch,
revision: lpd.Revision,
version: lpd.Version,
})
}

return bi
}

// LogFeedback logs a warning for all changes between the initially imported and post- solve locks
func (b BrokenImportFeedback) LogFeedback(logger *log.Logger) {
for _, bi := range b.brokenImports {
logger.Printf("Warning: Unable to preserve imported lock %v\n", bi)
}
}

// GetUsingFeedback returns a dependency "using" feedback message. For example:
//
// Using ^1.0.0 as constraint for direct dep github.com/foo/bar
@@ -99,13 +210,7 @@ func GetUsingFeedback(version, consType, depType, projectPath string) string {
// Locking in v1.1.4 (bc29b4f) for direct dep github.com/foo/bar
// Locking in master (436f39d) for transitive dep github.com/baz/qux
func GetLockingFeedback(version, revision, depType, projectPath string) string {
// Check if it's a valid SHA1 digest and trim to 7 characters.
if len(revision) == 40 {
if _, err := hex.DecodeString(revision); err == nil {
// Valid SHA1 digest
revision = revision[0:7]
}
}
revision = trimSHA(revision)

if depType == DepTypeImported {
if version == "" {
@@ -115,3 +220,15 @@ func GetLockingFeedback(version, revision, depType, projectPath string) string {
}
return fmt.Sprintf("Locking in %s (%s) for %s %s", version, revision, depType, projectPath)
}

// trimSHA checks if revision is a valid SHA1 digest and trims to 7 characters.
func trimSHA(revision string) string {
if len(revision) == 40 {
if _, err := hex.DecodeString(revision); err == nil {
// Valid SHA1 digest
revision = revision[0:7]
}
}

return revision
}
66 changes: 66 additions & 0 deletions internal/feedback/feedback_test.go
Original file line number Diff line number Diff line change
@@ -10,6 +10,7 @@ import (
"strings"
"testing"

"github.com/golang/dep"
"github.com/golang/dep/gps"
)

@@ -92,3 +93,68 @@ func TestFeedback_LockedProject(t *testing.T) {
}
}
}

func TestFeedback_BrokenImport(t *testing.T) {
pi := gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/foo/bar")}

cases := []struct {
oldVersion gps.Version
currentVersion gps.Version
pID gps.ProjectIdentifier
altPID gps.ProjectIdentifier
want string
name string
}{
{
oldVersion: gps.NewVersion("v1.1.4").Pair("bc29b4f"),
currentVersion: gps.NewVersion("v1.2.0").Pair("ia3da28"),
pID: pi,
altPID: pi,
want: "Warning: Unable to preserve imported lock v1.1.4 (bc29b4f) for github.com/foo/bar. Locking in v1.2.0 (ia3da28)",
name: "Basic broken import",
},
{
oldVersion: gps.NewBranch("master").Pair("bc29b4f"),
currentVersion: gps.NewBranch("dev").Pair("ia3da28"),
pID: pi,
altPID: pi,
want: "Warning: Unable to preserve imported lock master (bc29b4f) for github.com/foo/bar. Locking in dev (ia3da28)",
name: "Branches",
},
{
oldVersion: gps.NewBranch("master").Pair("bc29b4f"),
currentVersion: gps.NewBranch("dev").Pair("ia3da28"),
pID: pi,
altPID: gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/foo/boo")},
want: "Warning: Unable to preserve imported lock master (bc29b4f) for github.com/foo/bar. The project was removed from the lock because it is not used.",
name: "Branches",
},
{
oldVersion: gps.NewBranch("master").Pair("bc29b4f"),
currentVersion: gps.NewBranch("dev").Pair("ia3da28"),
pID: gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/foo/boo"), Source: "github.com/das/foo"},
altPID: gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/foo/boo"), Source: "github.com/das/bar"},
want: "Warning: Unable to preserve imported lock master (bc29b4f) for github.com/foo/boo(github.com/das/foo). Locking in dev (ia3da28) for github.com/foo/boo(github.com/das/bar)",
name: "With a source",
},
}

for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
buf := &bytes.Buffer{}
ol := dep.Lock{
P: []gps.LockedProject{gps.NewLockedProject(c.pID, c.oldVersion, nil)},
}
l := dep.Lock{
P: []gps.LockedProject{gps.NewLockedProject(c.altPID, c.currentVersion, nil)},
}
log := log2.New(buf, "", 0)
feedback := NewBrokenImportFeedback(gps.DiffLocks(&ol, &l))
feedback.LogFeedback(log)
got := strings.TrimSpace(buf.String())
if c.want != got {
t.Errorf("Feedbacks are not expected: \n\t(GOT) '%s'\n\t(WNT) '%s'", got, c.want)
}
})
}
}