git-p4: fixing --changes-block-size handling
The --changes-block-size handling was intended to help when a user has a limited "maxscanrows" (see "p4 group"). It used "p4 changes -m $maxchanges" to limit the number of results. Unfortunately, it turns out that the "maxscanrows" and "maxresults" limits are actually applied *before* the "-m maxchanges" parameter is considered (experimentally). Fix the block-size handling so that it gets blocks of changes limited by revision number ($Start..$Start+$N, etc). This limits the number of results early enough that both sets of tests pass. Note that many other Perforce operations can fail for the same reason (p4 print, p4 files, etc) and it's probably not possible to workaround this. In the real world, this is probably not usually a problem. Signed-off-by: Luke Diamand <luke@diamand.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Luke Diamand committed
Jun 10, 2015 at 08:30 UTC
1051ef00636357061d72bcf673da86054fb14a12
2 files changed
+69
-28
git-p4.py
+63
-22
@@ -43,6 +43,9 @@ verbose = False
43
# Only labels/tags matching this will be imported/exported
44
defaultLabelRegexp = r'[a-zA-Z0-9_\-.]+$'
45
46
+# Grab changes in blocks of this many revisions, unless otherwise requested
47
+defaultBlockSize = 512
48
+
49
def p4_build_cmd(cmd):
50
"""Build a suitable p4 command line.
51
@@ -249,6 +252,10 @@ def p4_reopen(type, f):
252
def p4_move(src, dest):
253
p4_system(["move", "-k", wildcard_encode(src), wildcard_encode(dest)])
254
255
+def p4_last_change():
256
+ results = p4CmdList(["changes", "-m", "1"])
257
+ return int(results[0]['change'])
258
+
259
def p4_describe(change):
260
"""Make sure it returns a valid result by checking for
261
the presence of field "time". Return a dict of the
@@ -742,43 +749,77 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix = "refs/remotes/p4/", silent
749
def originP4BranchesExist():
750
return gitBranchExists("origin") or gitBranchExists("origin/p4") or gitBranchExists("origin/p4/master")
751
745
-def p4ChangesForPaths(depotPaths, changeRange, block_size):
752
+
753
+def p4ParseNumericChangeRange(parts):
754
+ changeStart = int(parts[0][1:])
755
+ if parts[1] == '#head':
756
+ changeEnd = p4_last_change()
757
+ else:
758
+ changeEnd = int(parts[1])
759
+
760
+ return (changeStart, changeEnd)
761
+
762
+def chooseBlockSize(blockSize):
763
+ if blockSize:
764
+ return blockSize
765
+ else:
766
+ return defaultBlockSize
767
+
768
+def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
769
assert depotPaths
747
- assert block_size
770
749
- # Parse the change range into start and end
771
+ # Parse the change range into start and end. Try to find integer
772
+ # revision ranges as these can be broken up into blocks to avoid
773
+ # hitting server-side limits (maxrows, maxscanresults). But if
774
+ # that doesn't work, fall back to using the raw revision specifier
775
+ # strings, without using block mode.
776
+
777
if changeRange is None or changeRange == '':
751
- changeStart = '@1'
752
- changeEnd = '#head'
778
+ changeStart = 1
779
+ changeEnd = p4_last_change()
780
+ block_size = chooseBlockSize(requestedBlockSize)
781
else:
782
parts = changeRange.split(',')
783
assert len(parts) == 2
756
- changeStart = parts[0]
757
- changeEnd = parts[1]
784
+ try:
785
+ (changeStart, changeEnd) = p4ParseNumericChangeRange(parts)
786
+ block_size = chooseBlockSize(requestedBlockSize)
787
+ except:
788
+ changeStart = parts[0][1:]
789
+ changeEnd = parts[1]
790
+ if requestedBlockSize:
791
+ die("cannot use --changes-block-size with non-numeric revisions")
792
+ block_size = None
793
794
# Accumulate change numbers in a dictionary to avoid duplicates
795
changes = {}
796
797
for p in depotPaths:
798
# Retrieve changes a block at a time, to prevent running
764
- # into a MaxScanRows error from the server.
765
- start = changeStart
766
- end = changeEnd
767
- get_another_block = True
768
- while get_another_block:
769
- new_changes = []
799
+ # into a MaxResults/MaxScanRows error from the server.
800
+
801
+ while True:
802
cmd = ['changes']
771
- cmd += ['-m', str(block_size)]
772
- cmd += ["%s...%s,%s" % (p, start, end)]
803
+
804
+ if block_size:
805
+ end = min(changeEnd, changeStart + block_size)
806
+ revisionRange = "%d,%d" % (changeStart, end)
807
+ else:
808
+ revisionRange = "%s,%s" % (changeStart, changeEnd)
809
+
810
+ cmd += ["%s...@%s" % (p, revisionRange)]
811
+
812
for line in p4_read_pipe_lines(cmd):
813
changeNum = int(line.split(" ")[1])
775
- new_changes.append(changeNum)
814
changes[changeNum] = True
777
- if len(new_changes) == block_size:
778
- get_another_block = True
779
- end = '@' + str(min(new_changes))
780
- else:
781
- get_another_block = False
815
+
816
+ if not block_size:
817
+ break
818
+
819
+ if end >= changeEnd:
820
+ break
821
+
822
+ changeStart = end + 1
823
824
changelist = changes.keys()
825
changelist.sort()
@@ -1974,7 +2015,7 @@ class P4Sync(Command, P4UserMap):
2015
self.syncWithOrigin = True
2016
self.importIntoRemotes = True
2017
self.maxChanges = ""
1977
- self.changes_block_size = 500
2018
+ self.changes_block_size = None
2019
self.keepRepoPath = False
2020
self.depotPaths = None
2021
self.p4BranchesInGit = []
t/t9818-git-p4-block.sh
+6
-6
@@ -49,11 +49,11 @@ test_expect_success 'Default user cannot fetch changes' '
49
! p4 changes -m 1 //depot/...
50
'
51
52
-test_expect_failure 'Clone the repo' '
52
+test_expect_success 'Clone the repo' '
53
git p4 clone --dest="$git" --changes-block-size=7 --verbose //depot/included@all
54
'
55
56
-test_expect_failure 'All files are present' '
56
+test_expect_success 'All files are present' '
57
echo file.txt >expected &&
58
test_write_lines outer0.txt outer1.txt outer2.txt outer3.txt outer4.txt >>expected &&
59
test_write_lines outer5.txt >>expected &&
@@ -61,18 +61,18 @@ test_expect_failure 'All files are present' '
61
test_cmp expected current
62
'
63
64
-test_expect_failure 'file.txt is correct' '
64
+test_expect_success 'file.txt is correct' '
65
echo 55 >expected &&
66
test_cmp expected "$git/file.txt"
67
'
68
69
-test_expect_failure 'Correct number of commits' '
69
+test_expect_success 'Correct number of commits' '
70
(cd "$git" && git log --oneline) >log &&
71
wc -l log &&
72
test_line_count = 43 log
73
'
74
75
-test_expect_failure 'Previous version of file.txt is correct' '
75
+test_expect_success 'Previous version of file.txt is correct' '
76
(cd "$git" && git checkout HEAD^^) &&
77
echo 53 >expected &&
78
test_cmp expected "$git/file.txt"
@@ -102,7 +102,7 @@ test_expect_success 'Add some more files' '
102
103
# This should pick up the 10 new files in "included", but not be confused
104
# by the additional files in "excluded"
105
-test_expect_failure 'Syncing files' '
105
+test_expect_success 'Syncing files' '
106
(
107
cd "$git" &&
108
git p4 sync --changes-block-size=7 &&