git-p4: use -m when running p4 changes
Simply running "p4 changes" on a large branch can result in a "too many rows scanned" error from the Perforce server. It is better to use a sequence of smaller calls to "p4 changes", using the "-m" option to limit the size of each call. Signed-off-by: Lex Spoon <lex@lexspoon.org> Acked-by: Luke Diamand <luke@diamand.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Lex Spoon committed
Apr 20, 2015 at 11:00 UTC
96b2d54aeeda3af3775f571a0ab706e6f3678d7a
3 files changed
+119
-14
Documentation/git-p4.txt
+14
-3
@@ -225,9 +225,20 @@ Git repository:
225
they can find the p4 branches in refs/heads.
226
227
--max-changes <n>::
228
- Limit the number of imported changes to 'n'. Useful to
229
- limit the amount of history when using the '@all' p4 revision
230
- specifier.
228
+ Import at most 'n' changes, rather than the entire range of
229
+ changes included in the given revision specifier. A typical
230
+ usage would be use '@all' as the revision specifier, but then
231
+ to use '--max-changes 1000' to import only the last 1000
232
+ revisions rather than the entire revision history.
233
+
234
+--changes-block-size <n>::
235
+ The internal block size to use when converting a revision
236
+ specifier such as '@all' into a list of specific change
237
+ numbers. Instead of using a single call to 'p4 changes' to
238
+ find the full list of changes for the conversion, there are a
239
+ sequence of calls to 'p4 changes -m', each of which requests
240
+ one block of changes of the given size. The default block size
241
+ is 500, which should usually be suitable.
242
243
--keep-path::
244
The mapping of file names from the p4 depot path to Git, by
git-p4.py
+41
-11
@@ -740,17 +740,43 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix = "refs/remotes/p4/", silent
740
def originP4BranchesExist():
741
return gitBranchExists("origin") or gitBranchExists("origin/p4") or gitBranchExists("origin/p4/master")
742
743
-def p4ChangesForPaths(depotPaths, changeRange):
743
+def p4ChangesForPaths(depotPaths, changeRange, block_size):
744
assert depotPaths
745
- cmd = ['changes']
746
- for p in depotPaths:
747
- cmd += ["%s...%s" % (p, changeRange)]
748
- output = p4_read_pipe_lines(cmd)
745
+ assert block_size
746
+
747
+ # Parse the change range into start and end
748
+ if changeRange is None or changeRange == '':
749
+ changeStart = '@1'
750
+ changeEnd = '#head'
751
+ else:
752
+ parts = changeRange.split(',')
753
+ assert len(parts) == 2
754
+ changeStart = parts[0]
755
+ changeEnd = parts[1]
756
757
+ # Accumulate change numbers in a dictionary to avoid duplicates
758
changes = {}
751
- for line in output:
752
- changeNum = int(line.split(" ")[1])
753
- changes[changeNum] = True
759
+
760
+ for p in depotPaths:
761
+ # Retrieve changes a block at a time, to prevent running
762
+ # into a MaxScanRows error from the server.
763
+ start = changeStart
764
+ end = changeEnd
765
+ get_another_block = True
766
+ while get_another_block:
767
+ new_changes = []
768
+ cmd = ['changes']
769
+ cmd += ['-m', str(block_size)]
770
+ cmd += ["%s...%s,%s" % (p, start, end)]
771
+ for line in p4_read_pipe_lines(cmd):
772
+ changeNum = int(line.split(" ")[1])
773
+ new_changes.append(changeNum)
774
+ changes[changeNum] = True
775
+ if len(new_changes) == block_size:
776
+ get_another_block = True
777
+ end = '@' + str(min(new_changes))
778
+ else:
779
+ get_another_block = False
780
781
changelist = changes.keys()
782
changelist.sort()
@@ -1911,7 +1937,10 @@ class P4Sync(Command, P4UserMap):
1937
optparse.make_option("--import-labels", dest="importLabels", action="store_true"),
1938
optparse.make_option("--import-local", dest="importIntoRemotes", action="store_false",
1939
help="Import into refs/heads/ , not refs/remotes"),
1914
- optparse.make_option("--max-changes", dest="maxChanges"),
1940
+ optparse.make_option("--max-changes", dest="maxChanges",
1941
+ help="Maximum number of changes to import"),
1942
+ optparse.make_option("--changes-block-size", dest="changes_block_size", type="int",
1943
+ help="Internal block size to use when iteratively calling p4 changes"),
1944
optparse.make_option("--keep-path", dest="keepRepoPath", action='store_true',
1945
help="Keep entire BRANCH/DIR/SUBDIR prefix during import"),
1946
optparse.make_option("--use-client-spec", dest="useClientSpec", action='store_true',
@@ -1940,6 +1969,7 @@ class P4Sync(Command, P4UserMap):
1969
self.syncWithOrigin = True
1970
self.importIntoRemotes = True
1971
self.maxChanges = ""
1972
+ self.changes_block_size = 500
1973
self.keepRepoPath = False
1974
self.depotPaths = None
1975
self.p4BranchesInGit = []
@@ -2586,7 +2616,7 @@ class P4Sync(Command, P4UserMap):
2616
branchPrefix = self.depotPaths[0] + branch + "/"
2617
range = "@1,%s" % maxChange
2618
#print "prefix" + branchPrefix
2589
- changes = p4ChangesForPaths([branchPrefix], range)
2619
+ changes = p4ChangesForPaths([branchPrefix], range, self.changes_block_size)
2620
if len(changes) <= 0:
2621
return False
2622
firstChange = changes[0]
@@ -3002,7 +3032,7 @@ class P4Sync(Command, P4UserMap):
3032
if self.verbose:
3033
print "Getting p4 changes for %s...%s" % (', '.join(self.depotPaths),
3034
self.changeRange)
3005
- changes = p4ChangesForPaths(self.depotPaths, self.changeRange)
3035
+ changes = p4ChangesForPaths(self.depotPaths, self.changeRange, self.changes_block_size)
3036
3037
if len(self.maxChanges) > 0:
3038
changes = changes[:min(int(self.maxChanges), len(changes))]
t/t9818-git-p4-block.sh
new
+64
@@ -0,0 +1,64 @@
1
+#!/bin/sh
2
+
3
+test_description='git p4 fetching changes in multiple blocks'
4
+
5
+. ./lib-git-p4.sh
6
+
7
+test_expect_success 'start p4d' '
8
+ start_p4d
9
+'
10
+
11
+test_expect_success 'Create a repo with ~100 changes' '
12
+ (
13
+ cd "$cli" &&
14
+ >file.txt &&
15
+ p4 add file.txt &&
16
+ p4 submit -d "Add file.txt" &&
17
+ for i in $(test_seq 0 9)
18
+ do
19
+ >outer$i.txt &&
20
+ p4 add outer$i.txt &&
21
+ p4 submit -d "Adding outer$i.txt" &&
22
+ for j in $(test_seq 0 9)
23
+ do
24
+ p4 edit file.txt &&
25
+ echo $i$j >file.txt &&
26
+ p4 submit -d "Commit $i$j" || exit
27
+ done || exit
28
+ done
29
+ )
30
+'
31
+
32
+test_expect_success 'Clone the repo' '
33
+ git p4 clone --dest="$git" --changes-block-size=10 --verbose //depot@all
34
+'
35
+
36
+test_expect_success 'All files are present' '
37
+ echo file.txt >expected &&
38
+ test_write_lines outer0.txt outer1.txt outer2.txt outer3.txt outer4.txt >>expected &&
39
+ test_write_lines outer5.txt outer6.txt outer7.txt outer8.txt outer9.txt >>expected &&
40
+ ls "$git" >current &&
41
+ test_cmp expected current
42
+'
43
+
44
+test_expect_success 'file.txt is correct' '
45
+ echo 99 >expected &&
46
+ test_cmp expected "$git/file.txt"
47
+'
48
+
49
+test_expect_success 'Correct number of commits' '
50
+ (cd "$git" && git log --oneline) >log &&
51
+ test_line_count = 111 log
52
+'
53
+
54
+test_expect_success 'Previous version of file.txt is correct' '
55
+ (cd "$git" && git checkout HEAD^^) &&
56
+ echo 97 >expected &&
57
+ test_cmp expected "$git/file.txt"
58
+'
59
+
60
+test_expect_success 'kill p4d' '
61
+ kill_p4d
62
+'
63
+
64
+test_done