Skip to content

Commit 16fb85c

Browse files
authored
Accelerate reconcile using Perforce digests (#7)
Instead of copying every file from the source repo to the destination repo and then running `p4 revert -a`, switch to comparing the Perforce-provided digest and only copying the files whose digest, type, or case has changed. 1. Replaces `p4 -ztag files -e` with `p4 fstat -T depotFile,headAction,headChange,headType,digest -Ol -F '^(headAction=move/delete | headAction=purge | headAction=archive | headAction=delete)'` (which contains the same information, though it also includes the digest) 2. Updates `p4.runAndParseDepotFiles` to support the `head*` names of fields and also parse `digest` 3. Modifies `Reconcile` to compare digests and only adding files to `Match` if they're actually different in some way This also has the benefit of not returning that there are no changes if only existing file contents has changed, not paths or types.
1 parent 8f0ea09 commit 16fb85c

4 files changed

Lines changed: 39 additions & 20 deletions

File tree

cmd/p4harmonize/update.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -323,8 +323,13 @@ func Reconcile(src []p4.DepotFile, dst []p4.DepotFile) DepotFileDiff {
323323
cmp := strings.Compare(srcCmp, dstCmp)
324324
switch {
325325
case cmp == 0:
326-
out.Match = append(out.Match, [2]p4.DepotFile{src[is], dst[id]})
327-
if src[is].Path != dst[id].Path || src[is].Type != dst[id].Type {
326+
caseDifference := src[is].Path != dst[id].Path
327+
typeDifference := src[is].Type != dst[id].Type
328+
// If we don't have a digest, assume it's different (since we'll use `p4 revert -a`),
329+
// otherwise compare digests to see if there's a difference.
330+
contentDifference := len(src[is].Digest) == 0 || src[is].Digest != dst[id].Digest
331+
if caseDifference || typeDifference || contentDifference {
332+
out.Match = append(out.Match, [2]p4.DepotFile{src[is], dst[id]})
328333
out.HasDifference = true
329334
}
330335
is++

cmd/p4harmonize/update_test.go

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,18 @@ func Test_ReconcileJustPaths(t *testing.T) {
1717
SrcOnly string
1818
DstOnly string
1919
}{
20-
{"simple match", "foo", "foo", "foo:foo", "", ""},
20+
{"simple match (no digest)", "foo", "foo", "foo:foo", "", ""},
2121
{"missing src", "", "foo", "", "", "foo"},
2222
{"missing dst", "foo", "", "", "foo", ""},
2323
{"complex match", "a,b,c,f", "b,c,d,f,g", "b:b,c:c,f:f", "a", "d,g"},
24+
{"same digest", "foo+somedigest", "foo+somedigest", "", "", ""},
25+
{"differing digest", "foo+somedigest", "foo+otherdigest", "foo:foo", "", ""},
2426
{"mismatched case", "a,b,c", "a,B,", "a:a,b:B", "c", ""},
2527
}
2628

2729
for _, tc := range cases {
2830
t.Run(tc.Name, func(t *testing.T) {
29-
src, dst := makeDepotFilesFromStrings(tc.Src, tc.Dst)
31+
src, dst := makeDepotFilesFromString(tc.Src), makeDepotFilesFromString(tc.Dst)
3032
actual := Reconcile(src, dst)
3133

3234
var actualMatch string
@@ -72,16 +74,18 @@ func Test_ReconcileHasDifference(t *testing.T) {
7274
Dst string
7375
Expected bool
7476
}{
75-
{"simple match", "foo", "foo", false},
77+
{"simple match (no digest)", "foo", "foo", true},
7678
{"missing src", "", "foo", true},
7779
{"missing dst", "foo", "", true},
7880
{"complex match", "a,b,c,f", "b,c,d,f,g", true},
7981
{"mismatched case", "a,b,c", "a,B,", true},
82+
{"same digest", "foo+somedigest", "foo+somedigest", false},
83+
{"differing digest", "foo+somedigest", "foo+otherdigest", true},
8084
}
8185

8286
for _, tc := range cases {
8387
t.Run(tc.Name, func(t *testing.T) {
84-
src, dst := makeDepotFilesFromStrings(tc.Src, tc.Dst)
88+
src, dst := makeDepotFilesFromString(tc.Src), makeDepotFilesFromString(tc.Dst)
8589
actual := Reconcile(src, dst)
8690

8791
if tc.Expected != actual.HasDifference {
@@ -91,18 +95,18 @@ func Test_ReconcileHasDifference(t *testing.T) {
9195
}
9296
}
9397

94-
func makeDepotFilesFromStrings(s, d string) (src []p4.DepotFile, dst []p4.DepotFile) {
95-
for _, path := range strings.Split(s, ",") {
98+
func makeDepotFilesFromString(paths string) (depotFiles []p4.DepotFile) {
99+
for _, path := range strings.Split(paths, ",") {
96100
if len(path) == 0 {
97101
continue
98102
}
99-
src = append(src, p4.DepotFile{Path: path})
100-
}
101-
for _, path := range strings.Split(d, ",") {
102-
if len(path) == 0 {
103-
continue
103+
104+
pathAndDigest := strings.SplitN(path, "+", 2)
105+
if len(pathAndDigest) == 2 {
106+
depotFiles = append(depotFiles, p4.DepotFile{Path: pathAndDigest[0], Digest: pathAndDigest[1]})
107+
} else {
108+
depotFiles = append(depotFiles, p4.DepotFile{Path: path})
104109
}
105-
dst = append(dst, p4.DepotFile{Path: path})
106110
}
107-
return src, dst
111+
return depotFiles
108112
}

internal/p4/files.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ import (
44
"fmt"
55
)
66

7-
// ListDepotFiles runs "p4 files -e" and parses the results into a slice of DepotFile structs.
7+
// ListDepotFiles runs "p4 fstat" and parses the results into a slice of DepotFile structs.
88
// Order of resulting slice is alphabetical by Path, ignoring case.
99
func (p *P4) ListDepotFiles() ([]DepotFile, error) {
10-
return p.runAndParseDepotFiles(fmt.Sprintf(`%s -z tag files -e //%s/...`, p.cmd(), p.Client))
10+
return p.runAndParseDepotFiles(fmt.Sprintf(`%s fstat -T depotFile,headAction,headChange,headType,digest -Ol -F '^(headAction=move/delete | headAction=purge | headAction=archive | headAction=delete)' //%s/...`, p.cmd(), p.Client))
1111
}

internal/p4/p4.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ type DepotFile struct {
140140
Action string
141141
CL string
142142
Type string
143+
Digest string
143144
}
144145

145146
// DepotFileCaseInsensitive allows sorting slices of DepotFile by path, but ignoring case.
@@ -152,11 +153,12 @@ func (x DepotFileCaseInsensitive) Less(i, j int) bool {
152153
func (x DepotFileCaseInsensitive) Swap(i, j int) { x[i], x[j] = x[j], x[i] }
153154

154155
// runAndParseDepotFiles calls the given command, which is expected to return a list of records, each
155-
// with at least a depotFile, and optionally also a type, change, and action.
156+
// with at least a depotFile, and optionally also a type, change, action, digest, headType, headChange,
157+
// and headAction.
156158
// The results are then sorted by Path (case-insensitive) and returned.
157159
func (p *P4) runAndParseDepotFiles(cmd string) ([]DepotFile, error) {
158-
if !strings.Contains(cmd, "-ztag") && !strings.Contains(cmd, "-z tag") {
159-
return nil, fmt.Errorf("missing '-z tag' in cmd: %s", cmd)
160+
if !strings.Contains(cmd, "-ztag") && !strings.Contains(cmd, "-z tag") && !strings.Contains(cmd, "fstat") {
161+
return nil, fmt.Errorf("missing '-z tag' in non-fstat cmd: %s", cmd)
160162
}
161163

162164
_, streamDepth, err := p.StreamInfo()
@@ -198,10 +200,18 @@ func (p *P4) runAndParseDepotFiles(cmd string) ([]DepotFile, error) {
198200
cur.Path = raw[len(prefix):]
199201
case strings.HasPrefix(line[4:], "action"):
200202
cur.Action = strings.TrimSpace(line[10:])
203+
case strings.HasPrefix(line[4:], "headAction"):
204+
cur.Action = strings.TrimSpace(line[14:])
201205
case strings.HasPrefix(line[4:], "change"):
202206
cur.CL = strings.TrimSpace(line[10:])
207+
case strings.HasPrefix(line[4:], "headChange"):
208+
cur.CL = strings.TrimSpace(line[14:])
203209
case strings.HasPrefix(line[4:], "type"):
204210
cur.Type = strings.TrimSpace(line[8:])
211+
case strings.HasPrefix(line[4:], "headType"):
212+
cur.Type = strings.TrimSpace(line[12:])
213+
case strings.HasPrefix(line[4:], "digest"):
214+
cur.Digest = strings.TrimSpace(line[10:])
205215
}
206216

207217
return nil

0 commit comments

Comments
 (0)