threads / discuss / 41040

[PATCHv2 3/3] git-p4: reduce number of server queries for fetches

Subject: [PATCHv2 3/3] git-p4: reduce number of server queries for fetches

## tl;dr

5 messages between Dec 19, 2015 and Dec 21, 2015.

replies: 4people: 2as markdown or json

Luke Diamand· Dec 19, 2015, 09:39 UTC · lore

[PATCHv2 0/3] git-p4: fixing p4ChangesForPaths

James Farwell found a bug in p4ChangesForPaths() when handling changes across multiple depot paths.

Sam Hocevar had already submitted a change to the same function to get P4 to do queries across all of the depot paths, in order to reduce server queries, which had the side effect of fixing James' problem.

This followup just fixes Sam's original fix to restore the behavior of p4ChangesForPaths() so that it returns a sorted list of changes. That fixes a failing a testcase.

Luke Diamand (1):
  git-p4: failing test case for skipping changes with multiple depots
Sam Hocevar (2):
  git-p4: support multiple depot paths in p4 submit
  git-p4: reduce number of server queries for fetches
 git-p4.py               | 55 +++++++++++++++++++++++++++----------------------
 t/t9818-git-p4-block.sh | 28 ++++++++++++++++++++++++-
 2 files changed, 57 insertions(+), 26 deletions(-)
-- 
2.6.2.474.g3eb3291
Luke Diamand· Dec 19, 2015, 09:39 UTC · re: Luke Diamand · lore

[PATCHv2 1/3] git-p4: failing test case for skipping changes with multiple depots

James Farwell reported that with multiple depots git-p4 would skip changes.

http://article.gmane.org/gmane.comp.version-control.git/282297
Add a failing test case demonstrating the problem.
Signed-off-by: Luke Diamand <luke@diamand.org>
---
 t/t9818-git-p4-block.sh | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh
index 3b3ae1f..64510b7 100755
--- a/t/t9818-git-p4-block.sh
+++ b/t/t9818-git-p4-block.sh
@@ -84,7 +84,7 @@ p4_add_file() {
 	(cd "$cli" &&
 		>$1 &&
 		p4 add $1 &&
-		p4 submit -d "Added a file" $1
+		p4 submit -d "Added file $1" $1
 	)
 }
 
@@ -112,6 +112,32 @@ test_expect_success 'Syncing files' '
 	)
 '
 
+# Handling of multiple depot paths:
+#    git p4 clone //depot/pathA //depot/pathB
+#
+test_expect_success 'Create a repo with multiple depot paths' '
+	client_view "//depot/pathA/... //client/pathA/..." \
+		    "//depot/pathB/... //client/pathB/..." &&
+	mkdir -p "$cli/pathA" "$cli/pathB" &&
+	for p in pathA pathB
+	do
+		for i in $(test_seq 1 10)
+		do
+			p4_add_file "$p/file$p$i"
+		done
+	done
+'
+
+test_expect_failure 'Clone repo with multiple depot paths' '
+	(
+		cd "$git" &&
+		git p4 clone --changes-block-size=4 //depot/pathA@all //depot/pathB@all \
+			--destination=dest &&
+		ls -1 dest >log &&
+		test_line_count = 20 log
+	)
+'
+
 test_expect_success 'kill p4d' '
 	kill_p4d
 '
-- 
2.6.2.474.g3eb3291
Luke Diamand· Dec 19, 2015, 09:39 UTC · re: Luke Diamand · lore

[PATCHv2 2/3] git-p4: support multiple depot paths in p4 submit

From: Sam Hocevar <sam@hocevar.net>

When submitting from a repository that was cloned using a client spec, use the full list of paths when ruling out files that are outside the view. This fixes a bug where only files pertaining to the first path would be included in the p4 submit.

Signed-off-by: Sam Hocevar <sam@hocevar.net>
Signed-off-by: Luke Diamand <luke@diamand.org>
---
 git-p4.py | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 7a9dd6a..122aff2 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1458,6 +1458,8 @@ class P4Submit(Command, P4UserMap):
            Remove lines in the Files section that show changes to files
            outside the depot path we're committing into."""
 
+        [upstream, settings] = findUpstreamBranchPoint()
+
         template = ""
         inFilesSection = False
         for line in p4_read_pipe_lines(['change', '-o']):
@@ -1470,8 +1472,13 @@ class P4Submit(Command, P4UserMap):
                     lastTab = path.rfind("\t")
                     if lastTab != -1:
                         path = path[:lastTab]
-                        if not p4PathStartsWith(path, self.depotPath):
-                            continue
+                        if settings.has_key('depot-paths'):
+                            if not [p for p in settings['depot-paths']
+                                    if p4PathStartsWith(path, p)]:
+                                continue
+                        else:
+                            if not p4PathStartsWith(path, self.depotPath):
+                                continue
                 else:
                     inFilesSection = False
             else:
-- 
2.6.2.474.g3eb3291
Luke Diamand· Dec 19, 2015, 09:39 UTC · re: Luke Diamand · lore
From: Sam Hocevar <sam@hocevar.net>

When fetching changes from a depot using a full client spec, there is no need to perform as many queries as there are top-level paths in the client spec. Instead we query all changes in chronological order, also getting rid of the need to sort the results and remove duplicates.

Signed-off-by: Sam Hocevar <sam@hocevar.net>
Signed-off-by: Luke Diamand <luke@diamand.org>
---
 git-p4.py               | 44 +++++++++++++++++++++-----------------------
 t/t9818-git-p4-block.sh |  2 +-
 2 files changed, 22 insertions(+), 24 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 122aff2..c33dece 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -822,39 +822,37 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
                 die("cannot use --changes-block-size with non-numeric revisions")
             block_size = None
 
-    # Accumulate change numbers in a dictionary to avoid duplicates
-    changes = {}
+    changes = []
 
-    for p in depotPaths:
-        # Retrieve changes a block at a time, to prevent running
-        # into a MaxResults/MaxScanRows error from the server.
+    # Retrieve changes a block at a time, to prevent running
+    # into a MaxResults/MaxScanRows error from the server.
 
-        while True:
-            cmd = ['changes']
+    while True:
+        cmd = ['changes']
 
-            if block_size:
-                end = min(changeEnd, changeStart + block_size)
-                revisionRange = "%d,%d" % (changeStart, end)
-            else:
-                revisionRange = "%s,%s" % (changeStart, changeEnd)
+        if block_size:
+            end = min(changeEnd, changeStart + block_size)
+            revisionRange = "%d,%d" % (changeStart, end)
+        else:
+            revisionRange = "%s,%s" % (changeStart, changeEnd)
 
+        for p in depotPaths:
             cmd += ["%s...@%s" % (p, revisionRange)]
 
-            for line in p4_read_pipe_lines(cmd):
-                changeNum = int(line.split(" ")[1])
-                changes[changeNum] = True
+        # Insert changes in chronological order
+        for line in reversed(p4_read_pipe_lines(cmd)):
+            changes.append(int(line.split(" ")[1]))
 
-            if not block_size:
-                break
+        if not block_size:
+            break
 
-            if end >= changeEnd:
-                break
+        if end >= changeEnd:
+            break
 
-            changeStart = end + 1
+        changeStart = end + 1
 
-    changelist = changes.keys()
-    changelist.sort()
-    return changelist
+    changes = sorted(changes)
+    return changes
 
 def p4PathStartsWith(path, prefix):
     # This method tries to remedy a potential mixed-case issue:
diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh
index 64510b7..8840a18 100755
--- a/t/t9818-git-p4-block.sh
+++ b/t/t9818-git-p4-block.sh
@@ -128,7 +128,7 @@ test_expect_success 'Create a repo with multiple depot paths' '
 	done
 '
 
-test_expect_failure 'Clone repo with multiple depot paths' '
+test_expect_success 'Clone repo with multiple depot paths' '
 	(
 		cd "$git" &&
 		git p4 clone --changes-block-size=4 //depot/pathA@all //depot/pathB@all \
-- 
2.6.2.474.g3eb3291

← back to recent threads