Skip to content

Commit 69fdac2

Browse files
authored
Merge pull request golang#143 from sdboyer/refactor-hashing
Refactor hashing
2 parents 35b6698 + 366fea2 commit 69fdac2

13 files changed

+1169
-620
lines changed

bridge.go

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ type bridge struct {
4040
// held by the solver that it ends up being easier and saner to do this.
4141
s *solver
4242

43+
// Whether to sort version lists for downgrade.
44+
down bool
45+
4346
// Simple, local cache of the root's PackageTree
4447
crp *struct {
4548
ptree PackageTree
@@ -58,17 +61,18 @@ type bridge struct {
5861

5962
// Global factory func to create a bridge. This exists solely to allow tests to
6063
// override it with a custom bridge and sm.
61-
var mkBridge = func(s *solver, sm SourceManager) sourceBridge {
64+
var mkBridge = func(s *solver, sm SourceManager, down bool) sourceBridge {
6265
return &bridge{
6366
sm: sm,
6467
s: s,
68+
down: down,
6569
vlists: make(map[ProjectIdentifier][]Version),
6670
}
6771
}
6872

6973
func (b *bridge) GetManifestAndLock(id ProjectIdentifier, v Version) (Manifest, Lock, error) {
70-
if id.ProjectRoot == ProjectRoot(b.s.rpt.ImportRoot) {
71-
return b.s.rm, b.s.rl, nil
74+
if b.s.rd.isRoot(id.ProjectRoot) {
75+
return b.s.rd.rm, b.s.rd.rl, nil
7276
}
7377

7478
b.s.mtr.push("b-gmal")
@@ -94,7 +98,7 @@ func (b *bridge) ListVersions(id ProjectIdentifier) ([]Version, error) {
9498
return nil, err
9599
}
96100

97-
if b.s.params.Downgrade {
101+
if b.down {
98102
SortForDowngrade(vl)
99103
} else {
100104
SortForUpgrade(vl)
@@ -120,7 +124,7 @@ func (b *bridge) SourceExists(id ProjectIdentifier) (bool, error) {
120124
}
121125

122126
func (b *bridge) vendorCodeExists(id ProjectIdentifier) (bool, error) {
123-
fi, err := os.Stat(filepath.Join(b.s.params.RootDir, "vendor", string(id.ProjectRoot)))
127+
fi, err := os.Stat(filepath.Join(b.s.rd.dir, "vendor", string(id.ProjectRoot)))
124128
if err != nil {
125129
return false, err
126130
} else if fi.IsDir() {
@@ -279,7 +283,7 @@ func (b *bridge) vtu(id ProjectIdentifier, v Version) versionTypeUnion {
279283
// The root project is handled separately, as the source manager isn't
280284
// responsible for that code.
281285
func (b *bridge) ListPackages(id ProjectIdentifier, v Version) (PackageTree, error) {
282-
if id.ProjectRoot == ProjectRoot(b.s.rpt.ImportRoot) {
286+
if b.s.rd.isRoot(id.ProjectRoot) {
283287
panic("should never call ListPackages on root project")
284288
}
285289

@@ -327,7 +331,7 @@ func (b *bridge) breakLock() {
327331
return
328332
}
329333

330-
for _, lp := range b.s.rl.Projects() {
334+
for _, lp := range b.s.rd.rl.Projects() {
331335
if _, is := b.s.sel.selected(lp.pi); !is {
332336
// TODO(sdboyer) use this as an opportunity to detect
333337
// inconsistencies between upstream and the lock (e.g., moved tags)?

constraint_test.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -848,3 +848,58 @@ func TestVersionUnionPanicOnString(t *testing.T) {
848848
}()
849849
_ = versionTypeUnion{}.String()
850850
}
851+
852+
func TestTypedConstraintString(t *testing.T) {
853+
// Also tests typedVersionString(), as this nests down into that
854+
rev := Revision("flooboofoobooo")
855+
v1 := NewBranch("master")
856+
v2 := NewBranch("test").Is(rev)
857+
v3 := NewVersion("1.0.1")
858+
v4 := NewVersion("v2.0.5")
859+
v5 := NewVersion("2.0.5.2")
860+
861+
table := []struct {
862+
in Constraint
863+
out string
864+
}{
865+
{
866+
in: anyConstraint{},
867+
out: "any-*",
868+
},
869+
{
870+
in: noneConstraint{},
871+
out: "none-",
872+
},
873+
{
874+
in: mkSVC("^1.0.0"),
875+
out: "svc-^1.0.0",
876+
},
877+
{
878+
in: v1,
879+
out: "b-master",
880+
},
881+
{
882+
in: v2,
883+
out: "b-test-r-" + string(rev),
884+
},
885+
{
886+
in: v3,
887+
out: "sv-1.0.1",
888+
},
889+
{
890+
in: v4,
891+
out: "sv-v2.0.5",
892+
},
893+
{
894+
in: v5,
895+
out: "pv-2.0.5.2",
896+
},
897+
}
898+
899+
for _, fix := range table {
900+
got := typedConstraintString(fix.in)
901+
if got != fix.out {
902+
t.Errorf("Typed string for %v (%T) was not expected %q; got %q", fix.in, fix.in, fix.out, got)
903+
}
904+
}
905+
}

constraints.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,26 @@ type Constraint interface {
3232
_private()
3333
}
3434

35+
// typedConstraintString emits the normal stringified representation of the
36+
// provided constraint, prefixed with a string that uniquely identifies the type
37+
// of the constraint.
38+
func typedConstraintString(c Constraint) string {
39+
var prefix string
40+
41+
switch tc := c.(type) {
42+
case Version:
43+
return typedVersionString(tc)
44+
case semverConstraint:
45+
prefix = "svc"
46+
case anyConstraint:
47+
prefix = "any"
48+
case noneConstraint:
49+
prefix = "none"
50+
}
51+
52+
return fmt.Sprintf("%s-%s", prefix, c.String())
53+
}
54+
3555
func (semverConstraint) _private() {}
3656
func (anyConstraint) _private() {}
3757
func (noneConstraint) _private() {}

hash.go

Lines changed: 75 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,18 @@ package gps
33
import (
44
"bytes"
55
"crypto/sha256"
6+
"io"
67
"sort"
8+
"strings"
9+
)
10+
11+
// string headers used to demarcate sections in hash input creation
12+
const (
13+
hhConstraints = "-CONSTRAINTS-"
14+
hhImportsReqs = "-IMPORTS/REQS-"
15+
hhIgnores = "-IGNORES-"
16+
hhOverrides = "-OVERRIDES-"
17+
hhAnalyzer = "-ANALYZER-"
718
)
819

920
// HashInputs computes a hash digest of all data in SolveParams and the
@@ -17,99 +28,91 @@ import (
1728
//
1829
// (Basically, this is for memoization.)
1930
func (s *solver) HashInputs() (digest []byte) {
20-
buf := new(bytes.Buffer)
21-
s.writeHashingInputs(buf)
31+
h := sha256.New()
32+
s.writeHashingInputs(h)
2233

23-
hd := sha256.Sum256(buf.Bytes())
34+
hd := h.Sum(nil)
2435
digest = hd[:]
2536
return
2637
}
2738

28-
func (s *solver) writeHashingInputs(buf *bytes.Buffer) {
29-
// Apply overrides to the constraints from the root. Otherwise, the hash
30-
// would be computed on the basis of a constraint from root that doesn't
31-
// actually affect solving.
32-
p := s.ovr.overrideAll(s.rm.DependencyConstraints().merge(s.rm.TestDependencyConstraints()))
33-
34-
for _, pd := range p {
35-
buf.WriteString(string(pd.Ident.ProjectRoot))
36-
buf.WriteString(pd.Ident.Source)
37-
// FIXME Constraint.String() is a surjective-only transformation - tags
38-
// and branches with the same name are written out as the same string.
39-
// This could, albeit rarely, result in input collisions when a real
40-
// change has occurred.
41-
buf.WriteString(pd.Constraint.String())
42-
}
43-
44-
// Write each of the packages, or the errors that were found for a
45-
// particular subpath, into the hash. We need to do this in a
46-
// deterministic order, so expand and sort the map.
47-
var pkgs []PackageOrErr
48-
for _, perr := range s.rpt.Packages {
49-
pkgs = append(pkgs, perr)
50-
}
51-
sort.Sort(sortPackageOrErr(pkgs))
52-
for _, perr := range pkgs {
53-
if perr.Err != nil {
54-
buf.WriteString(perr.Err.Error())
55-
} else {
56-
buf.WriteString(perr.P.Name)
57-
buf.WriteString(perr.P.CommentPath)
58-
buf.WriteString(perr.P.ImportPath)
59-
for _, imp := range perr.P.Imports {
60-
if !isStdLib(imp) {
61-
buf.WriteString(imp)
62-
}
63-
}
64-
for _, imp := range perr.P.TestImports {
65-
if !isStdLib(imp) {
66-
buf.WriteString(imp)
67-
}
68-
}
39+
func (s *solver) writeHashingInputs(w io.Writer) {
40+
writeString := func(s string) {
41+
// Skip zero-length string writes; it doesn't affect the real hash
42+
// calculation, and keeps misleading newlines from showing up in the
43+
// debug output.
44+
if s != "" {
45+
// All users of writeHashingInputs cannot error on Write(), so just
46+
// ignore it
47+
w.Write([]byte(s))
6948
}
7049
}
7150

72-
// Write any require packages given in the root manifest.
73-
if len(s.req) > 0 {
74-
// Dump and sort the reqnores
75-
req := make([]string, 0, len(s.req))
76-
for pkg := range s.req {
77-
req = append(req, pkg)
78-
}
79-
sort.Strings(req)
51+
// We write "section headers" into the hash purely to ease scanning when
52+
// debugging this input-constructing algorithm; as long as the headers are
53+
// constant, then they're effectively a no-op.
54+
writeString(hhConstraints)
55+
56+
// getApplicableConstraints will apply overrides, incorporate requireds,
57+
// apply local ignores, drop stdlib imports, and finally trim out
58+
// ineffectual constraints.
59+
for _, pd := range s.rd.getApplicableConstraints() {
60+
writeString(string(pd.Ident.ProjectRoot))
61+
writeString(pd.Ident.Source)
62+
writeString(typedConstraintString(pd.Constraint))
63+
}
8064

81-
for _, reqp := range req {
82-
buf.WriteString(reqp)
83-
}
65+
// Write out each discrete import, including those derived from requires.
66+
writeString(hhImportsReqs)
67+
imports := s.rd.externalImportList()
68+
sort.Strings(imports)
69+
for _, im := range imports {
70+
writeString(im)
8471
}
8572

86-
// Add the ignored packages, if any.
87-
if len(s.ig) > 0 {
88-
// Dump and sort the ignores
89-
ig := make([]string, 0, len(s.ig))
90-
for pkg := range s.ig {
73+
// Add ignores, skipping any that point under the current project root;
74+
// those will have already been implicitly incorporated by the import
75+
// lister.
76+
writeString(hhIgnores)
77+
ig := make([]string, 0, len(s.rd.ig))
78+
for pkg := range s.rd.ig {
79+
if !strings.HasPrefix(pkg, s.rd.rpt.ImportRoot) || !isPathPrefixOrEqual(s.rd.rpt.ImportRoot, pkg) {
9180
ig = append(ig, pkg)
9281
}
93-
sort.Strings(ig)
82+
}
83+
sort.Strings(ig)
9484

95-
for _, igp := range ig {
96-
buf.WriteString(igp)
97-
}
85+
for _, igp := range ig {
86+
writeString(igp)
9887
}
9988

100-
for _, pc := range s.ovr.asSortedSlice() {
101-
buf.WriteString(string(pc.Ident.ProjectRoot))
89+
// Overrides *also* need their own special entry distinct from basic
90+
// constraints, to represent the unique effects they can have on the entire
91+
// solving process beyond root's immediate scope.
92+
writeString(hhOverrides)
93+
for _, pc := range s.rd.ovr.asSortedSlice() {
94+
writeString(string(pc.Ident.ProjectRoot))
10295
if pc.Ident.Source != "" {
103-
buf.WriteString(pc.Ident.Source)
96+
writeString(pc.Ident.Source)
10497
}
10598
if pc.Constraint != nil {
106-
buf.WriteString(pc.Constraint.String())
99+
writeString(typedConstraintString(pc.Constraint))
107100
}
108101
}
109102

103+
writeString(hhAnalyzer)
110104
an, av := s.b.AnalyzerInfo()
111-
buf.WriteString(an)
112-
buf.WriteString(av.String())
105+
writeString(an)
106+
writeString(av.String())
107+
}
108+
109+
// bytes.Buffer wrapper that injects newlines after each call to Write().
110+
type nlbuf bytes.Buffer
111+
112+
func (buf *nlbuf) Write(p []byte) (n int, err error) {
113+
n, _ = (*bytes.Buffer)(buf).Write(p)
114+
(*bytes.Buffer)(buf).WriteByte('\n')
115+
return n + 1, nil
113116
}
114117

115118
// HashingInputsAsString returns the raw input data used by Solver.HashInputs()
@@ -118,10 +121,10 @@ func (s *solver) writeHashingInputs(buf *bytes.Buffer) {
118121
// This is primarily intended for debugging purposes.
119122
func HashingInputsAsString(s Solver) string {
120123
ts := s.(*solver)
121-
buf := new(bytes.Buffer)
124+
buf := new(nlbuf)
122125
ts.writeHashingInputs(buf)
123126

124-
return buf.String()
127+
return (*bytes.Buffer)(buf).String()
125128
}
126129

127130
type sortPackageOrErr []PackageOrErr

0 commit comments

Comments
 (0)