# [PATCHv2 3/3] git-p4: reduce number of server queries for fetches 5 messages from 2015-12-19 to 2015-12-21. Participants: Luke Diamand, Junio C Hamano. Thread: https://gitlist.dev/t/41040 ## Luke Diamand, 2015-12-19 09:39 Subject: [PATCHv2 0/3] git-p4: fixing p4ChangesForPaths Message-ID: <1450517980-1744-1-git-send-email-luke@diamand.org> URL: https://gitlist.dev/e/1450517980-1744-1-git-send-email-luke%40diamand.org ``` 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, 2015-12-19 09:39 Subject: [PATCHv2 1/3] git-p4: failing test case for skipping changes with multiple depots Message-ID: <1450517980-1744-2-git-send-email-luke@diamand.org> URL: https://gitlist.dev/e/1450517980-1744-2-git-send-email-luke%40diamand.org In-Reply-To: <1450517980-1744-1-git-send-email-luke@diamand.org> ``` 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 --- 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, 2015-12-19 09:39 Subject: [PATCHv2 2/3] git-p4: support multiple depot paths in p4 submit Message-ID: <1450517980-1744-3-git-send-email-luke@diamand.org> URL: https://gitlist.dev/e/1450517980-1744-3-git-send-email-luke%40diamand.org In-Reply-To: <1450517980-1744-1-git-send-email-luke@diamand.org> ``` From: Sam Hocevar 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 Signed-off-by: Luke Diamand --- 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, 2015-12-19 09:39 Subject: [PATCHv2 3/3] git-p4: reduce number of server queries for fetches Message-ID: <1450517980-1744-4-git-send-email-luke@diamand.org> URL: https://gitlist.dev/e/1450517980-1744-4-git-send-email-luke%40diamand.org In-Reply-To: <1450517980-1744-1-git-send-email-luke@diamand.org> ``` From: Sam Hocevar 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 Signed-off-by: Luke Diamand --- 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 ``` ## Junio C Hamano, 2015-12-21 19:27 Subject: Re: [PATCHv2 0/3] git-p4: fixing p4ChangesForPaths Message-ID: URL: https://gitlist.dev/e/xmqqlh8nfvq2.fsf%40gitster.mtv.corp.google.com In-Reply-To: <1450517980-1744-1-git-send-email-luke@diamand.org> ``` Thanks, will replace what was queued on 'pu'. ```